🔤 feat: Bundle Inconsolata for the ClickHouse Theme - #16376
Conversation
c6d6dc1 to
9847d13
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6d6dc180c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 2) Original or Modified Versions of the Font Software may be bundled, | ||
| redistributed and/or sold with any software, provided that each copy | ||
| contains the above copyright notice and this license. These can be | ||
| included either as stand-alone text files, human-readable headers or | ||
| in the appropriate machine-readable metadata fields within text or |
There was a problem hiding this comment.
Ship the OFL with production font artifacts
In production builds this license file is never emitted: client/vite.config.ts disables publicDir and copyPublicAssets() copies only public/assets and robots.txt, while Dockerfile.multi copies only client/dist into the runtime image. The CSS references cause the WOFF2 files themselves to enter dist, so the multi-stage image redistributes Inconsolata without the notice and license required here. Emit the OFL as a bundle asset or explicitly copy it into the production artifact.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2a9690f. Production builds disable publicDir, so only the woff2 faces reached dist; copyPublicAssets now copies the licence texts from public/fonts into assets/fonts beside them. The inconsolata-licence-ships-with-font scenario fetches both files from the served build and checks the copyright and OFL text (desktop light, dark, mobile).
The ClickHouse theme names Inconsolata as its mono family, but the app did not ship the face, so code fell back to the system monospace. Self-host the latin 400 and 700 woff2 faces from Fontsource 5.3.0 with their OFL licence, declared with font-display: swap in a stylesheet of theme-opt-in faces. The browser fetches a declared face only when text renders in it, so the default theme never downloads them.
Production builds disable publicDir, so the woff2 faces reached dist through the $fonts alias while the OFL text next to them in public/fonts did not. The build now copies the licence texts from public/fonts into assets/fonts, beside the faces they cover.
9847d13 to
2a9690f
Compare
Pull Request
Summary
The ClickHouse reference theme names Inconsolata as its mono family (Click UI's
typography.font.families.mono), but the client never shipped the face, so withinterface.theme: clickhouseevery code block, inline code span andfont-monoutility fell back to whatever monospace the OS provides. This self-hosts Inconsolata the way Roboto Mono already is: the latin 400 and 700 woff2 faces from Fontsource 5.3.0 underclient/public/fonts, the SIL OFL 1.1 licence beside them, and@font-facerules withfont-display: swapresolved through the existing$fontsalias.The rules live in a new
client/src/fonts.css, imported frommain.jsxafterstyle.css, becausestyle.cssis owned by the open theming stack (#16243 to #16248). A browser fetches a declared face only when text renders in it, and the PWA precache globs only js, css and html, so the default theme downloads nothing new.Type of change
Testing
Tested environments/configuration:
interface.theme: clickhousein a worktree-locallibrechat.yaml, light and dark mode.client/distbuild: both faces are emitted underassets/fonts/, andsw.jsprecaches neither.Automated tests:
e2e/specs/mock/scenarios/clickhouse-theme-shape.spec.ts: under the ClickHouse theme a code element loads the Inconsolata face over the network anddocument.fonts.check('16px Inconsolata')is true; under the default theme both declared faces stayunloadedand no Inconsolata request is made.npx playwright test --config=e2e/playwright.config.mock.ts e2e/specs/mock/scenarios/clickhouse-theme-shape.spec.ts: 5 passed.cd packages/client && npx jest src/theme: 8 suites, 273 tests passed;npx tsc --noEmitclean.cd client && npm run typecheck: clean.npx eslint,npx prettier --checkandnode scripts/sort-imports.mts --checkon the touched files: clean.npm run lintreports no design-rule findings; it exits 1 on prettier drift in 259 files this change does not touch.Screenshots / recordings
Code blocks under the ClickHouse theme, before (system mono fallback) and after (Inconsolata).
Risk / compatibility
Two woff2 files (about 35 KB together) join the build output and are only requested by a theme whose mono stack names Inconsolata. Deployments on the default theme see no new requests.