Conversation
Signed-off-by: samzong <samzong.lu@gmail.com>
|
Hi @samzong, DetailsInstructions for interacting with me using comments are available here. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5dc15b64f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /> | ||
| <Toaster | ||
| theme={theme === 'auto' ? 'system' : theme} | ||
| theme={resolvedTheme} |
There was a problem hiding this comment.
Keep Toaster in system mode for auto theme
Passing resolvedTheme here hard-codes Sonner to 'light'/'dark' even when the user selected auto theme. In this commit, ThemeProvider handles OS theme changes by mutating document.documentElement directly, not by updating React state, so useResolvedTheme() does not necessarily update when the system theme flips at runtime; both toasters can remain on the old palette until an unrelated rerender. Keep using 'system' for auto mode (or make resolved theme stateful on media-query changes) so toast styling tracks OS theme changes immediately.
Useful? React with 👍 / 👎.
Signed-off-by: samzong <samzong.lu@gmail.com>
876b43e to
d13067d
Compare
Summary
Complete the UI contract system by wiring density tokens end-to-end, adding layout tokens, building the density settings UI, and cleaning up dead code. This closes the gap where density switching had no effect on buttons, inputs, or avatars, and where
max-w-3xlwas duplicated across 5 files with no single source of truth.Type of change
[Refactor]internal cleanup[UI]UI or UX changeWhy is this needed?
The
ui-contract-refactorbranch established the theme token system and CI checker, but left density tokens unconnected: buttons, search inputs, form inputs, and chat avatars all used hardcoded Tailwind classes (h-9,h-10,w-8 h-8,max-w-3xl). Switching density mode (compact/comfortable/spacious) had no visible effect on these elements. There was also no user-facing UI to change density, and the setting wasn't persisted across restarts.What changed?
theme.css:--density-control-height(3 tiers),--density-avatar-size— each with compact/comfortable/spacious values--content-max-widthin@themeblock (replaces 5×max-w-3xl)densityfield toAppConfig(main) andAppSettings(preload); ThemeProvider now loads and persists density via independent effectsbutton.tsxsize variants, search inputs, form inputs, chat avatars, content max-width all use tokenshardcoded-content-widthto preventmax-w-3xlregressionAppPanel.tsx,DataList.tsx(0 imports),Settings/components/SettingRow.tsx(unnecessary re-export shim); 3 unused CSS vars (--radius,--shadow-color,--state-disabled-opacity)STAGGER_STEPconstant indesign-tokens.tsArchitecture impact
densityfield added toAppConfig(main/workspace/config.ts) andAppSettings(preload/clawwork.d.ts). IPC handlers are genericPartial<AppConfig>so no handler changes needed.docs/architecture-invariants.md: none — density follows the same persistence pattern as theme (ThemeProvider effect → updateSettings → IPC → config file)setDensity(), neverupdateSettingsdirectlyLinked issues
Part of the
ui-contract-refactorbranch work.Validation
pnpm lintpnpm testpnpm check:ui-contractpnpm check(full pipeline: lint + architecture + ui-contract + renderer-copy + format + typecheck + test)pnpm buildCommands, screenshots, or notes:
```text
pnpm check — all green (89/89 tests pass, 0 UI contract violations)
Test fix: settings-ui-regressions expects 2 updateSettings calls (theme + density)
```
Screenshots or recordings
No screenshots — density UI is a standard SegmentedControl matching existing theme toggle pattern. Comfortable mode values match previous hardcoded values (zero visual regression).
Token changes:
--density-control-height(32/36/40px),--density-control-height-sm(28/32/36px),--density-control-height-lg(36/40/44px),--density-avatar-size(28/32/36px),--content-max-width(48rem). New checker rulehardcoded-content-widthpreventsmax-w-3xlregression.Release note
```release-note
Add display density setting (compact/comfortable/spacious) that adjusts button heights, input sizes, and chat avatar dimensions across the app. Setting persists across restarts.
```
Checklist
[Feat],[Fix],[UI],[Docs],[Refactor],[Build], or[Chore]