Repository navigation
Improve dark-mode separators and composer contrast - #1711
Conversation
Co-authored-by: Codex <[email protected]> Signed-off-by: Arjun Mahanti <[email protected]>
Co-authored-by: Codex <[email protected]> Signed-off-by: Arjun Mahanti <[email protected]>
kalvinnchau
left a comment
kalvinnchau
left a comment
There was a problem hiding this comment.
🤖 Dark composer fill collisions remain at this commit. Please preserve distinct nested fills and cover inline code or recipient pills with a paint regression assertion. This is a comment review, not approval; actual-app validation covered the preceding commit only.
Co-authored-by: Codex <[email protected]> Signed-off-by: Arjun Mahanti <[email protected]>
|
loganj
left a comment
loganj
left a comment
There was a problem hiding this comment.
🤖 Larry and Rbot: an independent adversarial review of 6dad69e. Source review only; we did not run the app or inspect native rendering.
Approving. No must-fix findings. Both composer hosts (the message form, its read-only footer, and the shared Composer) opt into the same surface context, the token registry and Tailwind registration are updated, and no reader of the removed --affordance-inline-chip remains. The new paint test is the right kind of evidence for CSS inheritance. CI is green.
Two inline notes: one about how the wrapper override interacts with derived roles, and one small border overlap.
| interaction states. Both composer hosts opt into this shared surface context. */ | ||
| .dark [data-composer-surface], | ||
| :root[data-color-mode="dark"] [data-composer-surface] { | ||
| --affordance-subtle: var(--neutral-5); |
There was a problem hiding this comment.
Advice (not blocking). This retints upstream --affordance-subtle* on a wrapper. The Surface-aware interactions section of DESIGN.md says not to do that, and the composer paragraph you added now contradicts it.
The concrete risk is that derived roles were already resolved at :root and do not follow this override inside the composer. In dark mode:
--affordance-row-hover(and so--interaction-row-hover),--affordance-chromeand--affordance-accent(neutral accent) stay neutral-4, which is now the composer fill itself, so those states would paint invisibly on this surface.--interaction-selectedstays--affordance-selected(neutral-5), which is now the same as the rest fill.
I did not find a composer control that hits these today, so this is about the next one. A dedicated interaction-composer recipe that sets the --interaction-* aliases, plus one named role for chip/code fill, would keep the guide's rule intact and avoid the partial retint. If you keep this shape, please add the composer as a named exception in the Surface-aware interactions paragraph so the two parts of the guide agree.
| --text-link: var(--text-standard); | ||
| --affordance-link-hover: var(--affordance-subtle-hover); | ||
| --border-standard: var(--neutral-4); | ||
| --border-standard: var(--neutral-5); |
There was a problem hiding this comment.
Question. Dark border-standard is now neutral-5 (#333), the same value as --affordance-selected and --affordance-subtle-pressed. A bordered item that is selected or pressed loses its edge. Was that checked on bordered cards and choice tiles (.buzz-choice uses border-standard at rest)? Probably fine, since the selected tile switches to --interaction-border, but worth a glance.
* origin/main: (39 commits) fix(github): keep PR action menu trigger in place (#1736) fix(goose): preserve default-model focus during pending saves (#1495) test(fixtures): reduce polling waits and repeated signing (#1493) test(inbox): wait for conversation focus before filtering (#1491) fix(desktop): keep the main webview running while it is hidden (#1713) chore(release): version desktop previews from 0.7.0 (#1643) fix(agent-harness): remove the unavailable-tools line from the base prompt (#1715) fix(agents): keep View transcript visible in narrow Pi and goose activity panes (#1717) fix(github-plugin): prefix GitHub plugin tools with github_ (#1726) perf(agents): load Claude and Codex activity without scanning every session (#1674) Improve new DM previews, picker performance, and composer focus (#1664) Polish individual message reactions and emoji settings (#1628) Improve dark-mode separators and composer contrast (#1711) Make decision providers plugins, and move TypeSafe into one (#1353) fix(avatars): use static pack portraits in chat (#1670) fix(channels): read the open thread when returning to a channel (#1639) Add app-wide GIF shortcut and composer previews (#1558) Add PR failure/conflict actions and sidebar unlinking (#1599) Polish queued follow-up previews and controls (#1554) feat(conversation): make emoji and mentions built in (#1673) ... Signed-off-by: morgmart <[email protected]>
…-stop * origin/main: (132 commits) feat(workflows): add combined filters and recent sorting (block#1699) ci: smoke test the browser perf suite on pull requests (block#1662) feat(acp): share one ACP client and server pool across harnesses (block#1227) Preserve recipient picker rows and scroll position during paging (block#1739) Group matching emoji reactions at nine people (block#1740) Open channel Canvas from the channel menu in a side-pane tab (block#570) fix(github): keep PR action menu trigger in place (block#1736) fix(goose): preserve default-model focus during pending saves (block#1495) test(fixtures): reduce polling waits and repeated signing (block#1493) test(inbox): wait for conversation focus before filtering (block#1491) fix(desktop): keep the main webview running while it is hidden (block#1713) chore(release): version desktop previews from 0.7.0 (block#1643) fix(agent-harness): remove the unavailable-tools line from the base prompt (block#1715) fix(agents): keep View transcript visible in narrow Pi and goose activity panes (block#1717) fix(github-plugin): prefix GitHub plugin tools with github_ (block#1726) perf(agents): load Claude and Codex activity without scanning every session (block#1674) Improve new DM previews, picker performance, and composer focus (block#1664) Polish individual message reactions and emoji settings (block#1628) Improve dark-mode separators and composer contrast (block#1711) Make decision providers plugins, and move TypeSafe into one (block#1353) ... Signed-off-by: Joah Gerstenberg <[email protected]> # Conflicts: # src/bundled/voice-notes/VoiceCapture.tsx
Dark-mode separators were difficult to distinguish from panels, and the composer blended into its background. Standard dark borders now use neutral-5 (#333333); both composers use surface-composer with the former dark stroke color, neutral-4 (#232323), as their fill. Light-mode appearance and focus behavior remain unchanged.
Both composer hosts opt into one shared surface context. In dark mode, nested subtle fills use neutral-5, hover neutral-6, and pressed neutral-7. Reapplying the existing interaction-panel recipe on that surface keeps active formatting controls visible after pointer departure. Inline code, reference chips and recipient pills stay distinct from the composer. Controls outside the composer retain their existing styling; the chip-specific workaround is removed.
The shared token registry and design guide document the treatment. Date dividers and panel headers inherit border-standard without local overrides.
Validation at
6dad69ec118dea45996e321d1e5a52a4717e9566:Both review findings are addressed: distinct nested fills use the shared composer context, and interactive chips retain their standard paired rest/hover recipe.
Hosted CI is rerunning. Draft pending human acceptance. Native visual acceptance, enlarged-text and the full interaction-state matrix remain deferred. To try in dark mode: insert a mention and inline code, then enable Code/Bold formatting and move the pointer away; chip, code, recipient pill and active format should remain distinguishable. No editor, signing, persistence or delivery behavior changes are included.