You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "liugddx (via GitHub)" <gi...@apache.org> on 2026/08/27 12:30:52 UTC

[PR] feat(ui): add font-size controls (UI scale + terminal) to Appearance settings [maka]

liugddx opened a new pull request, #4024:
URL: https://github.com/apache/maka/pull/4024

   ## Summary
   
   Appearance settings only exposed *theme* and *palette* — there was no in-app way to change font size. The renderer type scale was fixed at `base: 14` and both xterm.js terminals hardcoded `fontSize: 12`, so users on large / high-DPI displays had to fall back to coarse OS-level scaling.
   
   This adds two controls to the Appearance page, mirroring Codex Desktop's *UI Font* / *Code Font* but deliberately kept as **separate** knobs:
   
   - **UI font size** — a document-root multiplier (`UI_FONT_SCALES`). Every `--font-size-*` token is `rem`, so scaling the root font-size uniformly grows text, icons and rem spacing (a real UI zoom). This is the intended hook, **not** the old `html { font-size }` density hack removed in `makaTheme.ts`. Restored before first paint in `cached-theme-bootstrap` to avoid a resize flash.
   - **Terminal font size** — feeds `getTerminalFontSize()` into both `Terminal` instances; open terminals subscribe and re-fit (re-emitting geometry to the PTY) on change.
   
   Both persist under `appearance` (already client-owned) and **fail closed** to their defaults on any malformed/out-of-range value — so a bad persisted value can't drive an arbitrary root size (the failure mode that made Codex's font size unusable).
   
   Refs #4021
   
   ## Verification
   
   - `npm --workspace @maka/core run typecheck` — pass
   - `npm --workspace @maka/desktop run typecheck` — pass (preload/main/renderer/storybook)
   - `node --test packages/core/dist/__tests__/settings.test.js` — 20/20 pass, including a new test covering the fail-closed normalize for `uiFontScale` / `terminalFontSize`
   - `biome check` on all changed files — clean
   
   Not run: full desktop app launch / screenshot (headless env). The change is behind the existing appearance apply-then-persist path used by theme/palette.
   
   ## AI use
   
   Select exactly one:
   
   - [ ] No generative tool made a substantive contribution
   - [x] Generative tooling made a substantive contribution
   
   Tool(s) and scope: Claude Opus 4.8 — codebase investigation, implementation across settings schema / theme apply layer / terminal wiring / settings UI + i18n, and the added unit test. Commit carries a `Generated-by` trailer.
   
   ## Checklist
   
   - [x] Tests cover the change and fail without it
   - [x] Lint, format, typecheck and the affected suites pass locally
   
   Does this PR entail a change in behavior?
   
   - [x] Yes — described under Summary above
   - [ ] No
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org


Re: [PR] feat(ui): add UI and terminal font size settings to Appearance [maka]

Posted by "liugddx (via GitHub)" <gi...@apache.org>.
liugddx commented on PR #4024:
URL: https://github.com/apache/maka/pull/4024#issuecomment-5447965353

   Thanks @Astro-Han — both points taken, addressed in d32399b:
   
   **P2 — wording + boundary evidence.** You're right that the px/rem ratio makes "a real UI zoom" an overclaim: the root scale moves what is rem-derived (text, Astryx's rem icon atoms) while px-literal spacing and widths stay put. Reworded in the PR body, `settings.ts` and `theme.ts` to say exactly that, and framed the tight clamp (11–22 around base 14) as the consequence. Added boundary screenshots to the body — collapsed sections "Boundary: 22px (max)" and "Boundary: 11px (min)". At 22px the layout absorbs the ~57% text growth by wrapping (palette tile descriptions go to 2–3 lines inside their fixed boxes); nothing clips or overflows, including the settings rows and the `px` steppers themselves. 11px is uneventful.
   
   **Suggestion — single-sourcing the base.** Done, with one deviation from "import from `makaTheme.ts` directly": the constant lives in a new dependency-free `astryx-theme/type-scale.ts` that both `makaTheme.ts` (scale input) and `theme.ts` (root-size divisor) import — `theme.ts` sits on the pre-paint bootstrap path (`cached-theme-bootstrap.ts`) and importing `makaTheme.ts` would pull the full theme + icon registry into the first frame. `astryx theme build` output verified byte-identical after the change. Since `DEFAULT_UI_FONT_SIZE` must stay in `@maka/core`, the renderer-side test you suggested (`font-size-type-scale.test.ts`) asserts it equals `TYPE_SCALE_BASE_PX` and that the built `--font-size-base` token matches the constant — so a future divergence fails loudly instead of silently mis-scaling.
   
   <details><summary>简体中文</summary>
   
   谢谢 @Astro-Han,两点都已在 d32399b 处理:
   
   **P2 —— 措辞 + 极端档位证据。** px/rem 比例确实让 "a real UI zoom" 言过其实:根字号只缩放 rem 派生的部分(文字、Astryx 的 rem 图标原子),px 字面量的间距和宽度不动。PR 正文、`settings.ts`、`theme.ts` 均已改为如实描述,并把紧贴基准的 clamp(11–22,基准 14)解释为这一事实的直接后果。正文新增两个折叠区 "Boundary: 22px (max)" 和 "Boundary: 11px (min)"。22px 下布局靠换行消化约 57% 的文字增长(色板卡片描述在固定盒内折成 2–3 行),没有截断或溢出,设置行和 px 步进器本身也正常;11px 一切正常。
   
   **建议 —— base 单一来源。** 已做,但与"直接从 `makaTheme.ts` 导入"有一处偏差:常量放进了新的零依赖模块 `astryx-theme/type-scale.ts`,`makaTheme.ts`(scale 输入)与 `theme.ts`(根字号除数)共同引用——因为 `theme.ts` 在首帧前 bootstrap 路径上(`cached-theme-bootstrap.ts`),直接导入 `makaTheme.ts` 会把整个主题连同图标注册表拖进首帧。改动后 `astryx theme build` 产物逐字节一致。`DEFAULT_UI_FONT_SIZE` 必须留在 `@maka/core`,所以按你的建议加了 renderer 侧测试(`font-size-type-scale.test.ts`):断言它等于 `TYPE_SCALE_BASE_PX`,且构建出的 `--font-size-base` token 与常量一致——未来若发散会显式失败而不是静默错缩放。
   
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org


Re: [PR] feat(ui): add UI and terminal font size settings to Appearance [maka]

Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #4024:
URL: https://github.com/apache/maka/pull/4024


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org