🧭 feat: Give the Mobile Drawer Edge a Role and Move the Last Page Colours Onto Roles - #16631
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The semantic role migration is internally consistent, backward-compatible, and covered by focused unit and browser tests.
Review effort: Balanced
Findings: None
What changed in this PR
Adds semantic theme roles to remove remaining hard-coded page colors while preserving mode-specific drawer behavior.
Changes:
- Adds and documents the
drawer-edgetheme role across all palettes. - Applies semantic overlay, scrim, and widget surfaces to affected UI.
- Adds unit and end-to-end coverage for theme resolution and rendering.
| File | Description |
|---|---|
packages/data-provider/src/theme.ts |
Registers the drawer-edge token. |
packages/client/src/theme/types/index.ts |
Adds drawer-edge theme types. |
packages/client/src/theme/tokens.css |
Exposes the Tailwind color variable. |
packages/client/src/theme/themes/highContrast.ts |
Defines high-contrast edge colors. |
packages/client/src/theme/themes/default.ts |
Defines the light edge color. |
packages/client/src/theme/themes/dark.ts |
Defines the dark edge color. |
packages/client/src/theme/themes/clickui.spec.ts |
Records Click UI token sources. |
packages/client/src/theme/themes/clickhouse.ts |
Defines ClickHouse edge colors. |
packages/client/src/theme/registry.ts |
Resolves backward-compatible edge fallbacks. |
packages/client/src/theme/registry.spec.ts |
Tests fallback and explicit overrides. |
packages/client/src/theme/README.md |
Documents the new utility. |
packages/client/src/theme/defaults.spec.ts |
Updates token-count coverage. |
packages/client/src/theme/defaults.css |
Adds mode-specific CSS defaults. |
packages/client/src/components/MultiSelect.tsx |
Adds the widget surface variant. |
packages/client/src/components/MultiSelect.spec.tsx |
Tests popover surface selection. |
eslint-suppressions.json |
Reduces resolved design-lint suppressions. |
e2e/specs/mock/scenarios/mobile-drawer-controls.spec.ts |
Updates drawer boundary assertions. |
e2e/specs/mock/scenarios/drawer-edge-theme.spec.ts |
Tests custom edge theming. |
client/src/components/Web/Sources.tsx |
Uses media-overlay roles for captions. |
client/src/components/UnifiedSidebar/UnifiedSidebar.tsx |
Applies the drawer-edge role. |
client/src/components/Prompts/forms/PromptForm.tsx |
Uses the semantic scrim role. |
client/src/components/Insights/InsightsView.tsx |
Uses the widget popover surface. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
berry-13
marked this pull request as ready for review
October 1, 2026 16:29
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. |
…rs Onto Roles The mobile drawer drew its edge with dark-only border utilities, the web source caption with gray-900 and white, the prompt panel's mobile backdrop with black at 20%, and the Insights agent picker popover with a dark-only override, so no theme could reach them. drawer-edge paints the drawer's trailing edge: the drawer's own fill in light, where the scrim already separates it, and border-xheavy in dark, so dark mode keeps today's edge; a theme that sets only those two roles keeps the edge its mode drew. The source caption reads the media overlay roles, the prompt panel backdrop takes the page scrim the mobile drawer uses, and MultiSelect gains a widget surface that the Insights picker uses in place of its dark override.
resolveTheme spreads every role fallback into one object literal, and each untyped conditional spread doubles the union the checker builds. One more pushed tsc for packages/client past CI's 6 GB heap. Typing the fallback as Partial<IThemeRGB> keeps it from adding to that union; a full check now peaks at about 4 GB.
berry-13
force-pushed
the
berry-13/theme-leakage-followups
branch
from
October 2, 2026 21:55
cfd97ad to
bda163c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four places still painted colours no theme could reach. The mobile drawer drew its edge with dark-only utilities (
dark:border-r dark:border-border-xheavy, added with the drawer rework in #16248). The web source image caption usedbg-gray-900/80 text-white. The prompt panel's mobile backdrop usedbg-black/20. The Insights agent picker popover usedbg-surface-primary dark:bg-chart-widget-surface.A new
drawer-edgerole carries the drawer's trailing edge. It defaults to the drawer's own fill in light, where the scrim already separates the drawer, and toborder-xheavyin dark, so dark mode keeps today's edge. A theme that sets onlysurface-primary-altorborder-xheavykeeps the edge its mode drew, and ClickHouse sources both modes from the same Click UI tokens as those roles. The source caption reads the media overlay roles from #16596. The prompt panel backdrop takesbg-scrim, the page scrim the mobile drawer already uses.MultiSelectgainssurface="widget", which the Insights picker uses in place of its dark override.Audit (themable rubric, 2026-10-01), origin/dev to this head: C6
c6.shhits 13 to 7, outside the allowlist 11 to 5. The 5 left areArtifacts.tsx(2, owned by #15911) andAgentConfig.tsx(3, owned by #16523). C7c7refined.shis unchanged at 127, 1 outside the allowlist (ArtifactCodeEditor.tsx:64, #15911). Design-rule suppressions 2506 to 2503.Closes berry-13#222
Type of change
Testing
Tested environments/configuration:
Automated tests:
e2e/specs/mock/scenarios/drawer-edge-theme.spec.ts: a definition that setsrgb-drawer-edgerepaints the drawer's 1px edge in light and in dark.mobile-drawer-controls.spec.tsboundary scenarios (default and ClickHouse, light and dark) still hold. The light case now checks that the edge paints exactly the drawer fill instead of checking that no border exists.registry.spec.ts: a theme that sets onlysurface-primary-altandborder-xheavykeeps the drawer edge on the fill in light and the heavy border in dark, and an explicitrgb-drawer-edgewins.MultiSelect.spec.tsx: the popover keeps the shared menu surface by default and takes the widget surface withsurface="widget".packages/clienttheme suites: 505 passed.clientjest--findRelatedTestson the four touched components: 6579 passed.npx tsc --noEmitinclient,packages/clientandpackages/data-provider: clean. The new fallback is typedPartial<IThemeRGB>:resolveThemespreads every role fallback into one literal, and an untyped one pushed a fullpackages/clientcheck past CI's 6 GB heap; typed, it peaks at about 4 GB. ESLint, Prettier, import sort and the design-rule suppression check pass.Screenshots / recordings
Captured on this branch (mock harness, Chromium, 390px). Dark mode draws the same edge as dev; in light the edge is the drawer's own fill, so the drawer looks as it did.
The source caption, prompt panel backdrop and Insights popover are not captured: the caption needs web search results and the backdrop the advanced prompt editor on a phone, neither of which the mock harness serves. Their changes are described under Risk.
Risk / compatibility
Not every default stays pixel-identical, and these are deliberate:
PromptForm.tsxalso carries Prettier's class-order fix-ups on lines dev had left unformatted, which the commit hook applies. #16611 edits the drawer class line directly above the edge line this PR changes, so whichever merges second resolves a one-line conflict.Checklist