fix(settings): make the interface font size a pixel slider - #1962
Merged
Conversation
GeneralSettingsSchema.fontSizePx and VaultPreferencesSchema.fontSizePx both declare .int(), but resolveFontSizePx only clamped. readPreferences is hand-rolled and never parses through the schema, so a hand-edited fontSizePx of 16.4 in config.json reached the root element as '16.4px', was announced as aria-valuenow="16.4", and made every step-1 arrow produce another fraction that never lands on a legacy bucket.
Radix puts role="slider" on the Thumb, and a single-thumb slider gets no generated label, so the Thumb resolved its name from its own props and found nothing. The Root the aria-label landed on is a role-less span, so a screen reader announced the font size control unnamed — a regression against the SegmentedControl it replaced. Route the name to the Thumb. Neither audio-player slider passes an aria-label, so no consumer needed migrating. The drag tests move to the Root, which is now the only one of the two elements the label does not select. The reset button had no styled focus indicator; give it the same focus-visible ring the other settings sections use.
Replaces four overlapping guards with a single path. onValueChange previews and schedules; there is no onValueCommit, whose semantics caused three of these: - Radix skips onValueCommit when pointer-up lands on the value pointer-down started from, so dragging out to 20 and back to 16 left fontSizePxDraft stuck at 16 forever. A later remote change then resized the app while the slider and readout kept showing 16. - Radix commits on every keydown, so holding ArrowRight from 12 to 24 fired twelve IPC round trips, twelve config.json rewrites and twelve encrypted settings uploads. The S/M/L control it replaced could manage three. - The commit cleared the draft unconditionally after its await, discarding one a later call owned, which made a held arrow key jump backwards. - The preview writes the root font size directly and useThemeSync cannot undo it, so unmounting mid-drag stranded the whole interface at the previewed size until restart. The unmount cleanup now flushes the pending write instead of dropping it. A commit whose value already equals the saved one skips the write and just releases the draft, and the draft is only released by the call that still owns it. The three new tests fail against the previous mechanism: the row reports aria-valuenow 16 against a remote 22, updateSettings is called 6 times instead of 1, and the unmount saves nothing while the page sits at 12px.
general.fontSize and general.fontSizePx carry independent field clocks, and mergeRemote only writes a field the inbound payload actually carries. A device on a build from before the slider pushes only the bucket, so its "make it Small" left the pixel value untouched, resolveFontSizePx preferred the pixel value unconditionally, and the change was ignored forever while config.json kept fontSize 'small' next to fontSizePx 22. Reconcile at the propagate boundary: pass the merged field clocks through with the merged settings and let the bucket supply the pixel value when its clock is strictly newer. Concurrent and missing-clock cases keep the pixel value. Derived locally, deliberately. No updateField call and no clock bump, so the synced payload is untouched and every device that receives the same merge derives the same value. Deterministic, idempotent, and clear of the echo-dedupe path. Against the previous code the two-device test writes fontSizePx 22 beside fontSize 'small'; with the fix it writes 14.
resolveFontSizePx preferred a finite fontSizePx unconditionally, so a device
on a build from before the slider could only ever move general.fontSize and
its change was ignored forever.
Every build that has the slider writes the pair atomically as
{ fontSizePx: px, fontSize: toLegacyFontSize(px) }, so a coherent pair always
satisfies toLegacyFontSize(px) === fontSize. An old build breaks that
invariant precisely when it moves the bucket alone, so the pixel value is
trusted only while the pair agrees, and every read path self-heals: the
renderer row, useThemeSync, readPreferences and the settings cache all funnel
through here.
The two fields carry independent per-field vector clocks, so nothing here
compares them. An absent or unknown bucket cannot contradict a pixel value,
and rounding happens first so a fractional value is not failed against an
invariant it could never satisfy.
reconcileFontSizePx decided between general.fontSizePx and general.fontSize by calling compare() on their two field clocks. SettingsSyncManager.updateField increments a clock per field path, so the two are independent per-device counters with unrelated histories and comparing them is unsound. It also regressed the migration it existed for. A user who had changed the font size before upgrading carries general.fontSize one tick ahead of general.fontSizePx forever, because the very first slider drag writes both fields and every later drag bumps both by one. Any inbound settings payload about any setting at all then read 'before' and discarded the slider value, so a theme change from another device silently reset the user's size. The value-coherence rule in resolveFontSizePx replaces it, so the handler needs no clocks: the two-device scenario the deleted tests proved now runs end to end through the real writePreferences/readPreferences pair in settings-handler.font-size.test.ts, alongside the regression guard for the unrelated-merge case this commit's mechanism failed (it resolved 20, not 22).
Three fixtures set fontSizePx to 22 and left fontSize at 'medium'. No build
writes that pair — a save always writes { fontSizePx: px, fontSize:
toLegacyFontSize(px) } — and resolveFontSizePx now reads a disagreeing pair as
an older device having moved the bucket alone, so it resolved 16 and the reset
button had nothing to write.
Also fixes the ApplyContext shape in the new two-device test, which carried
deviceId/vaultId rather than the emit the interface declares.
#1961 replaced the Custom Font text field with a system font picker and moved FONT_FAMILY_MAP into lib/interface-font, touching the same Typography rows and the same theme-sync effects as this branch. Both features are kept. The font size row is the slider from this branch; the font family row is the picker from main. Dropped from both sides: the FONT_SIZE_MAP and FONT_SIZE_OPTIONS this branch had already replaced, and the customFontDraft / commitCustomFont state whose input #1961 removed.
github-actions
Bot
added
bug
h4yfans
marked this pull request as ready for review
September 2, 2026 16:14
|
React Doctor found 1 new issue in 1 file · 1 warning · score 92 / 100 (Great) · 1 fixed · vs 1 warning
Reviewed by React Doctor for commit |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Appearance → Font Size is now a slider in pixels (12–24, default 16) instead of the Small / Medium / Large segmented control. The size previews live while you drag and settles into one write when you let go.
The legacy
fontSizeenum stays in the data model. It is not a type change: turningfontSizeinto a number would make an older build drop the entire synced settings payload on Zod validation, stalling every other setting on that device. Instead a newfontSizePxfield rides alongside, and every write sets both —{ fontSizePx: px, fontSize: toLegacyFontSize(px) }. Existing installs derive their pixel value from whichever bucket they are on (14 / 16 / 20) the first time they read; anyone on the defaultmediumlands on 16.packages/contracts/src/font-size.tsowns the whole thing: the bucket table and two pure functions, so the boundary values live in exactly one place.Cross-version conflict
The two fields carry independent per-device sync clocks, so their clocks cannot be compared against each other to decide which one moved last — the counters have unrelated histories.
resolveFontSizePxuses a value invariant instead. Every build with the slider writes the pair atomically, so a coherent pair always satisfiestoLegacyFontSize(px) === fontSize. A build from before the slider moves only the bucket, and the invariant it breaks by doing so is the signal that the bucket carries the more recent intent. Both directions work: your slider value survives an unrelated settings merge, and an older device's "make it Small" is honored here.Also in here
role="slider"there, not on the Root, and a single-thumb slider gets no generated label, so the name on the Root was not exposed at all. The old segmented control had a working name, so this was a regression.onValueCommit. Radix skips that callback entirely when a drag returns to where it started (which stranded the draft state) and fires it on every keydown (so a held arrow key meant a dozen IPC round trips, config.json rewrites and encrypted settings uploads). Unmount flushes the pending write rather than dropping it.resolveFontSizePxrounds as well as clamps, so the.int()both schemas declare stops being something the runtime can violate.Release note
Font size is now a slider you drag, in pixels, instead of three fixed sizes. Your current size carries over.
Test plan
pnpm typecheck— 19/19pnpm --filter @memry/desktop test:renderer— 8712 passedpnpm --filter @memry/desktop test:main— 7804 passedpnpm --filter @memry/cli test— 37/37pnpm lint— 0 errorspnpm ipc:check,pnpm --filter @memry/desktop i18n:check,pnpm docs:impact --base origin/main --strictsettings-handler.font-size.test.ts, driving realwritePreferences/readPreferencesagainst a temp vault. Both guards were run against the previous mechanism first and observed to fail.Not verified on the real surface. The live drag preview, persist-on-release and held-arrow-key behavior have not been watched in an actual Electron window — the evidence for them is jsdom plus mutation testing.