Repository navigation
🎯 refactor: Hold ClickHouse Colors at Their Click UI Values - #16821
Conversation
codegraph-librechat
Bot
added
the
🗺️ Design Tokens
752028b to
7e5815b
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e5815bad5
ℹ️ 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".
| * names a value for holds it exactly, by owner decision, including where that value | ||
| * misses a WCAG floor; `clickhouse.spec.tsx` pins each pair that does. A token Click UI |
There was a problem hiding this comment.
When a deployment selects the ClickHouse theme, this policy deliberately replaces accessible adjustments with values that fail the relevant contrast floors: in light mode link/link-prose fall to 3.58–3.84:1, border-control falls to 1.16–1.24:1, and several series marks fall as low as 1.19:1; dark controls and prose markers also fall below 3:1. Updating the tests to accept those values does not keep normal-sized links, field boundaries, switches, and graphical cues perceivable, so retain accessible semantic-role values rather than letting exact Click UI parity override the floors.
AGENTS.md reference: AGENTS.md:L49-L52
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not changed: by owner decision the ClickHouse theme matches Click UI over WCAG contrast (default and high-contrast themes keep theirs). Every sub-floor pair is pinned with its measured ratio in clickhouse.spec.tsx and listed in the PR body.
| ['rgb-border-menu', 'rgb-border-light', 'rgb-border-light'], | ||
| ['rgb-surface-composer', 'rgb-surface-chat', 'rgb-surface-chat'], | ||
| ['rgb-surface-search', 'rgb-surface-secondary', 'rgb-surface-secondary'], | ||
| ['rgb-surface-sidebar', 'rgb-surface-primary-alt', 'rgb-surface-primary-alt'], |
There was a problem hiding this comment.
When a custom light ThemeDefinition sets the new rgb-surface-sidebar role but omits rgb-drawer-edge, this entry resolves the panel color correctly, but resolveTheme's drawerEdgeSource still consults only rgb-surface-primary-alt. Because applyResolvedTheme writes the complete resolved palette, the edge remains LibreChat's bundled color rather than the custom drawer fill, producing a stray trailing border on the mobile drawer. Prefer rgb-surface-sidebar there, with rgb-surface-primary-alt retained as the legacy fallback.
AGENTS.md reference: AGENTS.md:L147-L151
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7ef05ec: the light drawer edge now derives from rgb-surface-sidebar, falling back to rgb-surface-primary-alt, with a registry spec.
| textLg: '1.125rem', // typography.font.sizes.4 | ||
| textXl: '1.25rem', // typography.font.sizes.5 | ||
| text2xl: '1.5rem', // LibreChat's: Click UI's next step (sizes.6, 2rem) passes the unthemed text-3xl | ||
| text2xl: '2rem', // typography.font.sizes.6 |
There was a problem hiding this comment.
With the ClickHouse theme selected, the workspace project-name editor renders the shared Input title variant as h-theme-field-lg text-2xl. That field remains 3rem tall and includes two 0.2813rem paddings plus its borders, while this change makes the text line itself 3rem tall (2rem at leading2xl: 1.5), so the editable title no longer fits its content box and is vertically clipped. Increase the large field height or give the title variant compatible leading/padding rather than enlarging this step independently of its control geometry.
AGENTS.md reference: AGENTS.md:L148-L151
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7ef05ec: ClickHouse sets fieldHeightLg to 3.6876rem so the 3rem title line, field padding and stroke fit; sourced in the Click UI drift spec.
|
Lighthouse CI failed. The last 80 log lines contain the measured budgets and assertion failures. |
9392f84 to
05b56cc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05b56cc6f1
ℹ️ 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".
| * on the same surface, so the fill no longer separates them on its | ||
| * own and the boundary has to be a line in every theme. */ | ||
| 'border-border-light bg-surface-primary-alt min-h-0 flex-1 overflow-hidden border-r', | ||
| 'border-border-light bg-surface-sidebar min-h-0 flex-1 overflow-hidden border-r', |
There was a problem hiding this comment.
When ClickHouse is active and the user opens Schedules, Memories, MCP Builder, or Bookmarks, this nav now uses surface-sidebar, but the shared PanelContent fade and PanelFooter still paint surface-primary-alt (client/src/components/ui/PanelContent.tsx:75 and PanelFooter.tsx:18). Since ClickHouse deliberately makes those colors different, scrollable panels show an off-color bottom gradient and panels with footers show a contrasting strip; pass/use surface-sidebar for these sidebar-owned layers as well.
AGENTS.md reference: AGENTS.md:L142-L151
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ee5ad0f: PanelContent's fade and PanelFooter now paint surface-sidebar, which equals surface-primary-alt in the default, dark and high-contrast themes. Checked under ClickHouse in light and dark: nav, fade and footer share one colour.
| isActiveConvo || isPopoverActive | ||
| ? 'ring-surface-nav-selected' | ||
| : 'ring-surface-primary-alt group-hover:ring-surface-nav-hover', | ||
| : 'ring-surface-sidebar group-hover:ring-surface-nav-hover', |
There was a problem hiding this comment.
When pull-request badges are enabled under ClickHouse, this updates the unread dot's idle ring for the new sidebar surface, but the adjacent PullRequestRowMark still defines its idle ring as ring-surface-primary-alt (client/src/components/Chat/PullRequest/RowMark.tsx:17). Because surface-sidebar is white/#1f1f1c while surface-primary-alt is #f6f7fa/#282828, the CI dot retains a visible halo on otherwise idle sidebar rows; update that marker to use the same sidebar role.
AGENTS.md reference: AGENTS.md:L142-L151
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ee5ad0f: the pull-request row mark's idle ring moves to surface-sidebar like the unread dot; the RowMark spec follows.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee5ad0f3f4
ℹ️ 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".
| 'rgb-border-menu', | ||
| 'rgb-surface-composer', | ||
| 'rgb-surface-search', | ||
| 'rgb-surface-sidebar', |
There was a problem hiding this comment.
When a signed-in user reloads after upgrading from the previous release, the added surface-sidebar, text3xl, and leading3xl properties are absent from their serialized deployment-theme entry, but both readThemeCache and the pre-bundle script in client/index.html still accept cache version 2. The boot paint therefore falls back to the bundled values—for example, cached ClickHouse renders the sidebar as surface-primary-alt and text-3xl at 1.875rem—until React replaces it with the new white sidebar and 2rem type scale, reintroducing the flash this cache exists to prevent. Bump the cache version in both readers when extending the resolved property schema.
AGENTS.md reference: AGENTS.md:L49-L52
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Already covered on dev by #16819: the cache version is derived from themeRoleFingerprint(), so adding surface-sidebar, text3xl and leading3xl changes it (hash 1iop44p against xbzud8 without them) and old entries are dropped. This head is rebased onto it (68e1349) and also bumps THEME_CACHE_EPOCH to 2 because the ClickHouse values changed without role changes. themeCache and Providers specs pass (136 tests).
ee5ad0f to
68e1349
Compare
Summary
The ClickHouse reference theme departed from Click UI in about 28 colour roles to keep WCAG contrast, and the drift spec recorded each departure as a documented mismatch. By owner decision ClickHouse now matches Click UI over WCAG contrast: every role takes the Click UI token value exactly in light and dark, and each value cites its token. The default, dark and high-contrast themes are untouched. ClickHouse file-type tiles map to distinct Click UI palette tokens (document, sheet, code, artifact, audio, video, generic). Roles Click UI draws with alpha (the danger alert fill and edge, the sidebar hover and selected fills) are held as the token composited on the surface it sits on, since a role holds an opaque triplet.
text2xltakestypography.font.sizes.6(2rem), sotext-3xlbecomes a theme role (default 1.875rem, unchanged) and a spec holds the type scale monotonic in every bundled theme;listMaxHeightmoves to the not-expressible list (3 entries, tracked at berry-13#282) because Click UI caps its select list at the popover's available height, which a length role cannot express.The snapshot is re-pinned to Click UI v0.14.0 (
c161141f4e, no token files changed), and the Lia sprite palette (client/src/components/Lia/engine/*.ts) is added to allowlist entry 4 as artwork (10 entries or fewer).A new
surface-sidebarrole (defaultsurface-primary-alt, so the default themes are unchanged) paints the sidebar panel, the mobile drawer, its list fade and the conversation row ring. ClickHouse sets it fromsidebar.main.color.background.default(#ffffff/#1f1f1c), recomposites the sidebar hover and selected fills on it, and takes the drawer edge fromsidebar.main.color.stroke.default; the search pill moves tofield.color.background.defaultin light so it still steps off the white sidebar. TheDataTableheader already paintstable-header-fill, which ClickHouse sets from Click UI's table header token, so nothing more was needed there. Fixes berry-13#141.Roles that now fall below WCAG, each pinned with its measured ratio in
clickhouse.spec.tsx. Light, AA text:text-secondary,-altand-tertiaryon the info, warning and error subtle fills (4.05 to 4.42:1),status-successon its fill (4.27:1),status-infoon its fill (3.32:1),linkandlink-proseon the page (3.84:1, 3.58:1 on the secondary surface). Light, 3:1 non-text:border-xheavy(2.03:1),prose-bulletandprose-quote-bar(2.03:1, 1.64:1 on the user bubble), chart series 2, 3, 5 and 7 (2.65, 1.72, 1.19 and 1.95:1),border-control(1.24:1) andswitch-unchecked(1.56:1). Dark, 3:1 non-text:border-xheavy(1.62:1),prose-bulletandprose-quote-bar(1.62:1, 1.26:1 on the user bubble),border-control(1.50:1) andswitch-unchecked(2.90:1). Dark cards (#1f1f1c, the canvas) and dark tooltips (#282828) follow Click UI's own definitions, so they sit close to the canvas. Darksurface-overlayis now Click UI's#606060scrim, which lifts the page rather than dimming it, and series 1 equalsstatus-infoin light because Click UI uses one blue for both.Type of change
Testing
Tested environments/configuration: Chromium, desktop light and dark plus mobile; the ClickHouse theme loaded from
librechat.yamlon a locallcpair, light and dark.Automated tests:
npx jest src/theme --maxWorkers=2inpackages/client: 12 suites, 559 tests pass (clickui drift spec reports 0 mismatch and 0 unsourced colour roles per mode;color-diff.cjsagrees)npx eslinton the touched theme files,prettier --check,node scripts/sort-imports.mts --check, andnpx tsc --noEmitinpackages/client: cleanclickhouse.spec.tsxpins every sub-floor pair instead, includingborder-controland the switch trackreviewctl precheck: every step passes (the config-migration step failed once on a Redis connection and passed on rerun)reviewctl verify --all --browsers chromium: the 12 contract scenarios pass (ClickHouse scenarios updated to the Click UI values; the default-theme layers scenario is unchanged and passes)Screenshots / recordings
No user-facing change for any deployment that does not select the ClickHouse theme. The sidebar surface is visible under ClickHouse only; light and dark were checked in the browser.
Risks
ClickHouse contrast regressions listed above are intentional.
text-3xlheadings now follow the theme scale, so none render smaller thantext-2xlunder ClickHouse.Checklist