refactor: reduce code duplication in font settings and sync logic

Address CodeRabbit review feedback:
- Create getEffectiveFont helper to deduplicate getEffectiveFontSans/Mono
- Extract getSettingsFieldValue and hasSettingsFieldChanged helpers
- Create reusable FontSelector component for font selection UI
- Refactor project-theme-section and appearance-section to use FontSelector
This commit is contained in:
Stefan de Vogelaere
2026-01-17 19:30:00 +01:00
parent a01f299597
commit ded5ecf4e9
6 changed files with 150 additions and 165 deletions

View File

@@ -0,0 +1,47 @@
import {
Select,
SelectContent,
SelectItem,
SelectTrigger,
SelectValue,
} from '@/components/ui/select';
import { DEFAULT_FONT_VALUE } from '@/config/ui-font-options';
interface FontOption {
value: string;
label: string;
}
interface FontSelectorProps {
id: string;
value: string;
options: readonly FontOption[];
placeholder: string;
onChange: (value: string) => void;
}
/**
* Reusable font selector component with live preview styling
*/
export function FontSelector({ id, value, options, placeholder, onChange }: FontSelectorProps) {
return (
<Select value={value} onValueChange={onChange}>
<SelectTrigger id={id} className="w-full">
<SelectValue placeholder={placeholder} />
</SelectTrigger>
<SelectContent>
{options.map((option) => (
<SelectItem key={option.value} value={option.value}>
<span
style={{
fontFamily: option.value === DEFAULT_FONT_VALUE ? undefined : option.value,
}}
>
{option.label}
</span>
</SelectItem>
))}
</SelectContent>
</Select>
);
}

View File

@@ -5,3 +5,6 @@ export {
type UseModelOverrideOptions,
type UseModelOverrideResult,
} from './use-model-override';
// Font Components
export { FontSelector } from './font-selector';

View File

@@ -1,13 +1,6 @@
import { useState, useEffect } from 'react';
import { Label } from '@/components/ui/label';
import { Checkbox } from '@/components/ui/checkbox';
import {
Select,
SelectContent,
SelectItem,
SelectTrigger,
SelectValue,
} from '@/components/ui/select';
import { Palette, Moon, Sun, Type } from 'lucide-react';
import { darkThemes, lightThemes, type Theme } from '@/config/theme-options';
import {
@@ -17,6 +10,7 @@ import {
} from '@/config/ui-font-options';
import { cn } from '@/lib/utils';
import { useAppStore } from '@/store/app-store';
import { FontSelector } from '@/components/shared';
import type { Project } from '@/lib/electron';
interface ProjectThemeSectionProps {
@@ -305,25 +299,13 @@ export function ProjectThemeSection({ project }: ProjectThemeSectionProps) {
<Label htmlFor="ui-font-select" className="text-sm">
Project UI Font
</Label>
<Select value={fontSansLocal} onValueChange={handleFontSansChange}>
<SelectTrigger id="ui-font-select" className="w-full">
<SelectValue placeholder="Default (Geist Sans)" />
</SelectTrigger>
<SelectContent>
{UI_SANS_FONT_OPTIONS.map((option) => (
<SelectItem key={option.value} value={option.value}>
<span
style={{
fontFamily:
option.value === DEFAULT_FONT_VALUE ? undefined : option.value,
}}
>
{option.label}
</span>
</SelectItem>
))}
</SelectContent>
</Select>
<FontSelector
id="ui-font-select"
value={fontSansLocal}
options={UI_SANS_FONT_OPTIONS}
placeholder="Default (Geist Sans)"
onChange={handleFontSansChange}
/>
</div>
)}
</div>
@@ -358,25 +340,13 @@ export function ProjectThemeSection({ project }: ProjectThemeSectionProps) {
<Label htmlFor="code-font-select" className="text-sm">
Project Code Font
</Label>
<Select value={fontMonoLocal} onValueChange={handleFontMonoChange}>
<SelectTrigger id="code-font-select" className="w-full">
<SelectValue placeholder="Default (Geist Mono)" />
</SelectTrigger>
<SelectContent>
{UI_MONO_FONT_OPTIONS.map((option) => (
<SelectItem key={option.value} value={option.value}>
<span
style={{
fontFamily:
option.value === DEFAULT_FONT_VALUE ? undefined : option.value,
}}
>
{option.label}
</span>
</SelectItem>
))}
</SelectContent>
</Select>
<FontSelector
id="code-font-select"
value={fontMonoLocal}
options={UI_MONO_FONT_OPTIONS}
placeholder="Default (Geist Mono)"
onChange={handleFontMonoChange}
/>
</div>
)}
</div>

View File

