Skip to content

fix(desktop): remove broken settings section navigation - #1251

Merged
skevetter merged 2 commits into
mainfrom
fix/settings-continuous-scroll
Sep 21, 2026
Merged

skevetter merged 2 commits into
mainfrom
fix/settings-continuous-scroll

Conversation

@skevetter

Copy link
Copy Markdown
Contributor

Summary

  • remove the Settings section navigation that routes hash links back to Dashboard
  • keep General, Appearance, Updates, and Advanced as one continuous scrollable page
  • add a regression test that rejects section navigation links and verifies every section remains rendered

Cause

The app uses svelte-spa-router, where #/... owns the URL hash. Plain href="#general" section links replace that router hash, so the router falls back to Dashboard instead of scrolling.

Validation

  • npm --prefix desktop run check
  • npm --prefix desktop test -- --run src/renderer/src/pages/SettingsPage.test.ts src/renderer/src/lib/components/update/UpdatesPanel.test.ts
  • npm --prefix desktop run electron:build
  • Electron render inspected at 1200 px and 640 px widths under Xvfb

Signed-off-by: Samuel K <skevetter@pm.me>
Signed-off-by: Samuel K <skevetter@pm.me>
@netlify

netlify Bot commented Sep 21, 2026

Copy link
Copy Markdown

Deploy Preview for devsydev canceled.

Name Link
🔨 Latest commit 210326b
🔍 Latest deploy log https://app.netlify.com/projects/devsydev/deploys/6ab0b0e9b09fde0008186b76

@coderabbitai

coderabbitai Bot commented Sep 21, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 17 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 848f9920-42ec-4d90-a1d3-932574a6abb1

📥 Commits

Reviewing files that changed from the base of the PR and between b88fad1 and 210326b.

📒 Files selected for processing (2)
  • desktop/src/renderer/src/pages/SettingsPage.svelte
  • desktop/src/renderer/src/pages/SettingsPage.test.ts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@netlify

netlify Bot commented Sep 21, 2026

Copy link
Copy Markdown

Deploy Preview for images-devsy-sh canceled.

Name Link
🔨 Latest commit 210326b
🔍 Latest deploy log https://app.netlify.com/projects/images-devsy-sh/deploys/6ab0b0e9acb27f0008e819e9

@skevetter

Copy link
Copy Markdown
Contributor Author

@greptileai review

@greptile-apps

greptile-apps Bot commented Sep 21, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge with no actionable correctness, security, or repository-rule issues identified.

Summary

This PR removes the Settings section navigation whose plain hash links conflicted with svelte-spa-router, while retaining General, Appearance, Updates, and Advanced as a continuous page.

  • Removes the navigation sidebar and its responsive grid wrapper.
  • Preserves all four settings sections and their existing content.
  • Adds regression coverage confirming the navigation is absent and every section remains rendered.

Reviews (1) · Last reviewed commit: "test(desktop): cover continuous settings..."

@skevetter
skevetter marked this pull request as ready for review September 21, 2026 05:38
@mergify

mergify Bot commented Sep 21, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@skevetter
skevetter merged commit 9d47372 into main Sep 21, 2026
32 checks passed
@skevetter
skevetter deleted the fix/settings-continuous-scroll branch September 21, 2026 05:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant