feat(settings): dedicated Appearance tab + global font selection - #170
Conversation
Add a persisted, system-safe global font selection. A new `font` pref in the prefs slice overrides the root `--font-sans` CSS variable (the whole UI uses `font-family: var(--font-sans)`), so the choice applies app-wide; `default` removes the override and falls back to the :root Inter stack. - prefsSlice: FontId type, FONT_OPTIONS/FONT_STACKS tables, font + setFont - store/index: persist `font` via partialize; re-export font tables/type - App.jsx: re-apply persisted font on launch in the rehydrate effect - AppearancePanel: Font row (Select) next to UI-scale and color-theme - AppearancePanel.test: covers render, selection, and default reset All stacks are system fonts (no web-font downloads) so behavior is identical offline across macOS/Windows/Linux. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Promote the Appearance panel out of the Credentials tab into a dedicated
top-level tab (with the Palette icon), placed before Credentials. Add the
English "appearance" label so the tab renders via t(`settings.${id}`).
Remove the AppearancePanel render (and its stale comment) from
CredentialsTab.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds a global font preference feature allowing users to select and persist a font choice across sessions. It defines font types and CSS stacks in the store, applies the selected font via CSS variable during app bootstrap, surfaces font selection in a new Appearance settings tab, and includes tests validating the selection and CSS override behavior. ChangesGlobal Font Preference Feature
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsUI as Settings / AppearancePanel
participant Store as Zustand Store
participant DOM as DOM CSS
User->>SettingsUI: Select font from dropdown
SettingsUI->>Store: Call setFont(newFont)
Store->>DOM: Update --font-sans CSS variable
Store->>Store: Persist font to localStorage
SettingsUI->>SettingsUI: Re-render with updated font value
Note over Store,DOM: On app rehydration
Store->>Store: Load persisted font from localStorage
DOM->>DOM: Apply --font-sans override at bootstrap
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly Related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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. Comment |
|
| Filename | Overview |
|---|---|
| frontend/src/store/prefsSlice.ts | Adds FontId type, FONT_OPTIONS, FONT_STACKS constants, and font/setFont to the prefs slice; the "rounded" stack silently degrades to system-ui on Windows/Linux where SF Pro Rounded, Nunito, and Quicksand are all absent. |
| frontend/src/store/index.ts | Re-exports FontId, FONT_OPTIONS, FONT_STACKS and adds font to the persist partialize list; export statements are interleaved with imports (style issue only). |
| frontend/src/components/settings/AppearancePanel.jsx | Adds the font Select row wired to the store; the new "Font" label is a hardcoded string, violating the project's i18n hard rule (existing labels in this panel share the same pre-existing gap). |
| frontend/src/App.jsx | Subscribes to the font preference and re-applies --font-sans on rehydration; mirrors the existing theme/locale hydration pattern correctly. |
| frontend/src/pages/Settings.jsx | Adds the Appearance tab to TAB_DEFS and renders AppearancePanel for it; removes AppearancePanel from inside CredentialsTab cleanly. |
| frontend/src/components/settings/AppearancePanel.test.jsx | New test file with three focused cases: options render, non-default font sets --font-sans, reverting to default removes it. Coverage looks solid. |
| frontend/src/i18n/locales/en.json | Adds the "appearance" tab label key; omits i18n keys for the panel's internal labels ("Font", etc.) which are hardcoded in the component. |
Sequence Diagram
sequenceDiagram
participant LS as localStorage
participant ZS as Zustand Store
participant AJ as App.jsx (useEffect)
participant DOM as document.documentElement
participant AP as AppearancePanel
Note over LS,DOM: App launch / rehydration
LS->>ZS: persist middleware rehydrates font (or default)
ZS->>AJ: font state change triggers effect
AJ->>DOM: setProperty(--font-sans, stack) or removeProperty
Note over AP,DOM: User changes font
AP->>ZS: setFont(id)
ZS->>ZS: "set({ font: id })"
ZS->>DOM: setProperty(--font-sans, stack) or removeProperty
ZS->>LS: persist middleware writes font to localStorage
Reviews (1): Last reviewed commit: "feat(settings): move Appearance into its..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| </div> | ||
|
|
||
| <div className="appearance-panel__row"> | ||
| <span className="appearance-panel__label">Font</span> |
There was a problem hiding this comment.
CLAUDE.md states: "All UI strings go through i18n (t('...') keys in locales/*.json)". The new "Font" label is a hardcoded English string — it should be a t('appearance.font') call backed by a key in en.json. The existing "UI scale", "Color theme", and "Appearance" labels on this same panel are also hardcoded, suggesting this is a pre-existing gap rather than something unique to this PR, but the new string still introduces another violation. The en.json hunk only adds "appearance" for the tab label and omits a font key entirely.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| system: '-apple-system, BlinkMacSystemFont, "Segoe UI", system-ui, sans-serif', | ||
| serif: 'Georgia, "Times New Roman", serif', | ||
| mono: 'ui-monospace, "SF Mono", "Cascadia Code", Menlo, Consolas, monospace', | ||
| rounded: '"SF Pro Rounded", "Nunito", "Quicksand", system-ui, sans-serif', |
There was a problem hiding this comment.
The rounded stack is "SF Pro Rounded", "Nunito", "Quicksand", system-ui, sans-serif. SF Pro Rounded is macOS/iOS only. Nunito and Quicksand are Google web fonts — they won't be present on a fresh Windows or Linux install without a prior web-font download. The effective fallback on most non-Apple machines is system-ui, sans-serif, which has no rounded appearance. A user who selects "Rounded" on Windows will see no visual difference from the default Inter stack, making the option silently confusing. Adding a parenthetical like "Rounded (macOS)" in the label, or replacing the font-family entry with a more widely available rounded font (e.g. "Century Gothic" which ships on Windows), would make the cross-platform behaviour predictable.
| import type { PrefsSlice, FontId } from './prefsSlice'; | ||
| import { createPrefsSlice, FONT_OPTIONS, FONT_STACKS } from './prefsSlice'; | ||
|
|
||
| // Re-export font preference tables so panels can import from the store root. | ||
| export type { FontId }; | ||
| export { FONT_OPTIONS, FONT_STACKS }; | ||
| import type { GlossarySlice } from './glossarySlice'; |
There was a problem hiding this comment.
| import type { PrefsSlice, FontId } from './prefsSlice'; | |
| import { createPrefsSlice, FONT_OPTIONS, FONT_STACKS } from './prefsSlice'; | |
| // Re-export font preference tables so panels can import from the store root. | |
| export type { FontId }; | |
| export { FONT_OPTIONS, FONT_STACKS }; | |
| import type { GlossarySlice } from './glossarySlice'; | |
| import type { PrefsSlice, FontId } from './prefsSlice'; | |
| import { createPrefsSlice, FONT_OPTIONS, FONT_STACKS } from './prefsSlice'; | |
| import type { GlossarySlice } from './glossarySlice'; | |
| // Re-export font preference tables so panels can import from the store root. | |
| export type { FontId }; | |
| export { FONT_OPTIONS, FONT_STACKS }; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/settings/AppearancePanel.jsx`:
- Around line 72-79: The "Font" label and aria-label on the Select in
AppearancePanel.jsx are hardcoded; replace them with i18n lookups (e.g. use
t('appearance.font') for both the visible label and aria-label) and ensure the
component uses the existing translation hook (e.g. the t function already used
elsewhere in this component) and that corresponding keys are added to locales
JSON (appearance.font). Update the span text and the Select aria-label to call
t(...) rather than the literal "Font".
In `@frontend/src/store/prefsSlice.ts`:
- Around line 23-30: FONT_OPTIONS currently embeds hardcoded English labels
which prevents localization; replace the label strings with i18n keys and update
consumers to call the translation function at render time. Specifically, change
FONT_OPTIONS (and any type FontId if needed) to store translation keys like
'prefs.fonts.inter', 'prefs.fonts.system', etc., then in the settings UI
component(s) that consume FONT_OPTIONS call the i18n t(...) function (e.g.,
t(option.labelKey)) instead of using the raw label; ensure locales/*.json
include those keys and update any tests that assert exact label text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1fadba09-a3d2-44d7-a717-a9110cb9f47b
📒 Files selected for processing (7)
frontend/src/App.jsxfrontend/src/components/settings/AppearancePanel.jsxfrontend/src/components/settings/AppearancePanel.test.jsxfrontend/src/i18n/locales/en.jsonfrontend/src/pages/Settings.jsxfrontend/src/store/index.tsfrontend/src/store/prefsSlice.ts
| <span className="appearance-panel__label">Font</span> | ||
| <Select | ||
| size="xs" | ||
| value={font} | ||
| onChange={(e) => setFont(e.target.value)} | ||
| data-testid="appearance-font-select" | ||
| aria-label="Font" | ||
| > |
There was a problem hiding this comment.
Localize the new font field label/aria text via t(...).
The newly added font control introduces hardcoded user-facing text ("Font" and aria-label="Font"). Please route these through i18n keys.
Suggested change
+import { useTranslation } from 'react-i18next';
export default function AppearancePanel() {
+ const { t } = useTranslation();
// ...
- <span className="appearance-panel__label">Font</span>
+ <span className="appearance-panel__label">{t('appearance.font_label')}</span>
<Select
size="xs"
value={font}
onChange={(e) => setFont(e.target.value)}
data-testid="appearance-font-select"
- aria-label="Font"
+ aria-label={t('appearance.font_label')}
>As per coding guidelines, "All user-facing text in the UI must go through the i18n translation layer using t('...') keys in locales/*.json files, never hardcode non-English text".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <span className="appearance-panel__label">Font</span> | |
| <Select | |
| size="xs" | |
| value={font} | |
| onChange={(e) => setFont(e.target.value)} | |
| data-testid="appearance-font-select" | |
| aria-label="Font" | |
| > | |
| <span className="appearance-panel__label">{t('appearance.font_label')}</span> | |
| <Select | |
| size="xs" | |
| value={font} | |
| onChange={(e) => setFont(e.target.value)} | |
| data-testid="appearance-font-select" | |
| aria-label={t('appearance.font_label')} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/settings/AppearancePanel.jsx` around lines 72 - 79,
The "Font" label and aria-label on the Select in AppearancePanel.jsx are
hardcoded; replace them with i18n lookups (e.g. use t('appearance.font') for
both the visible label and aria-label) and ensure the component uses the
existing translation hook (e.g. the t function already used elsewhere in this
component) and that corresponding keys are added to locales JSON
(appearance.font). Update the span text and the Select aria-label to call t(...)
rather than the literal "Font".
| export const FONT_OPTIONS: { id: FontId; label: string }[] = [ | ||
| { id: 'default', label: 'Inter (default)' }, | ||
| { id: 'system', label: 'System' }, | ||
| { id: 'serif', label: 'Serif' }, | ||
| { id: 'mono', label: 'Monospace' }, | ||
| { id: 'rounded', label: 'Rounded' }, | ||
| { id: 'readable', label: 'Readable' }, | ||
| ]; |
There was a problem hiding this comment.
Move font option labels to i18n keys instead of hardcoded strings.
FONT_OPTIONS currently stores English UI labels directly. Since these labels are rendered in the settings UI, this blocks localization for the new font selector.
Suggested refactor
-export const FONT_OPTIONS: { id: FontId; label: string }[] = [
- { id: 'default', label: 'Inter (default)' },
- { id: 'system', label: 'System' },
- { id: 'serif', label: 'Serif' },
- { id: 'mono', label: 'Monospace' },
- { id: 'rounded', label: 'Rounded' },
- { id: 'readable', label: 'Readable' },
+export const FONT_OPTIONS: { id: FontId; labelKey: string }[] = [
+ { id: 'default', labelKey: 'appearance.font.default' },
+ { id: 'system', labelKey: 'appearance.font.system' },
+ { id: 'serif', labelKey: 'appearance.font.serif' },
+ { id: 'mono', labelKey: 'appearance.font.mono' },
+ { id: 'rounded', labelKey: 'appearance.font.rounded' },
+ { id: 'readable', labelKey: 'appearance.font.readable' },
];As per coding guidelines, "All user-facing text in the UI must go through the i18n translation layer using t('...') keys in locales/*.json files, never hardcode non-English text".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/store/prefsSlice.ts` around lines 23 - 30, FONT_OPTIONS
currently embeds hardcoded English labels which prevents localization; replace
the label strings with i18n keys and update consumers to call the translation
function at render time. Specifically, change FONT_OPTIONS (and any type FontId
if needed) to store translation keys like 'prefs.fonts.inter',
'prefs.fonts.system', etc., then in the settings UI component(s) that consume
FONT_OPTIONS call the i18n t(...) function (e.g., t(option.labelKey)) instead of
using the raw label; ensure locales/*.json include those keys and update any
tests that assert exact label text.
1 participant
Two requested Settings changes:
Paletteicon).--font-sansCSS variable on the root (the same var the whole UI'sfont-familyalready uses — mirrors howsetThemesetsdata-theme). Persisted via zustand + re-applied on launch inApp.jsx's rehydrate effect.No web-font downloads (all stacks fall back to system fonts). Verified: typecheck:ci ✓, test:legacy 36 ✓, vitest 94/94 (incl. new AppearancePanel test) ✓, build ✓, CJK guard ✓.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests