Skip to content

fix(ui): use runtime :root color vars in shimmer gradient - #434

Open
Zerlight wants to merge 1 commit into
ruocheng/code-525from
ruocheng/code-575
Open

fix(ui): use runtime :root color vars in shimmer gradient#434
Zerlight wants to merge 1 commit into
ruocheng/code-525from
ruocheng/code-575

Conversation

@Zerlight

@Zerlight Zerlight commented Aug 8, 2026

Copy link
Copy Markdown
Member

Summary

The chat Shimmer gradient referenced var(--color-background) / var(--color-muted-foreground) — Tailwind v4 @theme inline names that never exist as runtime custom properties, so the whole background-image was invalid at computed-value time and the shimmer silently rendered with no colors in both themes. Switch to the :root/.dark vars coss-ui actually defines (--background, --muted-foreground), matching the existing color-mix(in oklab, var(--foreground) …) pattern in packages/presentation/ui/src/styles.css.

Swept packages/presentation/ui, packages/client/workbench, and both app renderers for other var(--color-*) usages — this was the only one.

Closes CODE-575

Verification

  • dev:mock webview, on a live Shimmer ("Running a command" during the showcase's streaming playback): computed background-image resolves to a white sweep over a muted-gray fill in light, and a near-black sweep over a lighter muted fill with the dark class toggled. Before the fix the computed value was none.
  • pnpm format:check, pnpm lint, pnpm typecheck clean; vitest run 2722 passed. The single git-mutations.test.ts failure is the signing-machine environment (production git commit path hits headless gpg pinentry) — the file passes 5/5 with GIT_CONFIG_GLOBAL=/dev/null.

Checklist

  • pnpm check:ci and pnpm test both pass (plus cargo fmt / clippy / test for Rust changes)
  • I ran the affected surface and observed the change working
  • If a wire message changed: WIRE_PROTOCOL_VERSION is bumped
  • New code and assets are my own work, or their origin and license compatibility are noted above
  • Docs and comments are updated where behavior changed

Copilot AI lite review requested due to automatic review settings August 8, 2026 14:20
@linear-code

linear-code Bot commented Aug 8, 2026

Copy link
Copy Markdown

CODE-575

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes the chat Shimmer text gradient by replacing references to Tailwind v4 @theme inline variables (--color-*, not available as runtime custom properties) with coss-ui’s runtime :root/.dark CSS variables (--background, --muted-foreground), ensuring the background-image computes correctly in both themes.

Changes:

  • Update Shimmer’s backgroundImage to use var(--background) and var(--muted-foreground) so the gradient no longer resolves to none at computed-value time.
  • Add an inline comment documenting why --color-* variables are not safe to use at runtime.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ The fix is correct and I verified the premise independently. One comment-accuracy nit inline, plus a stacking question below.

Reviewed changes — the single commit 8f7e4fb0, one file.

  • Shimmer gradient reads the :root varspackages/presentation/ui/src/chat/shimmer.tsx swaps var(--color-background) / var(--color-muted-foreground) for var(--background) / var(--muted-foreground) in the inline style.backgroundImage, and adds a one-line trap comment.

I confirmed the premise rather than taking it on faith, by compiling Tailwind 4.3.3 through its compile() API against a minimal repro of coss-ui's @theme inline block. The A/B was decisive: with no candidates the alias is not emitted; with a bg-muted utility candidate it is still not emitted (the value is inlined); only an arbitrary-value candidate like [background:var(--color-muted)] forces it into :root. A var(--color-*) written inside a JS inline style string is never a scanned candidate, so nothing forced --color-background to be emitted — the declaration was invalid at computed-value time and background-image fell back to none.

Worth recording that the blast radius was larger than "rendered with no colors": because the component itself sets bg-clip-text text-transparent, a background-image of none leaves nothing to clip, so every in-progress label was fully invisibletool.tsx:85, reasoning.tsx:92, activity-run.tsx:131,138, compaction-marker.tsx:42, terminal.tsx:46 all sit behind an isStreaming ? <Shimmer>x</Shimmer> : x ternary, so the text vanished while running and popped in on completion. Only the reduceMotion early-return branch was spared.

I also reproduced the sweep: var(--color- has no remaining non-vendor hits. The packages/vendor/coss-ui hits are correctly out of scope — --color-white / --color-zinc-* come from Tailwind's non-inline default @theme and are emitted normally, and skeleton.tsx:14's arbitrary value does force emission.

ℹ️ A standalone user-visible fix is gated behind an unmerged branch

The base is ruocheng/code-525, not master, and master still carries the broken string. Every streaming label in chat is invisible until this lands, so stacking it behind an unrelated agent-adapter branch delays a fix that has no dependency on that work.

Technical details
# Shimmer fix is stacked behind `ruocheng/code-525`

## Affected sites
- PR base ref — `ruocheng/code-525` (`a730f54f`), whose recent commits are opencode/MCP
  agent-adapter work unrelated to the shimmer gradient.
- `packages/presentation/ui/src/chat/shimmer.tsx` on `origin/master` — still has
  `var(--color-background)` / `var(--color-muted-foreground)`.

## Required outcome
- A conscious decision about whether this one-line fix should reach `master` on its own
  schedule rather than inheriting the parent branch's merge timeline.

## Open questions for the human
- Does this change actually depend on anything in `ruocheng/code-525`? If not, retargeting
  the base to `master` makes it mergeable immediately.
- If the stack is intentional (e.g. CODE-575 is scoped as a follow-up to CODE-525), no
  action needed — this is purely a sequencing observation.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread packages/presentation/ui/src/chat/shimmer.tsx Outdated
lucas77778
lucas77778 previously approved these changes Aug 8, 2026

@lucas77778 lucas77778 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking findings. Independently verified the Tailwind 4.3.3 behavior: @theme inline utilities inline var(--background) / var(--muted-foreground) and do not emit the corresponding --color-* runtime declarations for this JS inline-style usage. Also confirmed coss-ui defines both replacement variables in :root and .dark, and both renderer entries import the shared stylesheet.

Checked the complete one-commit diff, all Shimmer call sites, reduced-motion behavior, applicable repository/frontend guidance, and current CI results. git diff --check is clean and all required GitHub checks are green. The non-master base affects sequencing only; it does not introduce a correctness dependency in this change.

Copilot AI review requested due to automatic review settings August 9, 2026 11:37
@Zerlight
Zerlight force-pushed the ruocheng/code-575 branch from 8f7e4fb to 612f5c6 Compare August 9, 2026 11:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found. The one finding from the previous review is fixed, and the gradient line itself is unchanged from the version I already verified.

Reviewed changes — the delta from 8f7e4fb0 to 612f5c66: a rebase onto a newer ruocheng/code-525 plus a reword of the trap comment. The backgroundImage string is byte-identical to the previously reviewed version.

  • Trap comment now states the real invariantpackages/presentation/ui/src/chat/shimmer.tsx:30-31 replaces "--color-* are @theme-inline-only (no runtime custom properties)" with wording scoped to coss-ui's semantic @theme inline aliases, naming the actual rule: Tailwind never scans a JS style string, so nothing forces those aliases onto :root. That distinguishes "not emitted here" from "never exists", which is what the old wording got wrong for Tailwind's non-inline default palette and for @theme inline aliases referenced by a scanned candidate.

One note on reading this delta, since it will matter again if the branch is rebased: git diff-tree 8f7e4fb0 612f5c66 returns roughly ten files (codex MCP slug extraction, opencode config-read catch, command-echo gating, tool-search headers). All of it is ruocheng/code-525 drift absorbed by the rebase, not work authored on this branch — the merge-base diff is still the single shimmer.tsx file, and that is what I reviewed.

The sequencing observation from the previous review stands unchanged and needs no action here: the base is still ruocheng/code-525 and master still carries the broken string, so every in-progress chat label stays invisible until the parent branch merges. Whether that is worth retargeting is the author's call, not a defect in this diff.

Pullfrog  | View workflow run | Using Claude Opus𝕏

@Zerlight

Zerlight commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

On sequencing: keeping the stack — the branch was cut on top of code-525's tree and the stack merges bottom-up, so this lands with its parents rather than retargeting master.

Copilot AI review requested due to automatic review settings August 9, 2026 11:57
@Zerlight
Zerlight force-pushed the ruocheng/code-575 branch from 612f5c6 to bc57dbe Compare August 9, 2026 11:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@Zerlight
Zerlight force-pushed the ruocheng/code-575 branch from bc57dbe to 3ed2768 Compare August 9, 2026 12:12
Copilot AI review requested due to automatic review settings August 9, 2026 12:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants