Repository navigation
fix(web): Glass sidebar quiet ink is white at 65% - #238
Conversation
In the Glass theme the sidebar's muted marks and labels were opaque grays, so they read flat against the wallpaper. Status section headers and caret, card hover actions, draft pencil, idle circle, recent-projects + marks, footer icons, Gray project icons and receded card titles now take rgb(255 255 255 / 65%) and tint with the glass. Other themes are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
NoahHendrickson
left a comment
There was a problem hiding this comment.
Reviewed commit 7f8a92b against base 47a31ba after fetching PR #238 and verifying the detached checkout.
No actionable findings in the five-file diff. I checked the Glass/v2 selector scope, muted-token inheritance, footer and project-icon color paths, and existing hover overrides.
Validation: git diff --check passed. The focused forkCoolDarkerTheme and forkCoolDarkerSidebarVibrancy tests could not start because this isolated worktree has no installed dependencies (ERR_MODULE_NOT_FOUND for vite-plus). No browser verification was performed.
Review only; no changes pushed and no approval submitted.
NoahHendrickson
left a comment
There was a problem hiding this comment.
Code review of 7f8a92b2d778a8b99a75136da46d73e33c52bd14.
The scoping holds up. Every new rule sits under [data-sidebar-version="v2"], which is only on the sidebar element (AppSidebarLayout.tsx:358), so popovers and the main pane are untouched. --color-muted-foreground is in @theme inline, so the descendant --contrast-muted-foreground override does reach text-/border-muted-foreground (including the idle mark's border). gray-400 really is used only by projectIconColors.ts (dark:text-gray-400), and the footer rule wins over the vibrancy block's frozen #8a8a8a because it sits on a descendant. The forkCoolDarkerTheme guard passes locally (9/9).
Two inline notes. Neither blocks the merge.
Claude Opus 5.5 via Claude Code in T3 Code.
| [data-fork-projects-row], | ||
| [data-fork-recent-projects], | ||
| [data-fork-project-section-header], | ||
| [data-fork-status-section-header], |
There was a problem hiding this comment.
Status section count lands at ~39%, not 65%. SidebarV2StatusSectionHeader.tsx:63 draws the count with text-muted-foreground/60. With the token now rgb(255 255 255 / 65%), Tailwind's color-mix(… 60%, transparent) multiplies that to about 39% white. That's the same multiply the draft pencil gets its own rule to avoid. The PR body lists "Status section headers (label, count, caret)" as white at 65%.
If the count is meant to stay a step quieter than the label, this is fine, but then the description and manifest overstate it. If it should match, it needs the same direct-color treatment as [data-fork-draft-mark] svg.
There was a problem hiding this comment.
Intended: the count stays a step fainter than its label, as it was in every other theme. I corrected the PR body and the manifest note so they no longer claim the count is at 65% (9151acb).
| :root[data-fork="noahhendrickson-t3code"].dark[data-fork-theme="cool-darker"] | ||
| [data-sidebar-version="v2"] | ||
| [data-slot="sidebar-footer"] { | ||
| --sidebar-icon-color: rgb(255 255 255 / 65%); |
There was a problem hiding this comment.
This reaches more than Settings and Pull Requests. Every SidebarMenuButton svg in the footer reads --sidebar-icon-color (ui/sidebar.tsx:694), and so does SidebarUpdatePill's idle state (SidebarUpdatePill.tsx:458). So the Usage item (SidebarChrome.tsx) and the update pill's idle icon also turn white at 65%. That's probably what you want, but the comment here, the manifest entry, and the PR body only name Settings and Pull Requests, so the next reader won't expect the other two to change.
There was a problem hiding this comment.
Right, and that's intended. The CSS comment, the manifest and the PR body now name the update pill's idle icon too (9151acb). Usage doesn't change: the v2 sidebar renders SidebarChromeFooter hideUsage (Sidebar.tsx:5380), so that item never mounts there.
NoahHendrickson
left a comment
There was a problem hiding this comment.
Code-quality review of 7f8a92b2d778a8b99a75136da46d73e33c52bd14.
The change is small and correctly fenced. The Sidebar.tsx hook is 3 fenced lines, the manifest watch list is updated, and no file crosses a size threshold. The problem is the shape of what it adds. One design concept, "Glass quiet ink is white at 65%", now lives in five separate mechanisms with five copies of the same literal, and the guard needs ad-hoc code to find one of them. Not approving. Findings, most structural first:
1. One concept, five copies of the literal. Name it once. rgb(255 255 255 / 65%) is now stated at theme.custom.palettes.css:248, :286, :295, :304 and :313, and the guard has five regexes pinning the same value. Retuning the ink (to 60% or 70%) now means editing ten places in two files, and missing one splits the panel into two inks with nothing to catch it. The model this PR is missing is a single panel token, e.g. --fork-glass-quiet-ink: rgb(255 255 255 / 65%), declared once in the panel block at L224. Each consumer then reads var(--fork-glass-quiet-ink): --contrast-muted-foreground, the pencil's color, --sidebar-icon-color, --color-gray-400 and --fork-sidebar-card-title-receded. The guard pins the value once and asserts that each consumer reads the var. The CSS stays the same size, the test gets simpler, and the invariant becomes one line instead of an agreement across five places.
A related question: the allowlist grew from 3 hooks to 7, plus 4 side rules. Is the panel's opaque #8a8a8a still the default, with white as the exception, or has the default flipped? A lot still reads the opaque ink (top chrome rows, the update capsule, compact marks), so the allowlist may still be right. Please state the policy in the block comment, so the next "this one too" PR knows which side to land on.
2. The --color-gray-400 rule repeats the panel block's exact selector, and the guard works around it. L308–314 is a second rule with the same selector as the panel block at L224. That is the only reason the guard drops ruleBodyFor for a bespoke themeRules.find(... body.includes("--color-gray-400")) (test L165). Move the declaration into the panel block next to --fork-sidebar-card-title-receded, and both the extra rule and the special-case lookup go away.
The override also rests on an invariant nobody checks: "gray-400 is only the Gray project colour's dark ink." It redefines a Tailwind palette variable for the whole v2 sidebar subtree. Any future text-/border-/bg-gray-400 there, including one arriving in an upstream sync of Sidebar.tsx, would silently turn translucent white. Today only projectIconColors.ts:12 uses it, so this works. Either guard the claim (assert gray-400 appears only in projectIconColors.ts) or hook the Gray icon directly instead of overriding a palette primitive.
3. The vibrancy block's doctrine comment now contradicts this PR. theme.custom.palettes.css:1215–1224 says --sidebar-icon-color is frozen opaque "so icons stay glass-independent", and that "anything else derived from the panel fill needs the same treatment". The new footer rule deliberately undoes that freeze for the footer, and the receded-title token moves from opaque to translucent. Both may be the right call, but a reader of the vibrancy block will take the footer rule for a regression and "fix" it back. Rewrite that comment to say the freeze guards against panel-alpha leakage (the color-mix with var(--sidebar)), and that a deliberate flat white wash is a separate, intended choice.
4. The draft-pencil rule reaches into child structure in a file the fork owns. [data-fork-draft-mark] svg (L290–296) depends on the mark's markup shape. SidebarV2StatusIndicator.tsx is fork-owned, so put the hook on the PenLineIcon itself and target it directly. Even better, with finding 1 in place this rule becomes color: var(--fork-glass-quiet-ink) on a named element, with no descendant combinator.
Not blocking: the guard's single it now asserts five unrelated mechanisms under a 100-character title. Once there is one token, splitting it into "value" and "consumers" cases would read better.
Claude Opus 5.5 via Claude Code in T3 Code.
| [data-fork-thread-card-actions], | ||
| [data-fork-idle-mark] | ||
| ) { | ||
| --contrast-muted-foreground: rgb(255 255 255 / 65%); |
There was a problem hiding this comment.
This literal is now stated at L248, L286, L295, L304 and L313, and the guard pins it with five regexes. Declare one --fork-glass-quiet-ink in the panel block and have each consumer read var(--fork-glass-quiet-ink). Then the value lives in one place in the CSS and one assertion in the guard. See finding 1 in the review body.
There was a problem hiding this comment.
Done in 361286a. --fork-glass-quiet-ink: rgb(255 255 255 / 65%) is declared once in the panel block, and every consumer (--contrast-muted-foreground, the pencil's color, --sidebar-icon-color, --color-gray-400, --fork-sidebar-card-title-receded) reads the var. The guard pins the value once and checks that each consumer reads it, split into a 'value' case and a 'consumers' case. The block comment now states the policy: the opaque #8a8a8a stays the default, and this is an allowlist.
| other sidebar text. */ | ||
| :root[data-fork="noahhendrickson-t3code"].dark[data-fork-theme="cool-darker"] | ||
| [data-sidebar-version="v2"] { | ||
| --color-gray-400: rgb(255 255 255 / 65%); |
There was a problem hiding this comment.
This is a second rule with exactly the panel block's selector (L224). Fold the declaration into that block. Separately, the invariant in the comment (only the Gray project colour uses gray-400) isn't enforced anywhere, and this override redefines a Tailwind palette primitive for the whole sidebar subtree. Either guard the claim or hook the Gray icon directly.
There was a problem hiding this comment.
Folded --color-gray-400 into the panel block (361286a). I kept the override instead of hooking the icon, because the Gray class is applied inside upstream's ProjectFavicon. Instead, a new guard case, 'keeps gray-400 to the Gray project colour', scans apps/web/src and fails if any non-test file other than projectIconColors.ts uses gray-400. An upstream sync that introduces one will trip it.
| /\s--sidebar-icon-color:\s*rgb\(255 255 255 \/ 65%\);/u, | ||
| ); | ||
| // Gray project icons ink from gray-400, which only that colour uses. | ||
| const grayRule = themeRules.find( |
There was a problem hiding this comment.
This bespoke find(... body.includes(...)) exists only because the CSS has two rules with the same selector. Once --color-gray-400 moves into the panel block, the existing ruleBodyFor(themeRules, DARKER_PANEL) assertion covers it and this special case can go.
There was a problem hiding this comment.
Gone. With --color-gray-400 in the panel block, the plain ruleBodyFor(themeRules, DARKER_PANEL) lookup covers it (361286a).
| [data-sidebar-version="v2"] | ||
| [data-fork-draft-mark] | ||
| svg { | ||
| color: rgb(255 255 255 / 65%); |
There was a problem hiding this comment.
[data-fork-draft-mark] svg depends on the mark's markup. SidebarV2StatusIndicator.tsx is fork-owned, so put the hook on the PenLineIcon and target it directly.
There was a problem hiding this comment.
Moved: data-fork-draft-pencil now sits on the PenLineIcon itself, and the rule targets it directly with color: var(--fork-glass-quiet-ink), with no descendant combinator (361286a). I also rewrote the vibrancy block's freeze comment (review point 3). It now says the freeze guards against accidental leaks of the panel's alpha, and that the flat white wash is a separate, intended choice.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
One --fork-glass-quiet-ink token in the Glass panel block; every consumer reads it. gray-400 moves into the panel block and a guard holds that only the Gray project colour uses it. The pencil hook sits on the icon itself, and the vibrancy block's freeze comment now separates the alpha-leak guard from the deliberate white wash. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
In the Glass theme, the sidebar's muted marks and labels were opaque grays (
#8a8a8a,#aeb2b6,gray-400), so they read flat against the wallpaper instead of tinting with the glass like the Projects row and card meta already did.These now take
rgb(255 255 255 / 65%)in Glass only:muted/60, about 39%)+marks--sidebar-icon-color; Usage is hidden in the v2 footer)--color-gray-400, which only that colour uses)--fork-sidebar-card-title-receded)Most join the existing derived-token rule in
theme.custom.palettes.css. The draft pencil gets its own rule because itsmuted/70would otherwise multiply the token down to ~45%. One fenceddata-fork-thread-card-actionshook was added inSidebar.tsx(now on the Glass entry's watch list). Hover still lifts everything to full foreground, and other themes are untouched. The Glass guard pins each rule and hook.Fork guards: 84 files / 625 tests pass.
Reviewed with sol-loop: 2 rounds with GPT-6.1-Sol on high. Sol verdict: approved
Claude Opus 5.5 via Claude Code in T3 Code.
🤖 Generated with Claude Code