🧩 fix: Paint Panel Surfaces From Theme Roles - #16484
Merged
Merged
Conversation
berry-13
changed the title
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. |
Contributor
|
Lighthouse CI failed. The last 80 log lines contain the measured budgets and assertion failures. |
![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: caa1c1a228
ℹ️ 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".
| @@ -62,13 +62,13 @@ export default function CategoryFilter({ options, value, onChange }: Props) { | |||
| size="default" | |||
| className={cn( | |||
| 'h-10 gap-1.5 px-3 text-sm font-normal', | |||
| isFiltered && 'border-emerald-500/50 text-text-primary', | |||
| isFiltered && 'text-text-primary', | |||
There was a problem hiding this comment.
When value !== 'all', this conditional adds text-text-primary, but the outline button variant already applies that exact class, so isFiltered now has no visual effect. The removed border was the only styling that distinguished an active category filter; replace it with an appropriate semantic variant or state treatment rather than leaving a no-op conditional.
AGENTS.md reference: AGENTS.md:L92-L97
Useful? React with 👍 / 👎.
Collaborator
Author
| @@ -582,7 +582,7 @@ const PromptForm = ({ promptId: promptIdProp }: { promptId?: string }) => { | |||
| <div | |||
| aria-hidden={!showSidePanel} | |||
| className={cn( | |||
| 'fixed inset-0 z-[100] bg-black/20 lg:hidden', | |||
| 'bg-surface-overlay/20 fixed inset-0 z-[100] lg:hidden', | |||
There was a problem hiding this comment.
On the default light theme, the old bg-black/20 produced a roughly #ccc backdrop over white, while surface-overlay is #595959, so applying the same 20% alpha produces roughly #dedede. Consequently, opening the mobile versions panel now dims the underlying page substantially less even though this is a token migration rather than an explicit redesign; adjust the opacity so the default appearance remains equivalent.
AGENTS.md reference: AGENTS.md:L42-L45
Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment.
Fixed in 5f4a198acb by keeping bg-black/20: no role reproduces the per-mode dim, so the scrim moves to the theme only once scrim opacity is a theme role (berry-13#181).
| @@ -11,8 +11,7 @@ import { isImageURL } from '~/utils/icons'; | |||
| /** Callers frame the mark at two thirds of the round container around it. */ | |||
| const artScale = 2 / 3; | |||
|
|
|||
| const entityAvatarClassName = | |||
| 'bg-surface-secondary h-full w-full rounded-full object-cover dark:bg-surface-tertiary'; | |||
| const entityAvatarClassName = 'bg-surface-tertiary h-full w-full rounded-full object-cover'; | |||
There was a problem hiding this comment.
For transparent agent or assistant avatars, and while those images are loading, this background remains visible. In the default light theme surface-secondary is #f7f7f8 while surface-tertiary is #ececec, so collapsing the former light/dark pair to surface-tertiary changes the existing light appearance; the same regression is introduced in AgentAvatarRender. Introduce an avatar surface role that retains the former per-mode values instead of applying the dark-mode surface in every mode.
AGENTS.md reference: AGENTS.md:L42-L45
Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment.
Fixed in 5f4a198acb by keeping the original per-mode pair in ConvoIcon and AgentAvatarRender. An avatar placeholder role needs style.css and themes/dark.ts, both edited by an open PR, so it is tracked in berry-13#194.
berry-13
force-pushed
the
berry-13/theme-leakage-panels
branch
from
September 29, 2026 06:47
caa1c1a to
faac690
Compare
Contributor
|
Lighthouse CI failed. The last 80 log lines contain the measured budgets and assertion failures. |
berry-13
force-pushed
the
berry-13/theme-leakage-panels
branch
from
September 29, 2026 07:20
faac690 to
539d27d
Compare
Contributor
|
Lighthouse CI failed. The last 80 log lines contain the measured budgets and assertion failures. |
berry-13
added this pull request to stack #16505
September 29, 2026 11:36
berry-13
force-pushed
the
berry-13/theme-leakage-panels
branch
from
September 29, 2026 14:14
539d27d to
52cda24
Compare
berry-13
force-pushed
the
berry-13/theme-leakage-panels
branch
from
September 29, 2026 15:28
52cda24 to
2f00a00
Compare
Contributor
|
Lighthouse CI failed. The last 80 log lines contain the measured budgets and assertion failures. |
5 tasks done
berry-13
force-pushed
the
berry-13/theme-leakage-panels
branch
2 times, most recently
from
September 29, 2026 17:00
5aa1f28 to
83ff571
Compare
The tool dialog's logo tile takes the fixed surface that logo tiles use elsewhere, and the skill image preview icon takes a series slot like the other categorical marks. The tools category filter drops a raw emerald border and shows its active state on the filter icon in the accent role instead; the label still names the active category. The avatar cropper's media hint and the SharePoint picker background are recorded in the theme allowlist.
berry-13
force-pushed
the
berry-13/theme-leakage-panels
branch
from
September 29, 2026 17:17
83ff571 to
890a477
Compare
6 tasks done
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
1 participant
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
Part of the theme-leakage stack; based on the chat link. The tool dialog header's logo tile takes
bg-surface-fixed, the fixed white tile the other logo tiles use, and the skill tree's image file icon takestext-series-5, a categorical slot like the file-source badges. The skills category filter showed its active state with a rawborder-emerald-500/50no theme could set; it now draws the filter icon intext-accent-primarywhile a category is active (text-text-tertiaryat rest), without restyling the Button primitive. The avatar cropper's move hint over the user's photo and the SharePoint picker's iframe background go into the allowlist.The prompt form's mobile scrim and the conversation and agent avatar placeholders stay as they are: no existing role reproduces them per mode, and adding one needs
client/src/style.cssandthemes/dark.ts, which an open PR edits. They are tracked in berry-13#181 and berry-13#194.Counts, before to after this link: raw palette utilities 56 to 53. Suppressions:
client/srcno-raw-colors 30 to 28, no-restyle 2054 to 2052; the rest unchanged. Theeslint-suppressions.jsondiff only touches entries for files this link edits.Type of change
Testing
Tested environments/configuration:
Automated tests:
reviewctl precheckagainst the chat link: static checks andjest-clientpass.@scenario:skills-category-filter-shows-active-state(new,e2e/specs/mock/scenarios/category-filter-state.spec.ts): with a categorized skill, the filter icon is the tertiary text colour at rest and the accent colour once the category is picked.npx tsc --noEmit(incremental) forclient: clean.npm run static-checks -- --against berry-13/theme-leakage-chat: all affected checks pass.Screenshots / recordings
Not captured: each change is a single class whose resolved colour is listed above. Visible differences from before: the skill image icon moves from pink-400 to the series magenta, and the active category filter shows an accent-coloured filter icon instead of a green border.
Risk / compatibility
None.
Checklist