🎛️ feat: Close the Data-Level Click UI Gaps in the ClickHouse Theme - #16379
Conversation
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: bf1ad772bf
ℹ️ 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".
| shadowSm: '0 2px 2px 0 rgb(0 0 0 / 0.03)', // shadow.5 | ||
| controlHeight: '2rem', // genericMenu.panel.size.height | ||
| spaceCompact: '0.5rem', // button.basic.space.gap | ||
| spaceNormal: '1rem', // button.basic.space.x |
There was a problem hiding this comment.
With the ClickHouse theme active, this control-oriented value also drives px-theme-normal in the user-message bubble (client/src/components/Chat/Messages/ui/MessageRow.tsx:125) and queued-steer bubble (client/src/components/Chat/Messages/Content/Parts/SteerPart.tsx:126). Raising it from the 0.75rem default to 1rem therefore adds horizontal padding and makes conversation content wrap earlier even though the value is sourced from Click UI's button padding and the change is intended for controls. Keep message layout on its existing spacing or introduce a control-specific appearance role rather than using this shared spacing token for both concerns.
AGENTS.md reference: AGENTS.md:L81-L86
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 140b08f. spaceNormal and spaceCompact also pad the message and steer bubbles and the composer send button, so the ClickHouse theme now keeps LibreChat's values for both (0.75rem is also Click UI's field padding). The spec asserts both equal the defaults, and the clickhouse-controls-follow-click-ui scenario checks px-theme-normal renders 12px in desktop light, dark and mobile.
Set the theme values Click UI defines that the ClickHouse theme still left at LibreChat defaults: - controlHeight 2rem (genericMenu.panel.size.height), spaceCompact 0.5rem (button.basic.space.gap), spaceNormal 1rem (button.basic.space.x) and motionFast 100ms (transition.default). - Light ring-primary to outline.default #437eef, matching the dark mode ring, which already used outline.default. - Light surface-overlay to dialog.color.opaqueBackground #151515. - Dark surface-inverted to Click UI's primary fill #faff69 with its hover and #1f1f1c label, which is also its checked checkbox and switch colour.
…House theme spaceNormal and spaceCompact are not control-only: spaceNormal pads the user message and queued-steer bubbles, and spaceCompact is the composer send button's inner padding. Raising them to Click UI's button padding and gap widened the bubbles and shrank the send icon's box. Keep both at the defaults; 0.75rem is already Click UI's field padding.
bf1ad77 to
140b08f
Compare
1 participant
Pull Request
Summary
With
interface.theme: clickhouse, several control roles were still LibreChat defaults. This sets them from Click UI tokens, inthemes/clickhouse.tsonly: control height 2rem (genericMenu.panel.size.height), fast motion 100ms (transition.default), the light ring role asoutline.default#437eef, the light dialog scrim as #151515, and the dark checked and primary fill as #faff69.The shared spacing roles keep LibreChat's values because they also pad message bubbles and the send button. The dark scrim stays black (Click UI's #606060 drops the dialog edge under 3:1), and the light inverted fill stays #151515 because one role drives the button, checkbox and switch. The primitives that draw focus with
text-primaryare tracked in berry-13#146; the remaining gaps are filed as follow-ups.Type of change
Testing
Tested environments/configuration:
interface.theme: clickhousein a worktree-locallibrechat.yaml, light and dark mode,@librechat/clientrebuilt so the built-in theme carries the new values.Automated tests:
clickhouse.spec.tsx: the control roles now differ from LibreChat's, a new case pins the control height and motion and keeps the spacing roles at their defaults, and a per-mode case pins the checked fill and ring. Every existing contrast floor passes on the new values.cd packages/client && npx jest src/theme: 8 suites, 276 tests passed;npx tsc --noEmitclean.clickhouse-controls-follow-click-ui; the ClickHouse, deployment, radius/font and shadow theme scenarios: 24 passed.reviewctl verifyon the pushed head: 3 scenarios pass in desktop light, desktop dark and mobile.cd client && npm run typecheck: clean.npx eslint,npx prettier --check,node scripts/sort-imports.mts --checkon touched files: clean.Screenshots / recordings
Composer controls (32px control height):
Settings dialog (light scrim is now #151515; dark checked switches take #faff69):
Risk / compatibility
Only the ClickHouse theme definition changes; the default, dark and high-contrast themes are untouched. Deployments on the ClickHouse theme get 32px composer controls in place of 36px, which is still above the 24px WCAG 2.5.8 target minimum.