🧽 fix: Paint Freed Feature Surfaces From Theme Roles - #16517
Conversation
The output panes of code execution, file authoring and file reads painted surface-chat in light and surface-primary-alt in dark. surface-code-body is that same pair in the default, dark and high-contrast themes, and a theme that styles its code blocks now styles these panes with them.
The server logo tile and the QR backdrop were bg-white. They take surface-fixed and surface-qr, both white in every bundled theme, so a theme can own them without the QR losing its quiet zone.
…ents The tool-approval edit field flagged invalid JSON with border-red-500, a raw colour the primitive's owner rule rejects in any semantic form. TextareaAutosize now draws its border in border-destructive when aria-invalid is set, the attribute the Skills description fields already pass. ToolApproval sets aria-invalid and points aria-describedby at the Invalid JSON message, so a screen reader announces the error as well.
…ours Adds the image preview's media overlays, the sandboxed artifact document's scrollbar and the Azure file-source gradient to the theme allowlist.
… Tile Scenarios for the code-body pane behind a file-authoring card, the announced invalid state of the tool-approval edit field, and the MCP OAuth QR backdrop staying white in both modes.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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. |
…Scenarios at Desktop Size The pane is read with a retrying assertion against the resolved role, since the card re-renders as highlighting lands. The approval agent and the MCP Settings panel are reached through side panels the mobile layout keeps in its drawer, so those two scenarios run at desktop size on the mobile project.
|
@codex review Please review the current PR head b90015c. State the exact reviewed commit and ignore findings that apply only to earlier heads. Purpose supplied by the requester: Scenario fixes since reviewed head 19a3eac: retrying output-pane colour assertion, desktop-size setup for the approval and OAuth scenarios |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Pull Request
Summary
Part of the held-file sweep from the theme-leakage stack (#16482, #16483, #16484): the feature files that open canary PRs no longer hold, fixed where an existing theme role reproduces today's default appearance. Tracks berry-13#195.
The output panes of code execution, file authoring and file reads painted
surface-chatin light andsurface-primary-altin dark. They now readsurface-code-body, which is that same pair in the default, dark and high-contrast themes; in ClickHouse light the panes take Click UI's codeblock background, since they are code output. The MCP OAuth dialog's server logo tile and QR backdrop werebg-white; they readsurface-fixedandsurface-qr, white in every bundled theme, so the QR keeps its quiet zone.The tool-approval edit field flagged invalid JSON with
border-red-500, and the primitive's owner rule rejects any semantic border colour onTextareaAutosize. The primitive now draws its border inborder-destructivewhenaria-invalidis set. ToolApproval setsaria-invalidand pointsaria-describedbyat the Invalid JSON message, so a screen reader announces the error. The Bookmark and Skills description fields already passaria-invalid, so they now show the same destructive border as every other invalid field in the app. The border moves from red-500 to the role's red-600 in light mode.The image preview's media overlays, the artifact iframe's scrollbar and the Azure chip gradient are recorded in
packages/client/src/theme/allowlist.mdas colours the theme must not own.Still held, and left for later links in this stack: the avatar placeholder and default-avatar roles (berry-13#194) need
registry.ts,typesand the bundled themes, which #16486 edits;client/src/style.csswaits for #16486; the stylesheet and brand-mark allow entries ineslint.config.mjswait for #16248;client/src/utils/files.tsand the file-type tiles wait for #15684.Type of change
Testing
Tested environments/configuration:
Automated tests:
client:npx jest ToolApproval(the invalid edit field isaria-invalidand described by the Invalid JSON message) and the tests related to every changed file.packages/client: tests related toTextareaAutosize.reviewctl verify:tool-output-pane-follows-code-body,tool-approval-invalid-json-is-announcedandmcp-oauth-qr-tile-stays-whiteundere2e/specs/mock/scenarios/, on desktop light, desktop dark and mobile.npx eslintwith suppressions pruned for the touched files only (ToolApproval loses itsno-raw-colorsentry and oneno-restyle),prettier --check,sort-imports --check, clienttsc.Screenshots / recordings
No visible change in the default theme for the output panes, the OAuth tiles or valid fields: the three scenarios assert the default role values (white panes and QR in light, 23 23 23 panes in dark) on desktop light, desktop dark and mobile. The one visible change is the invalid tool-argument border, red-500 to the destructive role (red-600) in light mode and unchanged in dark.
Risk / compatibility
TextareaAutosizegains anaria-invalidborder for every caller. The five callers that set the attribute are listed above; callers that never set it are unchanged.Checklist