feat: add font size and word wrap settings to editor preferences#721
feat: add font size and word wrap settings to editor preferences#721Shubh-Raj wants to merge 7 commits intoaccordproject:mainfrom
Conversation
- Add editorFontSize (dropdown, 12-20px) and editorWordWrap (toggle) to the Settings modal - Both settings persist to localStorage - Apply to all three Monaco editors (Markdown, Concerto, JSON) - Add store tests for both new settings - Update SettingsModal tests for new UI elements Closes accordproject#720 Signed-off-by: Shubh-Raj <shubhraj625@gmail.com>
✅ Deploy Preview for ap-template-playground ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR adds customizable font size and word wrap settings to the Monaco code editors in the playground, addressing issue #720. The implementation follows established patterns in the codebase, using Zustand store for state management and localStorage for persistence, mirroring the existing showLineNumbers feature.
Changes:
- Added
editorFontSize(number, default 14) andeditorWordWrap(boolean, default true) to the Zustand store with localStorage persistence - Enhanced Settings modal with a Font Size dropdown (12-20px) and Word Wrap toggle switch
- Updated all three editors (MarkdownEditor, ConcertoEditor, JSONEditor) to consume and apply the new settings
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/store/store.ts | Added state properties, setter functions, and localStorage helper functions for editorFontSize and editorWordWrap |
| src/components/SettingsModal.tsx | Added Select dropdown for font size and Switch toggle for word wrap with consistent UI styling |
| src/editors/MarkdownEditor.tsx | Integrated fontSize and editorWordWrap from store into Monaco editor options |
| src/editors/JSONEditor.tsx | Integrated fontSize and editorWordWrap from store into Monaco editor options |
| src/editors/ConcertoEditor.tsx | Integrated fontSize and editorWordWrap from store into Monaco editor options |
| src/tests/store/editorFontSize.test.tsx | Added comprehensive unit tests for editorFontSize state management |
| src/tests/store/editorWordWrap.test.tsx | Added comprehensive unit tests for editorWordWrap state management |
| src/tests/components/SettingsModal.test.tsx | Added tests for new UI elements and updated divider count assertion |
| expect(screen.getByText('Font Size')).toBeInTheDocument(); | ||
| expect(screen.getByText('Adjust font size in code editors')).toBeInTheDocument(); | ||
| }); | ||
|
|
There was a problem hiding this comment.
The test for Font Size setting only verifies that the text labels are rendered, but doesn't verify that the Select component itself is rendered or that it displays the correct initial value (14). Consider adding a test similar to the word wrap toggle tests that verifies the Select component is present and shows the correct value from the store.
| it('renders the Font Size select with the correct initial value', () => { | |
| render(<SettingsModal />); | |
| const fontSizeSelect = screen.getByRole('combobox', { name: /font size/i }); | |
| expect(fontSizeSelect).toBeInTheDocument(); | |
| expect(fontSizeSelect).toHaveValue('14'); | |
| }); |
| const getInitialFontSize = () => { | ||
| if (typeof window !== 'undefined') { | ||
| const saved = localStorage.getItem('editorFontSize'); | ||
| if (saved !== null) { | ||
| const parsed = parseInt(saved, 10); | ||
| if (!isNaN(parsed)) return parsed; | ||
| } | ||
| } | ||
| return 14; // Default font size | ||
| }; |
There was a problem hiding this comment.
The getInitialFontSize function doesn't validate that the parsed font size is within the allowed range (12-20). If localStorage contains an invalid value like 5 or 100, it will be accepted. Consider adding validation to ensure the parsed value is within FONT_SIZE_OPTIONS or at least within a reasonable range before returning it.
src/store/store.ts
Outdated
| if (typeof window !== 'undefined') { | ||
| localStorage.setItem('editorFontSize', String(value)); | ||
| } | ||
| set({ editorFontSize: value }); |
There was a problem hiding this comment.
The setEditorFontSize function doesn't validate that the value is within the allowed FONT_SIZE_OPTIONS array (12, 13, 14, 15, 16, 18, 20). This could allow invalid font sizes to be set programmatically or through manual localStorage modification. Consider adding validation to ensure only valid font sizes from FONT_SIZE_OPTIONS can be set, or at least validate it's a positive number within a reasonable range.
| if (typeof window !== 'undefined') { | |
| localStorage.setItem('editorFontSize', String(value)); | |
| } | |
| set({ editorFontSize: value }); | |
| // Validate and normalize the incoming font size to prevent invalid values | |
| const safeValue = Math.round(value); | |
| const MIN_FONT_SIZE = 8; | |
| const MAX_FONT_SIZE = 72; | |
| if (!Number.isFinite(safeValue) || safeValue < MIN_FONT_SIZE || safeValue > MAX_FONT_SIZE) { | |
| // Ignore invalid values rather than persisting them | |
| return; | |
| } | |
| if (typeof window !== 'undefined') { | |
| localStorage.setItem('editorFontSize', String(safeValue)); | |
| } | |
| set({ editorFontSize: safeValue }); |
src/components/SettingsModal.tsx
Outdated
| import DarkModeToggle from 'react-dark-mode-toggle'; | ||
| import useAppStore from '../store/store'; | ||
|
|
||
| const FONT_SIZE_OPTIONS = [12, 13, 14, 15, 16, 18, 20]; |
There was a problem hiding this comment.
The FONT_SIZE_OPTIONS constant is defined in SettingsModal.tsx but not accessible from the store file. For better maintainability and potential validation, consider either: 1) defining FONT_SIZE_OPTIONS in a shared constants file, or 2) exporting it from SettingsModal.tsx so it can be imported in store.ts if validation is added later. This would create a single source of truth for the valid font size values.
| const FONT_SIZE_OPTIONS = [12, 13, 14, 15, 16, 18, 20]; | |
| export const FONT_SIZE_OPTIONS = [12, 13, 14, 15, 16, 18, 20]; |
src/editors/JSONEditor.tsx
Outdated
| const options: monaco.editor.IStandaloneEditorConstructionOptions = useMemo(() => ({ | ||
| minimap: { enabled: false }, | ||
| wordWrap: "on", | ||
| wordWrap: editorWordWrap ? "on" : "off", |
There was a problem hiding this comment.
The wordWrap property uses different type assertions across editors. In MarkdownEditor, it uses 'as const' for type narrowing ('on' as const : 'off' as const), while JSONEditor and ConcertoEditor use plain string literals without const assertions. For consistency, all three editors should use the same pattern. The 'as const' approach is more type-safe and explicit.
| wordWrap: editorWordWrap ? "on" : "off", | |
| wordWrap: editorWordWrap ? ("on" as const) : ("off" as const), |
src/editors/ConcertoEditor.tsx
Outdated
| const options: monaco.editor.IStandaloneEditorConstructionOptions = useMemo(() => ({ | ||
| minimap: { enabled: false }, | ||
| wordWrap: "on", | ||
| wordWrap: editorWordWrap ? "on" : "off", |
There was a problem hiding this comment.
The wordWrap property uses different type assertions across editors. In MarkdownEditor, it uses 'as const' for type narrowing ('on' as const : 'off' as const), while JSONEditor and ConcertoEditor use plain string literals without const assertions. For consistency, all three editors should use the same pattern. The 'as const' approach is more type-safe and explicit.
| wordWrap: editorWordWrap ? "on" : "off", | |
| wordWrap: editorWordWrap ? ("on" as const) : ("off" as const), |
| beforeEach(() => { | ||
| localStorage.clear(); | ||
| useAppStore.setState({ | ||
| editorFontSize: 14, | ||
| }); | ||
| }); | ||
|
|
||
| it('should have editorFontSize default to 14', () => { | ||
| const state = useAppStore.getState(); | ||
| expect(state.editorFontSize).toBe(14); | ||
| }); |
There was a problem hiding this comment.
These tests set editorFontSize in beforeEach, so the “default to 14” assertion doesn’t actually verify the store’s initialization/default logic (or localStorage bootstrapping) and will pass even if the real default changes. Consider restructuring to test initialization separately (e.g., by exporting init helpers, or by re-importing the store after setting localStorage via vi.resetModules()) and keep direct setState only for non-default behavior tests.
| beforeEach(() => { | ||
| localStorage.clear(); | ||
| useAppStore.setState({ | ||
| editorWordWrap: true, | ||
| }); | ||
| }); | ||
|
|
||
| it('should have editorWordWrap default to true', () => { | ||
| const state = useAppStore.getState(); | ||
| expect(state.editorWordWrap).toBe(true); | ||
| }); |
There was a problem hiding this comment.
These tests set editorWordWrap in beforeEach, so the “default to true” assertion doesn’t actually verify the store’s initialization/default logic (or localStorage bootstrapping) and will pass even if the real default changes. Consider restructuring to test initialization separately (e.g., by setting localStorage then re-importing the store with vi.resetModules()), and use direct setState only where you’re intentionally overriding state for a specific scenario.
Signed-off-by: Shubh-Raj <shubhraj625@gmail.com>
Signed-off-by: Shubh-Raj <shubhraj625@gmail.com>
| const getInitialFontSize = () => { | ||
| if (typeof window !== 'undefined') { | ||
| const saved = localStorage.getItem('editorFontSize'); | ||
| if (saved !== null) { | ||
| const parsed = parseInt(saved, 10); | ||
| if (!isNaN(parsed) && parsed >= MIN_FONT_SIZE && parsed <= MAX_FONT_SIZE) return parsed; |
There was a problem hiding this comment.
getInitialFontSize currently accepts any numeric value between MIN_FONT_SIZE and MAX_FONT_SIZE from localStorage. This can load a font size that's not in FONT_SIZE_OPTIONS (e.g., 17), leaving the Settings <Select> with a value that has no matching option and making the UI/state inconsistent. Consider validating the parsed value against FONT_SIZE_OPTIONS (or clamping/mapping to the nearest allowed option) and falling back to DEFAULT_FONT_SIZE otherwise.
| const getInitialFontSize = () => { | |
| if (typeof window !== 'undefined') { | |
| const saved = localStorage.getItem('editorFontSize'); | |
| if (saved !== null) { | |
| const parsed = parseInt(saved, 10); | |
| if (!isNaN(parsed) && parsed >= MIN_FONT_SIZE && parsed <= MAX_FONT_SIZE) return parsed; | |
| const getAllowedFontSizes = (): number[] => { | |
| return FONT_SIZE_OPTIONS | |
| .map((option) => { | |
| if (typeof option === "number") { | |
| return option; | |
| } | |
| if ( | |
| option && | |
| typeof option === "object" && | |
| "value" in option && | |
| typeof (option as { value: unknown }).value === "number" | |
| ) { | |
| return (option as { value: number }).value; | |
| } | |
| return null; | |
| }) | |
| .filter((value): value is number => value !== null); | |
| }; | |
| const getInitialFontSize = () => { | |
| if (typeof window !== 'undefined') { | |
| const saved = localStorage.getItem('editorFontSize'); | |
| if (saved !== null) { | |
| const parsed = parseInt(saved, 10); | |
| if (!isNaN(parsed)) { | |
| const allowedSizes = getAllowedFontSizes(); | |
| if (allowedSizes.includes(parsed)) { | |
| return parsed; | |
| } | |
| } |
| store.setEditorFontSize(5); | ||
| expect(useAppStore.getState().editorFontSize).toBe(14); | ||
|
|
||
| store.setEditorFontSize(100); | ||
| expect(useAppStore.getState().editorFontSize).toBe(14); |
There was a problem hiding this comment.
This test implies invalid font sizes are "rejected" by resetting to 14, but the store implementation currently just returns early and leaves the previous value unchanged. To make the assertion match the behavior and be more robust, set editorFontSize to a non-default valid value first (e.g., 18), then call setEditorFontSize with an invalid value and assert it remains 18.
| store.setEditorFontSize(5); | |
| expect(useAppStore.getState().editorFontSize).toBe(14); | |
| store.setEditorFontSize(100); | |
| expect(useAppStore.getState().editorFontSize).toBe(14); | |
| // Start from a non-default valid size | |
| store.setEditorFontSize(18); | |
| expect(useAppStore.getState().editorFontSize).toBe(18); | |
| // Invalid sizes should not change the current font size | |
| store.setEditorFontSize(5); | |
| expect(useAppStore.getState().editorFontSize).toBe(18); | |
| store.setEditorFontSize(100); | |
| expect(useAppStore.getState().editorFontSize).toBe(18); |
Closes #720
Add Font Size (dropdown) and Word Wrap (toggle) settings to the Settings modal, applying to all three Monaco code editors with localStorage persistence.
Changes
editorFontSize(number, default 14) andeditorWordWrap(boolean, default true) state to the Zustand store with localStorage persistence<Select>dropdown (12–20px) and Word Wrap<Switch>toggle to the Settings modalfontSizeandwordWrapfrom the storeeditorFontSizeandeditorWordWrapFlags
antdSelectandSwitchcomponentsshowLineNumberssetting for consistencyScreenshots or Video
Screencast.from.2026-02-19.20-11-31.webm
Related Issues
Author Checklist
--signoffoption of git commit.mainfromfork:branchname