refactor(desktop): converge settings sidebar tier and row primitives - #1132
Merged
Conversation
The settings nav rows sat on the 36px xl control tier while the session-list rows sit on the 32px lg tier, so the two sidebars read as different rhythms. Drop .settingsNavItem and .settingsBackButton to --h-control-lg and move the 'settings nav row' example in the token comments accordingly. Also fixes three latent nav-item issues found in the same review: - the always-semibold .settingsNavItem strong rule shadowed the active rule's weight change (dead declaration); weight now lives on the item (medium, semibold when active — same recipe as .maka-list-row-name) - the 24px glyph track + 17px svg override contradicted the 16px icon the TSX renders and offset nav labels 8px from the back button label - leftover grid-template-columns on a flex container
The .settingsRow/.settingsRows primitives (settings-rows.tsx) had their styles scattered across four files — base layout in bot.css, states and hint in connection.css, label/value typography in provider-editor.css, and width variants plus the whole .settingsFormRow/.settingsField/ .settingsFormGrid family in nav-sidebar.css. That let the two row kinds drift: on the 通用 page, .settingsFormRow titles (隐身模式) rendered 15px while adjacent .settingsRow titles (默认模型) rendered 13px, with different padding. Move everything into styles/settings/rows.css as the single style home for the row primitives, and converge the contract: - row titles: one tier (--font-size-heading / medium) for both kinds - row padding: space-5/space-6 for both kinds - hints: one rule (--font-size-base, muted) - field labels (.settingsFormGrid / .settingsField): --font-size-ui, one tier below row titles instead of sharing the heading tier
…ntract The control-height contract pinned .settingsNavItem/.settingsBackButton to --h-control-xl; they now sit on --h-control-lg alongside the session-list rows.
The consolidated settings/rows.css carries the reduced-motion transition override that moved out of nav-sidebar.css; its only !important site is that a11y block.
…le-file skip rows.css is the style home for every Settings row primitive; the isA11yOnlyFile whole-file skip would blind the audit to any future non-a11y !important added there. Route it through the stricter allowlist + in-file Justified: path instead (codex review P2).
Nothing guarded the rows.css convergence: .settingsRow titles could drift back to 13px/space-4 padding silently. Pin the shared padding, title/field/hint tiers against the comma-grouped rules so both a value drift and a fork back into per-kind sibling rules fail (codex review P2). Verified the pin bites: reverting padding to space-4/space-5 fails the test.
The .settingsNavItem fixture still said xl after the mapping moved to lg; it only exercises missing-prop detection so nothing was weakened, but the mismatch misleads (codex review P3).
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
Two governance fixes for the Settings surface, from a review of the sidebar and the 通用 page:
.settingsNavItem+.settingsBackButton), the same tier as the session-list rows, so the two sidebars share one row rhythm. Along the way this fixes three latent nav-item issues: the always-semiboldstrongrule that shadowed the active weight bump (now medium → semibold on active, matching.maka-list-row-name), the 24px glyph track + 17px svg override that contradicted the 16px icon the TSX renders and offset nav labels 8px from the back-button label, and a leftovergrid-template-columnson a flex container.styles/settings/rows.css..settingsRow/.settingsRows(rendered bysettings-rows.tsx) had styles scattered across bot.css, connection.css, provider-editor.css, and nav-sidebar.css, which let the two row kinds drift — on the 通用 page,.settingsFormRowtitles (隐身模式) rendered 15px while adjacent.settingsRowtitles (默认模型) rendered 13px with different padding. One file now owns the row/field family with one typography contract: row titles--font-size-heading/medium, field labels--font-size-ui/medium (one tier below, instead of sharing the heading tier), hints--font-size-base, row padding space-5/space-6 for both kinds.Both contract tests that pin these invariants are updated at the same seam: the control-height contract moves the two settings nav selectors to the lg tier, and the
!importantaudit registers rows.css as an a11y-only file (its sole!importantis theprefers-reduced-motionblock that moved out of nav-sidebar.css).CSS + contract tests only; no TSX changes.
Before / after
Captured from the live app (screenshot harness,
settings-general, light 1280) at the merge-base vs this branch, composed with ImageMagick:Left: 36px sidebar rows; 默认模型/默认权限模式 card renders 13px titles with tighter padding than the 15px cards above it. Right: 32px sidebar rows matching the session list; all row titles on one 15px tier with uniform padding. (The evidence image is committed on this branch only for the pinned raw URL and is removed again before merge, following the #901/#939 + #1094 convention.)
Verification
vite build(renderer) passes — validates the new@importgraph and CSS syntax.control-height-converge-contract+renderer-important-audit-contractpass locally (11/11) after the paired updates; these were the two CItestfailures on the first push.capture-screenshots.mjs --scenario settings-generaland--scenario settings-about(light, 1280/990), 4/4 scenarios OK each. settings-general shows the tightened 32px sidebar rows and matching 15px titles across the personalization/toggle cards and the 默认模型/默认权限模式 card; settings-about confirms.settingsRowdisplay rows (label + right-aligned value, mono path variant) render correctly with the converged typography.