Repository navigation
feat: Added adjustable font size and line height support for desktop and web - #54499
ShogunPanda wants to merge 4 commits into
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for porting this! The new tests pass, typecheck is clean and vite build succeeds. I checked the built CSS: fixed px font sizes and line heights, including Tailwind's text tokens, are rewritten to scale with --font-size-scale. I didn't run the UI in a browser.
Public API: unchanged. No config schema, SDK, server or plugin changes; the value lives in the app's own saved settings (settings.v3).
What would block a merge:
- Needs a rebase onto current
dev. The branch forks from a commit about 1,100 commits behinddev.context/settings.tsx,context/settings.test.tsandcomponents/settings-v2/general.tsxno longer exist there. Settings now live inpackages/app/src/settings/model.tsx, which already stores an unusedappearance.fontSize. A test merge also conflicts in every i18n file, so the change has to be moved onto the new layout. - No line-height setting. The title and description promise a separate line-height setting, but only font size is added; line heights just scale with it. Please add the setting or update the description.
- Persistence. The description says the value is persisted in
opencode.json, but it is stored in the app's own settings (settings.v3), not the config file. - Fixed-size layout doesn't scale (see the inline comment). Text and line heights grow up to about 1.7x while heights and spacing stay fixed, so text may clip or overflow in fixed-height rows at large sizes. Could you add before/after screenshots at 24px of the session view, sidebar and settings?
|
I messed up the original branch. Fixing as we speak. |
9deb696 to
8cfb0ef
Compare
This is now fixed. Not sure how to trigger @opencode-agent[bot] review again |
There was a problem hiding this comment.
Thanks for the rebase. My earlier points on the rebase, the missing line-height setting and where defaults and clamping live are resolved: the change now uses settings/model.tsx and the Appearance page, invalid stored values fall back through the schema, and adding only English strings matches packages/app/AGENTS.md. Unit tests (typography.test.ts, model.test.ts, search-results.test.ts) pass, and typecheck is clean in packages/app and packages/gui-extensions. I checked the built CSS but did not run the UI in a browser.
Public API: the GUI extension SDK's Appearance interface (packages/gui-extensions/src/sdk/host-apis.ts) gains a required fontSize(): number. Extensions that read it are unaffected. Anything that implements its own Appearance, like the fixtures updated here, must add the method. Config, server and CLI are unchanged.
What I'd still like to see fixed:
- The line-height setting scales control heights but not widths (inline). In the build,
height,padding,gapand--spacingare multiplied by--app-line-height-scale, but fixedwidths are not. For example,icon-button-v2[data-size=small]becomeswidth: 1.25rem; height: calc(1.25rem * scale), and file icons get the same treatment. At 30, square buttons and icons become about 1.5x taller than wide. This is broader than "line spacing" and goes against the AGENTS.md rule to keep control and row heights explicit. I'd limit this setting toline-heightvalues. - Every
pxbecomesrem, scaled by the root font size (inline). Borders, radii and widths now scale too: 1px borders are about 0.7px at size 10. Sizes fixed in TypeScript don't follow:COMPOSER_SUGGESTION_ROW_HEIGHT = 28incomposer/editor/editor.tsxand the timeline virtualizer's fallback item size. Please check the composer suggestion popover and timeline at 10 and 24, and attach screenshots at the extremes. - The description still says the values are persisted in
opencode.json; they are stored in the app's own settings. typography.test.tsruns a full Vite build to test a PostCSS plugin. Calling the plugin on a small parsedpostcssroot would be quicker and clearer. The new e2e test checks six things in one flow; the font-size and line-height values surviving a refresh is the core case to keep.
There was a problem hiding this comment.
Thanks for the quick follow-up. Both inline points are resolved. The PostCSS plugin now only scales font-size/line-height and their tokens, so control geometry stays as written; in the build, icon-button-v2[data-size=small] is back to 20px × 20px. The root font size is no longer changed, so layouts sized in code (composer suggestion rows, the virtualizer's fallback size) and borders keep their scale. The smaller typography.test.ts (plugin run on a parsed root) and the trimmed e2e test are much clearer, and the description now matches where the values are stored. bun test (typography, settings model) passes and typecheck is clean. I checked the built CSS but did not run the UI.
Public API: unchanged since my last review. The GUI extension SDK's Appearance gains a required fontSize(): number: backwards compatible for extensions that read it; custom Appearance implementations must add it.
One thing still worth checking before merge: fixed-height rows at the limits (inline). Text and line boxes now grow while explicit row heights stay fixed. At font size 24 and line height 30, a --line-height-base line box can reach about 51px inside 20–28px rows. Could you add screenshots at 24 / 30 of the session view, sidebar, composer suggestions and settings? If they clip, lowering the maximums would be the simplest fix.
|
@opencode-agent All good now? |
|
Thanks, everything I raised has been addressed in code: the rebase, the separate line-height setting, controls that no longer stretch, the root font size left unchanged, smaller tests, the description fix, and the lower 16 / 22 maximums. The one open point is a small one: fixed 20px-tall rows (e.g. the review panel's empty-changes description) may still clip slightly at 16 / 22, since the line box is about 25px there. A screenshot at those maximums of the session view, sidebar and composer suggestions would settle it. I haven't run the UI myself; merging is up to the maintainers. |
|
Confirmed still unfixed on the currently shipped desktop build — OpenCode Desktop 2.0.29 / CLI sidecar 2.0.29, Windows 11 (build 26300), 1920×1080 @125%. Reproduction
"appearance": {
"fontSize": 18
}
Bundle inspection confirms the three-part problem from #241311. The store declares the setting, but nothing ever reads or writes it. Defaults and accessors, verbatim from appearance: {
fontSize: 14, mono: "", sans: "", terminal: "", tabLayout: "horizontal"
}
// ...
appearance: {
fontSize: Ak(() => e.appearance?.fontSize, kk.appearance.fontSize),
setFontSize(e) { t("appearance", "fontSize", e) },
font: Ak(() => e.appearance?.mono, kk.appearance.mono),
setFont(e) { t("appearance", "mono", e.trim() ? e : ``) },
uiFont: Ak(() => e.appearance?.sans, kk.appearance.sans),
setUIFont(e) { t("appearance", "sans", e.trim() ? e : ``) },
terminalFont: Ak(() => e.appearance?.terminal, kk.appearance.terminal),
setTerminalFont(e) { t("appearance", "terminal", e.trim() ? e : ``) },
tabLayout: ...
}A full scan of the bundle for 2. The tokens that actually drive the UI are hard-coded in the bundled CSS. --font-size-small: 13px;
--font-size-base: 14px;
--font-size-large: 16px;
--font-size-x-large: 20px;aliased as 3. The embedded terminal is initialised with a fixed size rather than the setting. Two points this adds on top of #24131
Workaround that works todayChromium page zoom: Happy to test against this branch if a build or instructions for running desktop from source are available. |
5af3816 to
1c693ec
Compare
1c08d0e to
0049dc5
Compare
There was a problem hiding this comment.
Lowering the limits to 10–16 and 20–22 addresses my last point about text clipping in fixed-height rows. The rest of the change is a rebase onto current dev with no other code differences. Unit tests (typography, settings model, search-results) pass, and tsgo -b reports no errors in packages/app and packages/gui-extensions. I didn't run the UI.
Public API: unchanged since my last review (the extension SDK's Appearance gains a required fontSize(): number).
One new bug: the two new settings descriptions use {min}/{max}, but the app's translator (@solid-primitives/i18n resolveTemplate) only fills in {{name}}. They currently render literally as ({min}–{max} px) (inline).
packages/app/src/runtime/i18n/en.ts:838: The app's translator (resolveTemplatefrom@solid-primitives/i18n) only fills in{{name}}placeholders, like the other strings in this file.resolveTemplate("Scale interface text ({min}–{max} px)", { min: 10, max: 16 })returns the string unchanged, so the settings row shows({min}–{max} px)literally. The same applies tolineHeight.descriptiontwo lines below. Using{{min}}–{{max}}in both strings fixes it.
0049dc5 to
ebdd2aa
Compare
…and web. Signed-off-by: Paolo Insogna <[email protected]>
Signed-off-by: Paolo Insogna <[email protected]>
Signed-off-by: Paolo Insogna <[email protected]>
Signed-off-by: Paolo Insogna <[email protected]>
ebdd2aa to
291f8a9
Compare
Issue for this PR
Closes #26269.
Closes #16145.
Closes #10423.
Port of #27684 to OpenCode 2.
Type of change
What does this PR do?
This PR introduces two new settings in the Desktop and Web application: font size and line height.
Both are numbers. They are applied consistently throughout the application and persisted in the app’s own settings store (
settings.v3), notopencode.json.How did you verify your code works?
Tested locally.
Screenshots / recordings
opencode.mov
Checklist
If you do not follow this template your PR will be automatically rejected.
Added by the opencode bot (AI-generated).
Fixes #49375
Related: #44184
Public API: adds a required
fontSize(): numberto the GUI extension SDKAppearanceinterface (backwards compatible for extensions that read it; customAppearanceimplementations must add it)