@@ -1,12 +1,5 @@
import { useState, useEffect } from 'react';
import { Label } from '@/components/ui/label';
import {
Select,
SelectContent,
SelectItem,
SelectTrigger,
SelectValue,
} from '@/components/ui/select';
import { Palette, Moon, Sun, Type } from 'lucide-react';
import { darkThemes, lightThemes } from '@/config/theme-options';
import {
@@ -16,6 +9,7 @@ import {
} from '@/config/ui-font-options';
import { cn } from '@/lib/utils';
import { useAppStore } from '@/store/app-store';
import { FontSelector } from '@/components/shared';
import type { Theme } from '../shared/types';
interface AppearanceSectionProps {
@@ -165,25 +159,13 @@ export function AppearanceSection({ effectiveTheme, onThemeChange }: AppearanceS
<Label htmlFor="global-ui-font-select" className="text-sm">
UI Font
</Label>
<Select value={fontSansValue} onValueChange={handleFontSansChange}>
<SelectTrigger id="global-ui-font-select" className="w-full">
<SelectValue placeholder="Default (Geist Sans)" />
</SelectTrigger>
<SelectContent>
{UI_SANS_FONT_OPTIONS.map((option) => (
<SelectItem key={option.value} value={option.value}>
<span
style={{
fontFamily:
option.value === DEFAULT_FONT_VALUE ? undefined : option.value,
}}
>
{option.label}
</span>
</SelectItem>
))}
</SelectContent>
</Select>
<FontSelector
id="global-ui-font-select"
value={fontSansValue}
options={UI_SANS_FONT_OPTIONS}
placeholder="Default (Geist Sans)"
onChange={handleFontSansChange}
/>
<p className="text-xs text-muted-foreground">
Used for headings, labels, and UI text
</p>
@@ -194,25 +176,13 @@ export function AppearanceSection({ effectiveTheme, onThemeChange }: AppearanceS
<Label htmlFor="global-code-font-select" className="text-sm">
Code Font
</Label>
<Select value={fontMonoValue} onValueChange={handleFontMonoChange}>
<SelectTrigger id="global-code-font-select" className="w-full">
<SelectValue placeholder="Default (Geist Mono)" />
</SelectTrigger>
<SelectContent>
{UI_MONO_FONT_OPTIONS.map((option) => (
<SelectItem key={option.value} value={option.value}>
<span
style={{
fontFamily:
option.value === DEFAULT_FONT_VALUE ? undefined : option.value,
}}
>
{option.label}
</span>
</SelectItem>
))}
</SelectContent>
</Select>
<FontSelector
id="global-code-font-select"
value={fontMonoValue}
options={UI_MONO_FONT_OPTIONS}
placeholder="Default (Geist Mono)"
onChange={handleFontMonoChange}
/>
<p className="text-xs text-muted-foreground">
Used for code blocks and monospaced text
</p>

View File

@@ -79,6 +79,41 @@ const SETTINGS_FIELDS_TO_SYNC = [
// Fields from setup store to sync
const SETUP_FIELDS_TO_SYNC = ['isFirstRun', 'setupComplete', 'skipClaudeSetup'] as const;
/**
* Helper to extract a settings field value from app state
* Handles special cases for nested/mapped fields
*/
function getSettingsFieldValue(
field: (typeof SETTINGS_FIELDS_TO_SYNC)[number],
appState: ReturnType<typeof useAppStore.getState>
): unknown {
if (field === 'currentProjectId') {
return appState.currentProject?.id ?? null;
}
if (field === 'terminalFontFamily') {
return appState.terminalState.fontFamily;
}
return appState[field as keyof typeof appState];
}
/**
* Helper to check if a settings field changed between states
*/
function hasSettingsFieldChanged(
field: (typeof SETTINGS_FIELDS_TO_SYNC)[number],
newState: ReturnType<typeof useAppStore.getState>,
prevState: ReturnType<typeof useAppStore.getState>
): boolean {
if (field === 'currentProjectId') {
return newState.currentProject?.id !== prevState.currentProject?.id;
}
if (field === 'terminalFontFamily') {
return newState.terminalState.fontFamily !== prevState.terminalState.fontFamily;
}
const key = field as keyof typeof newState;
return newState[key] !== prevState[key];
}
interface SettingsSyncState {
/** Whether initial settings have been loaded from API */
loaded: boolean;
@@ -157,15 +192,7 @@ export function useSettingsSync(): SettingsSyncState {
// Build updates object from current state
const updates: Record<string, unknown> = {};
for (const field of SETTINGS_FIELDS_TO_SYNC) {
if (field === 'currentProjectId') {
// Special handling: extract ID from currentProject object
updates[field] = appState.currentProject?.id ?? null;
} else if (field === 'terminalFontFamily') {
// Special handling: map terminalState.fontFamily to terminalFontFamily
updates[field] = appState.terminalState.fontFamily;
} else {
updates[field] = appState[field as keyof typeof appState];
}
updates[field] = getSettingsFieldValue(field, appState);
}
// Include setup wizard state (lives in a separate store)
@@ -262,13 +289,7 @@ export function useSettingsSync(): SettingsSyncState {
// (migration has already hydrated the store from server/localStorage)
const updates: Record<string, unknown> = {};
for (const field of SETTINGS_FIELDS_TO_SYNC) {
if (field === 'currentProjectId') {
updates[field] = appState.currentProject?.id ?? null;
} else if (field === 'terminalFontFamily') {
updates[field] = appState.terminalState.fontFamily;
} else {
updates[field] = appState[field as keyof typeof appState];
}
updates[field] = getSettingsFieldValue(field, appState);
}
for (const field of SETUP_FIELDS_TO_SYNC) {
updates[field] = setupState[field as keyof typeof setupState];
@@ -322,24 +343,9 @@ export function useSettingsSync(): SettingsSyncState {
// Check if any synced field changed
let changed = false;
for (const field of SETTINGS_FIELDS_TO_SYNC) {
if (field === 'currentProjectId') {
// Special handling: compare currentProject IDs
if (newState.currentProject?.id !== prevState.currentProject?.id) {
changed = true;
break;
}
} else if (field === 'terminalFontFamily') {
// Special handling: compare terminalState.fontFamily
if (newState.terminalState.fontFamily !== prevState.terminalState.fontFamily) {
changed = true;
break;
}
} else {
const key = field as keyof typeof newState;
if (newState[key] !== prevState[key]) {
changed = true;
break;
}
if (hasSettingsFieldChanged(field, newState, prevState)) {
changed = true;
break;
}
}
@@ -413,13 +419,7 @@ export async function forceSyncSettingsToServer(): Promise<boolean> {
const updates: Record<string, unknown> = {};
for (const field of SETTINGS_FIELDS_TO_SYNC) {
if (field === 'currentProjectId') {
updates[field] = appState.currentProject?.id ?? null;
} else if (field === 'terminalFontFamily') {
updates[field] = appState.terminalState.fontFamily;
} else {
updates[field] = appState[field as keyof typeof appState];
}
updates[field] = getSettingsFieldValue(field, appState);
}
const setupState = useSetupStore.getState();
for (const field of SETUP_FIELDS_TO_SYNC) {

View File

@@ -149,6 +149,31 @@ export function getStoredTheme(): ThemeMode | null {
return null;
}
/**
* Helper to get effective font value with validation
* Returns the font to use (project override -> global -> null for default)
* @param projectFont - The project-specific font override
* @param globalFont - The global font setting
* @param fontOptions - The list of valid font options for validation
*/
function getEffectiveFont(
projectFont: string | undefined,
globalFont: string | null,
fontOptions: readonly { value: string; label: string }[]
): string | null {
const isValidFont = (font: string | null | undefined): boolean => {
if (!font || font === DEFAULT_FONT_VALUE) return true;
return fontOptions.some((opt) => opt.value === font);
};
if (projectFont) {
if (!isValidFont(projectFont)) return null; // Fallback to default if font not in list
return projectFont === DEFAULT_FONT_VALUE ? null : projectFont;
}
if (!isValidFont(globalFont)) return null; // Fallback to default if font not in list
return globalFont === DEFAULT_FONT_VALUE ? null : globalFont;
}
/**
* Save theme to localStorage for immediate persistence
* This is used as a fallback when server settings can't be loaded
@@ -1873,43 +1898,13 @@ export const useAppStore = create<AppState & AppActions>()((set, get) => ({
},
getEffectiveFontSans: () => {
const currentProject = get().currentProject;
// Return project override if set, otherwise global, otherwise null for default
// 'default' value means explicitly using default font, so return null for CSS
// Also validate that the font is in the available options list
const isValidFont = (font: string | null | undefined): boolean => {
if (!font || font === DEFAULT_FONT_VALUE) return true;
return UI_SANS_FONT_OPTIONS.some((opt) => opt.value === font);
};
if (currentProject?.fontFamilySans) {
const font = currentProject.fontFamilySans;
if (!isValidFont(font)) return null; // Fallback to default if font not in list
return font === DEFAULT_FONT_VALUE ? null : font;
}
const globalFont = get().fontFamilySans;
if (!isValidFont(globalFont)) return null; // Fallback to default if font not in list
return globalFont === DEFAULT_FONT_VALUE ? null : globalFont;
const { currentProject, fontFamilySans } = get();
return getEffectiveFont(currentProject?.fontFamilySans, fontFamilySans, UI_SANS_FONT_OPTIONS);
},
getEffectiveFontMono: () => {
const currentProject = get().currentProject;
// Return project override if set, otherwise global, otherwise null for default
// 'default' value means explicitly using default font, so return null for CSS
// Also validate that the font is in the available options list
const isValidFont = (font: string | null | undefined): boolean => {
if (!font || font === DEFAULT_FONT_VALUE) return true;
return UI_MONO_FONT_OPTIONS.some((opt) => opt.value === font);
};
if (currentProject?.fontFamilyMono) {
const font = currentProject.fontFamilyMono;
if (!isValidFont(font)) return null; // Fallback to default if font not in list
return font === DEFAULT_FONT_VALUE ? null : font;
}
const globalFont = get().fontFamilyMono;
if (!isValidFont(globalFont)) return null; // Fallback to default if font not in list
return globalFont === DEFAULT_FONT_VALUE ? null : globalFont;
const { currentProject, fontFamilyMono } = get();
return getEffectiveFont(currentProject?.fontFamilyMono, fontFamilyMono, UI_MONO_FONT_OPTIONS);
},
// Feature actions