Skip to content

fix(appearance): unify zoom controls with a configurable default - #1720

Open
rodboev wants to merge 21 commits into
kenn-io:mainfrom
rodboev:pr/1697-appearance-preferences
Open

fix(appearance): unify zoom controls with a configurable default#1720
rodboev wants to merge 21 commits into
kenn-io:mainfrom
rodboev:pr/1697-appearance-preferences

Conversation

@rodboev

@rodboev rodboev commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Appearance, the desktop status bar, and desktop shortcuts use one Zoom preference in place of the separate Text size and Desktop zoom controls. Existing local preferences migrate to it, and all controls use the same supported percentages.

The optional zoom_level in config.toml supplies a default for browsers and desktop webviews without a local preference. Interface changes stay in that client’s local storage, so clients with different displays keep independent choices. Manual config edits require a daemon restart and client reload; explicit settings API updates retain validated TOML persistence.

Fixed typeahead menus, context menus, and chart tooltips keep their viewport positioning while their contents scale with the interface. The date picker retains its existing scale because the pinned component fixes its panel width. The pinned kit-ui integration and browser coverage are documented in DESIGN.md.

Closes #1697

@roborev-ci

roborev-ci Bot commented Sep 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (bb9e9c4)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 1m30s

@rodboev
rodboev marked this pull request as draft September 11, 2026 05:19
@rodboev
rodboev force-pushed the pr/1697-appearance-preferences branch from bb9e9c4 to a700645 Compare September 11, 2026 07:36
@rodboev rodboev changed the title feat(frontend): expose desktop zoom in appearance refactor(frontend): share one zoom preference across desktop and web Sep 11, 2026
@rodboev
rodboev marked this pull request as ready for review September 11, 2026 07:36
@roborev-ci

roborev-ci Bot commented Sep 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (a700645)

Verdict: Changes require fixes for 1 finding.

Medium

  • frontend/src/lib/stores/ui.svelte.ts:309-322: If a native zoom request fails after the webview was previously zoomed successfully, the fallback path applies the requested CSS zoom without accounting for the existing native factor, producing incorrect effective zoom (for example, 150% native followed by failed 120% fallback yields 180%). Track the last confirmed native zoom and compensate CSS relative to it, restore native zoom to 100% before switching to CSS fallback, or serialize native requests to prevent stale native state.

    Reported by: codex


Reviewers: 2 done | Synthesis: codex, 8s | Total: 6m8s

@roborev-ci

roborev-ci Bot commented Sep 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (0e82108)

Verdict: Changes require fixes for 1 finding.

Medium

  • frontend/src/lib/stores/ui.svelte.ts:317-340: A successful native zoom request that later becomes stale is not recorded in confirmedNativeZoom. If a newer request fails and its reset also fails, CSS fallback divides by the older factor, causing native and CSS scaling to compound incorrectly. Track the native factor whenever each queued native request succeeds, even if its UI request is stale, while gating CSS updates on the latest request; alternatively maintain the actual queued native state explicitly before computing fallback compensation.

    Reported by: codex


Reviewers: 2 done | Synthesis: codex, 8s | Total: 9m48s

@roborev-ci

roborev-ci Bot commented Sep 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (1e4ffde)

Verdict: No findings at or above medium severity.


Reviewers: 2 done | Synthesis: codex, 7s | Total: 11m24s

@rodboev
rodboev marked this pull request as draft September 11, 2026 10:06
@rodboev rodboev changed the title refactor(frontend): share one zoom preference across desktop and web fix(settings): persist shared zoom in config.toml Sep 11, 2026
@rodboev
rodboev marked this pull request as ready for review September 11, 2026 10:07
@roborev-ci

roborev-ci Bot commented Sep 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (16b01d6)

Verdict: Changes require fixes for 2 findings.

Medium

  • frontend/src/lib/components/settings/AppearanceSettings.svelte:97: The only browser zoom control is disabled for read-only backends, even when the server omits zoom_level and the documented behavior is to use a client-local preference. This regresses browser users of read-only PostgreSQL/DuckDB views; desktop status-bar controls still allow local changes. Allow zoom changes locally in read-only mode and skip only the server PUT, or disable the control only when a server-configured zoom must be enforced.

    Reported by: codex

  • frontend/src/lib/stores/ui.svelte.ts:353-355: Server hydration overwrites the generic local zoom key. After connecting to a server configured at (for example) 150%, switching to another server without zoom_level or removing the config causes that server's value to be reused as the client's local preference, contrary to the documented local fallback behavior. Keep the client-local fallback separate from the server-provided override, or scope fallback storage per server, and avoid persisting server hydration as the generic local preference.

    Reported by: codex


Reviewers: 2 done | Synthesis: codex, 8s | Total: 9m44s

@rodboev
rodboev marked this pull request as draft September 11, 2026 10:20
@roborev-ci

roborev-ci Bot commented Sep 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (de12c95)

Verdict: Changes require fixes for 1 finding.

Medium

  • frontend/src/lib/stores/settings.svelte.ts:89-122: load() snapshots zoomSaveVersion before a pending zoom save executes. If the user changes zoom while another settings mutation is active, the reload can apply stale server zoom, and the queued save does not reapply the user's selection to the UI. Track queued or in-flight zoom mutations separately, preserve the latest user zoom across reloads, or reapply the latest zoom after the queued save completes while hydrating only from current responses.

    Reported by: codex


Reviewers: 2 done | Synthesis: codex, 7s | Total: 7m34s

@roborev-ci

roborev-ci Bot commented Sep 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (b55cf4a)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 9m45s

@roborev-ci

roborev-ci Bot commented Sep 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (5c3d620)

Verdict: Changes require fixes for 3 findings.

Medium

  • frontend/src/lib/components/settings/AppearanceSettings.svelte:97: The Appearance Zoom control is disabled whenever the backend is read-only, preventing browser users from changing local interface zoom even when no server zoom is configured. Allow local zoom changes in read-only mode and skip server persistence unless a configured server zoom must be enforced.

    Reported by: codex

  • frontend/src/lib/stores/ui.svelte.ts:353-355: Server hydration persists the effective server zoom into the generic localStorage preference, so switching from a server configured at 150% to one without zoom_level incorrectly reuses 150% instead of the prior client-local preference. Keep server-provided zoom separate from the client-local preference, or scope local preferences per server, and do not persist hydration as a generic local choice.

    Reported by: codex

  • frontend/src/lib/components/settings/AppearanceSettings.svelte:153-159: Compensation rules for fixed kit-ui menus are scoped to AppearanceSettings, which is mounted only on the settings route; menus on other routes can therefore be mis-sized or mis-positioned under browser zoom. Move the compensation rules to an app-wide stylesheet or apply them whenever any fixed kit-ui menu is rendered.

    Reported by: codex


Reviewers: 2 done | Synthesis: codex, 9s | Total: 4m21s

@roborev-ci

roborev-ci Bot commented Sep 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (17094fd)

Verdict: Changes require fixes for 1 finding.

Medium

  • frontend/src/app.css:10-17: Root CSS zoom also scales custom fixed overlays, but compensation is applied only to kit-ui menus. The portaled session context menu and analytics tooltips use viewport coordinates, so at non-100% zoom they can be displaced or clipped. Apply the same coordinate compensation to custom fixed overlays or adjust their viewport coordinates for the active zoom, with geometry coverage for context menus and tooltips.

    Reported by: codex


Reviewers: 2 done | Synthesis: codex, 10s | Total: 8m54s

@roborev-ci

roborev-ci Bot commented Sep 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (27a7301)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 7m59s

@rodboev
rodboev marked this pull request as ready for review September 11, 2026 12:12
@rodboev
rodboev requested a review from wesm September 11, 2026 12:17
@wesm wesm self-assigned this Sep 11, 2026
@mariusvniekerk mariusvniekerk self-assigned this Sep 11, 2026
Keep supported zoom percentages in one declaration so config validation,
API schemas, and frontend controls cannot drift as choices change. Use
the existing enum generator and preserve numeric JSON and TOML values.
@roborev-ci

roborev-ci Bot commented Sep 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (41119ab)

Verdict: Changes require fixes for 1 finding.

Medium

  • frontend/src/lib/stores/settings.svelte.ts:159-168: Stale settings loads unconditionally update loaded and loading state, while only zoom hydration is generation-guarded. During overlapping loads, zoom changes may be saved to the wrong backend or dropped due to stale readOnly state. Serialize loads or generation-guard all load-state updates, including loaded, loading, readOnly, and errors; keep zoom saves pending until the latest load establishes the current backend mode.

    Reported by: codex


Reviewers: 2 done | Synthesis: codex, 8s | Total: 13m40s

Settings requests could outlive appearance assertions and fail during
browser teardown. Fetch the isolated settings fixture before navigation
so route callbacks only serve the captured response.
@roborev-ci

roborev-ci Bot commented Sep 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (281c030)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 8m43s

Resolve overlapping appearance settings changes so shared zoom and the
new XML rendering preference remain available together.
@roborev-ci

roborev-ci Bot commented Sep 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (1cfa0b9)

Verdict: Changes require fixes for 1 finding.

Medium

  • frontend/src/lib/stores/ui.svelte.ts:339-355: When a native setZoom request fails, its handler queues a reset to factor 1. A newer zoom request may already be queued ahead of that reset; it can succeed, then the stale failure handler resets the native zoom to 1 while skipping its CSS update because its request is obsolete. Make the fallback reset version-aware at execution time, or serialize the failed request's reset and any newer request so an obsolete reset cannot run after a newer zoom has been applied.

    Reported by: codex


Reviewers: 2 done | Synthesis: codex, 7s | Total: 8m17s

@roborev-ci

roborev-ci Bot commented Sep 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (842c019)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 12m25s

@wesm

wesm commented Sep 12, 2026

Copy link
Copy Markdown
Member

looking

Clients on different displays need independent zoom preferences. Use the
configured zoom as a default and keep interface changes in local storage,
removing the server write queue from zoom controls.

Keep fixed overlay positions in viewport coordinates while scaling their
contents with the interface. Document when config edits take effect.
@wesm wesm changed the title fix(settings): persist shared zoom in config.toml fix(appearance): unify zoom controls with a configurable default Sep 12, 2026
@roborev-ci

roborev-ci Bot commented Sep 12, 2026

Copy link
Copy Markdown

roborev: Combined Review (36e2e3b)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 9m53s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Persist appearance preferences

3 participants