Repository navigation
📝 feat: Visible Prose Markers and Composer Shape Roles for ClickHouse - #16736
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8cde0c50d
ℹ️ 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".
| --surface-code-inline: var(--surface-active-alt); | ||
| --prose-bullet: var(--border-medium); | ||
| --prose-quote-bar: var(--border-medium); |
There was a problem hiding this comment.
Rebind the prose aliases inside scoped legacy themes
When the public applyTheme(colors, nestedRoot) path is used, these aliases are inherited from html and remain bound to the document-level --border-medium/surface values; changing those source roles on the [data-theme-scope] root therefore no longer recolors bullets, quote bars, or inline-code chips as it did before this change. The existing avatar alias is explicitly redeclared for scoped roots for the same reason, so these three aliases should also be rebound there (or written by the legacy adapter) to preserve nested custom themes.
AGENTS.md reference: AGENTS.md:L147-L151
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6a6be6e: the scoped legacy root now redeclares the three aliases next to the avatar one, following the mode.
| control: 'rounded-theme-control', | ||
| theme: 'rounded-theme-control-round', | ||
| /** The composer's send and stop slot, whose corner a theme sets apart from the other round controls. */ | ||
| composer: 'rounded-theme-composer-action', |
There was a problem hiding this comment.
Apply the composer shape to speech controls
When speech input is enabled, RoundButton in client/src/components/Chat/Input/Composer/Bar.tsx renders both the mic/recording-stop control and the dictation send control without shape="composer", so IconButton keeps its default rounded-full shape. In the ClickHouse theme these controls therefore remain circular while the adjacent normal send/stop slot uses the new 4px composer radius, despite composerActionRadius explicitly covering the composer's mic buttons; pass the new shape through this speech-control path as well.
AGENTS.md reference: AGENTS.md:L147-L151
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6a6be6e: the dictation RoundButton in Composer/Bar.tsx now takes shape="composer" (covered in appearanceTokens.spec).
512ebe8 to
6a6be6e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a6be6eb9e
ℹ️ 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".
| --tw-prose-bold: rgb(var(--text-primary)); | ||
| --tw-prose-counters: rgb(var(--text-tertiary)); | ||
| --tw-prose-bullets: rgb(var(--border-medium)); | ||
| --tw-prose-bullets: rgb(var(--prose-bullet)); |
There was a problem hiding this comment.
Route all prose quote borders through the new role
When Markdown is rendered in a .prose container without the .markdown class, such as TermsAndConditionsModal.tsx, blockquotes use --tw-prose-quote-borders, which remains bound to border-light in light mode and border-medium in dark mode. Consequently, the new prose-quote-bar color is ignored on that user-visible Markdown surface, leaving ClickHouse's light-mode quote bar at the low-contrast color this change is intended to replace; update both quote-border variables to use the new role or otherwise wire that renderer to it.
AGENTS.md reference: AGENTS.md:L147-L151
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid but pre-existing and outside this change (only .prose renderers such as the terms modal): tracked at berry-13#272.
| largeSurfaceRadius: '--theme-large-surface-radius', | ||
| menuRadius: '--theme-menu-radius', | ||
| popoverRadius: '--theme-popover-radius', | ||
| menuPanelRadius: '--theme-menu-panel-radius', |
There was a problem hiding this comment.
Apply the menu-panel radius to composer menus
When a ClickHouse user opens a chip's sub-mode menu or the code-approval mode menu, those composer panels still use rounded-xl in Composer/Bar.tsx and CodeApprovalMenu.tsx, so they retain 12px corners while the model menu and other newly themed composer overlays use the intended 4px Click UI corner. Since both are Ariakit menu panels, route them through menuPanelRadius as well so the new semantic role consistently controls this surface.
AGENTS.md reference: AGENTS.md:L147-L151
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5dc1e3a: the chip sub-mode and code approval menus use rounded-theme-menu-panel.
feb731d to
02c59f1
Compare
|
Existing e2e spec edits in this PR, and why none of them moves a default-theme value:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02c59f11e4
ℹ️ 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".
| largeSurfaceRadius: '--theme-large-surface-radius', | ||
| menuRadius: '--theme-menu-radius', | ||
| popoverRadius: '--theme-popover-radius', | ||
| menuPanelRadius: '--theme-menu-panel-radius', |
There was a problem hiding this comment.
Apply the menu-panel role to code-workspace menus
After the chip-mode and approval-menu conversions in this head, the code-workspace Ariakit menus still use menuClasses with rounded-xl in client/src/components/Chat/Input/CodeWorkspaceMenu.tsx:57-63. When code-workspace controls are available, both the main menu and machine submenu therefore keep ClickHouse's 8px radiusXl instead of the new 4px menuPanelRadius, even though they are equivalent composer menu panels; route that shared class through rounded-theme-menu-panel as well.
AGENTS.md reference: AGENTS.md:L144-L151
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ee80ebe: CodeWorkspaceMenu's shared menuClasses use rounded-theme-menu-panel (default stays 0.75rem).
02c59f1 to
5dc1e3a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5dc1e3ab3e
ℹ️ 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".
| largeSurfaceRadius: '0.75rem', | ||
| roundControlRadius: '9999px', | ||
| menuRadius: '0.25rem', // genericMenu.panel.radii.all | ||
| popoverRadius: '0.25rem', // genericMenu.panel.radii.all, the panel the composer popovers are |
There was a problem hiding this comment.
Route the token-usage popover through a shape role
When token usage is available and the composer gauge is opened, client/src/components/Chat/Input/TokenUsage/index.tsx:238-243 still gives its Ariakit popover rounded-xl. ClickHouse maps that global scale step to 8px, so this composer popover remains visibly rounder than the new 4px popover/menu-panel roles; route it through an appropriate semantic role while preserving the default theme's existing 12px corner.
AGENTS.md reference: AGENTS.md:L144-L151
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ee80ebe: the token usage popover uses rounded-theme-menu-panel, which defaults to the existing 0.75rem corner.
There was a problem hiding this comment.
Fixed in ee80ebe and 970f688. Swept every popover, menu and overlay panel under Chat/Input, Chat/Menus and the shared HoverCard primitive; the decision for each:
Menu panel role (rounded-theme-menu-panel, default 0.75rem): CustomMenu (model selector), CodeApprovalMenu, Composer/Bar chip sub-mode menu, CodeWorkspaceMenu (main and machine submenu), TokenUsage popover, HoverCard primitive (Effort and token usage Breakdown cards).
Popover role (rounded-theme-popover, default 1rem): Palette, Thinking, Reasoning, Mention, PromptsCommand, SkillsCommand, AskUserQuestionPopover.
Deliberately different, unchanged: EscalateNowButton tooltip (tooltip, not a panel); OGDialogContent dialogs (Catalog, PastedTextDialog, MyFilesModal, PresetsMenu, EditPresetDialog use the dialog and surface roles); DropdownPopup, Select, Combobox, DropdownMenu, InputCombobox (shared primitives at rounded-md, a different step with many consumers outside the composer); TimePicker, ControlCombobox, MultiSelect (shared form-control panels outside the composer, left to the overlays lane); trigger buttons and list items (rounded-lg/rounded-xl on size-9 triggers and rows are controls, not panels).
The guard is Chat/Input/__tests__/panelRadius.spec.ts: every Ariakit Menu/Popover, Popover.Content and menuClasses constant under Chat/Input and Chat/Menus must name a theme radius role, and HoverCard.spec.tsx renders the primitive. Reverting either fix fails it locally.
There was a problem hiding this comment.
Repo-wide sweep of client/src and packages/client/src for popover, menu, combobox and listbox panels still on a literal rounded-(md|lg|xl|2xl) (added to the earlier composer list). The second batch is committed locally and goes out with the next push, not yet on the published head.
Menu panel or popover, now themed (default value unchanged: popover 1rem equals rounded-2xl, menu panel 0.75rem equals rounded-xl): Chat/Messages/Feedback.tsx up and down popovers and Chat/Messages/Fork.tsx popover and hover panels (popover role); Chat/BackgroundTasks/Button.tsx panel (menu panel role). The panelRadius guard now scans Messages and BackgroundTasks as well.
Not a menu panel, because: Chat/approval/Review.tsx (a pinned approval card, not an anchored panel); OriginalDialog (dialog surface); Nav/AccountSettings account popovers (rounded-lg, 0.5rem, a different step with no role of its own, outside this change); Select, Combobox, DropdownMenu, InputCombobox (rounded-md, shared form primitives with many consumers); TimePicker, ControlCombobox, MultiSelect, LocalizedDateRangePicker, VariablesDropdown, Variables/Editor (form-control pickers outside the composer and message surfaces, left to the overlays lane); trigger buttons, list rows and tooltips.
There was a problem hiding this comment.
Correction to the list above: the form-control pickers (TimePicker, ControlCombobox, MultiSelect, LocalizedDateRangePicker, VariablesDropdown, Variables/Editor) and the AccountSettings rounded-lg popovers are not owned by another lane. They are tracked in berry-13#280.
|
Lighthouse CI failed. The last 80 log lines contain the measured budgets and assertion failures. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a72357e7a1
ℹ️ 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".
| import path from 'path'; | ||
|
|
||
| const CHAT_DIR = path.resolve(__dirname, '../..'); | ||
| const SCAN_DIRS = ['Input', 'Menus', 'Messages', 'BackgroundTasks']; |
There was a problem hiding this comment.
Route the pending approval card through the popover role
When a tool approval interrupts a chat, Chat/approval/Review.tsx:106 renders its composer overlay with rounded-2xl, so ClickHouse leaves this card at its 8px radius2xl instead of the new 4px popoverRadius used by the equivalent ask-user and palette overlays. Fresh evidence after the earlier radius sweep is that this new guard scans only the listed directories and omits the sibling Chat/approval directory, allowing the remaining literal to pass; include that surface in the migration and guard it with rounded-theme-popover.
AGENTS.md reference: AGENTS.md:L144-L151
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 335f87f: Chat/approval/Review.tsx uses rounded-theme-popover (default 1rem unchanged) and the panel guard now scans Chat/approval and className="popover" panels.
335f87f to
08305d6
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08305d6e7c
ℹ️ 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".
| largeSurfaceRadius: '--theme-large-surface-radius', | ||
| menuRadius: '--theme-menu-radius', | ||
| popoverRadius: '--theme-popover-radius', | ||
| menuPanelRadius: '--theme-menu-panel-radius', |
There was a problem hiding this comment.
Apply the menu-panel radius to send-action flyouts
When generation is active, DuringRunSendButton supplies its alternate actions to the shared SendActions primitive, whose Ariakit hovercard still uses rounded-xl in packages/client/src/components/SendActions.tsx:63. Under ClickHouse that resolves to the 8px radiusXl, so this composer action menu remains rounder than the new 4px menuPanelRadius; route the shared flyout through rounded-theme-menu-panel and cover this package primitive in the radius guard.
AGENTS.md reference: AGENTS.md:L144-L151
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 982fa1f: SendActions' flyout uses rounded-theme-menu-panel (default 0.75rem unchanged) and the panel guard now covers the packages/client primitive.
| --theme-popover-radius: 1rem; | ||
| --theme-menu-panel-radius: 0.75rem; | ||
| --theme-composer-action-radius: 9999px; |
There was a problem hiding this comment.
Preserve scale-variable inheritance for the new radius roles
When a package consumer imports theme.css and customizes the previously public --theme-radius-2xl, --theme-radius-xl, or --theme-round-control-radius variables directly, migrated composer surfaces now ignore those overrides because these new properties are fixed literals. resolveTheme preserves the old relationships through inheritedAppearance, but the documented stylesheet customization path does not; define these properties as var() aliases to their former scale variables, with the current literals as fallbacks, so existing CSS themes retain their shapes.
AGENTS.md reference: AGENTS.md:L144-L151
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 982fa1f: --theme-popover-radius, --theme-menu-panel-radius and --theme-composer-action-radius now alias --theme-radius-2xl, --theme-radius-xl and --theme-round-control-radius with the former literal as fallback, so a stylesheet that overrides the scale variables keeps reshaping these surfaces.
982fa1f to
8ef6614
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ef6614518
ℹ️ 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-surface-code-inline': '230 231 233', // #e6e7e9 (palette.slate.100, one step under the chip it replaced, slate.200 at 1.56:1) | ||
| 'rgb-prose-bullet': '128 134 145', // #808691 (palette.slate.500, 3.5:1 on white; stroke.default is 1.24:1) | ||
| 'rgb-prose-quote-bar': '128 134 145', // #808691 (palette.slate.500, 3.5:1 on white) |
There was a problem hiding this comment.
Keep prose roles visible on user-message bubbles
When enableUserMsgMarkdown is enabled, user Markdown renders inside bg-surface-user-message (#e6e7e9), but this light palette sets the inline-code surface to that exact color, so the chip background disappears; before this change it used #cccfd3. The marker and quote-bar color also has only 2.96:1 contrast against that bubble, below the intended 3:1 floor, because the new contrast test checks only page surfaces. Include the actual user-message surface in the reference-theme tests and choose prose roles that remain distinguishable there.
AGENTS.md reference: AGENTS.md:L148-L151
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 706e319: light chip is slate.200 and dark chip neutral.700 so neither equals the user bubble, and the list marker and quote bar are slate.700, clearing 3:1 on the bubble; the reference-theme spec now checks the bubble.
| popoverRadius: '0.25rem', // genericMenu.panel.radii.all, the panel the composer popovers are | ||
| menuPanelRadius: '0.25rem', // genericMenu.panel.radii.all | ||
| composerActionRadius: '0.25rem', // button.basic.radii.all, Click UI draws its icon buttons square | ||
| inlineCodeWeight: '500', // typography.font.weights.2 |
There was a problem hiding this comment.
Ship the font face selected by the inline-code role
Under ClickHouse, .prose code inherits font-mono, which resolves to Inconsolata, but the bundled fonts.css declares that family only at weights 400 and 700. Requesting 500 therefore selects the regular 400 face rather than rendering the Click UI medium weight this role names; the current tests only inspect the computed font-weight: 500, which does not reveal the face substitution. Add a 500/variable Inconsolata face or choose an available weight and verify the rendered reference theme.
AGENTS.md reference: AGENTS.md:L148-L151
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 706e319: the Inconsolata 500 face (same fontsource source as the 400 and 700 files) is bundled and declared, and a spec ties the declared faces to inlineCodeWeight.
…he Menu Panel Radius
…rd Composer Panels
…e the Send Actions Flyout
… Inline Code Font Weight
…s the Earlier Surface
67e941e to
20b7e6c
Compare
Summary
In the ClickHouse theme, Markdown list bullets and the blockquote bar render on
border-medium(1.24:1 on the page), so they all but disappear; the inline code chip is a heavy#cccfd3pill in semibold; and the composer popovers, the model selector panel and the send and stop buttons keep the default theme's rounded corners next to 4px Click UI controls. This adds semantic roles for each so a theme can set them apart from the shared border, surface and radius scales, with defaults that reproduce today's appearance exactly.Colors:
surface-code-inline,prose-bullet,prose-quote-bar(a theme that names none keeps the border and surface roles those elements read before). Appearance:popoverRadius,menuPanelRadius,composerActionRadius,inlineCodeWeight. ClickHouse takes slate.500 / neutral.500 for the marker and bar (3.5:1 and 3.7:1), slate.100 / neutral.712 for the chip at weight 500, the Click UI menu panel corner for the popovers and selector, and the Click UI button corner (4px) for send and stop. The default theme keeps a circle for send and stop.Type of change
Testing
Tested environments/configuration:
interface.theme: clickhouse, and the default theme for the pixel-identity check#cdcdcd/#393939, code chip#e3e3e3/#424242at weight 600, popover radius 16px, send radius 9999pxAutomated tests:
cd packages/client && npx jest src/theme src/utils src/components/IconButton(theme specs include the Click UI drift guard, a 3:1 check on the marker and bar, and the fallback for themes that name none of the new roles)cd client && npx jest src/components/Chat/Input/__tests__and the Input component specs related to the changed filesnpx tsc --noEmitinpackages/client,packages/data-providerandclient;npm run static-checks -- --against origin/devScreenshots / recordings
ClickHouse theme, before and after. List bullets, quote bar and code chip in light and dark; the attach popover in light. Send and stop are 4px in the after state.
Risk / compatibility
New appearance and color roles are optional in theme definitions and default to today's values;
IconButtongains acomposershape used by the send and stop buttons.Checklist