Skip to content

fix(web): glass sidebar peek with a tail, off the stage edge - #273

Merged
NoahHendrickson merged 2 commits into
customfrom
t3/glass-sidebar-tail-offset
Nov 3, 2026
Merged

NoahHendrickson merged 2 commits into
customfrom
t3/glass-sidebar-tail-offset

Conversation

@NoahHendrickson

Copy link
Copy Markdown
Owner

Under Glass, the collapsed-sidebar peek card was an opaque #262626 slab. It lined up flush with the stage card's 8px edge, and nothing visually tied it to the toggle that opened it.

Glass background. The peek card now wears the docked sidebar's translucent glass tint. A CSS mask cuts the card and tail shape out of [data-slot="sidebar-inset"] while the peek is open, so only the window's native material shows behind it. This is the same idea as forkGlassPopupCutout.ts uses for popups. The peek gets its own arm in the docked tint rule, because narrowChatOverlay can mark a collapsed wrapper as overlaying, which would otherwise drop it back to #262626. The expanded narrow-chat overlay keeps its opaque fill.

Tail. An 18×8 tail points up at the floating toggle's glyph. The card's border and drop shadow moved from the container to a ::after, masked to leave a gap under the tail's base and none of the shadow under the tail. Without that, the border showed as a line across the join, and the shadow darkened the translucent tail. The inner panel now clips rows to the rounded corners, since the container has to let the tail out.

Offset. The card sits 14px in from the window's left instead of 8px, so it no longer aligns with the stage card's border. The bottom stays at 8px.

Verification: the fork guards (forkSidebarPeek, forkCoolDarkerSidebarVibrancy, forkGlassStageCard, forkGlassNewAgentStage) and vp run check:changed pass. The tail join, the hole, and the rule precedence were checked in Chromium against the real stylesheet, with colours matching across the join. Real Glass still needs a look in the desktop app.

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

Under Glass the collapsed-sidebar peek card was an opaque #262626 slab, it
sat flush with the stage card's 8px edge, and nothing tied it to the toggle
that opened it.

- Under Glass it now wears the docked panel's translucent tint, and a mask
  cuts the card and tail out of the sidebar inset beneath it, so only the
  window's material shows through (the popup cutout's approach). The peek
  gets its own arm in the tint rule, since narrowChatOverlay can flag a
  collapsed wrapper as overlaying.
- A tail points up at the floating toggle's glyph. The card's border and
  shadow moved to a ::after masked around the tail, so a translucent tail
  joins the card without a seam or a shadow showing through it.
- The card sits 14px in from the window's left instead of 8px.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. label Nov 3, 2026

@NoahHendrickson NoahHendrickson left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Code quality review of 19d9fc1245a77fcb954d692a088faf70183d8f5a

The behaviour looks deliberate and the cutout idea is sound. The way it's built has three structural problems, and one of them already shows up as a visible inconsistency. I'm not approving yet.

1. "Peek looks docked" is now decided by specificity, and it's written differently in each file

theme.custom.palettes.css:1243-1248 adds a second selector arm for the peek. Its own comment says it wins because "its extra attributes outrank the floating fill below". The floating rule (:1322-1324) still matches the same element whenever narrowChatOverlay has stamped data-fork-sidebar-overlay="true" on the collapsed wrapper. So which fill you get depends on one attribute selector's worth of specificity. Before this PR the two rules were mutually exclusive through :not() / :is(). Now they aren't.

Because the condition is rebuilt by hand in each place, the files have already drifted apart. theme.custom.glass-surfaces.css:682 lost its peek exclusion but didn't gain the peek arm. On a narrow window with the panel collapsed and peeking, the card gets the docked glass tint from palettes. The active-row rgb(255 255 255 / 6%) highlight that goes with that tint does not apply, because the wrapper is overlay="true". Palettes treats the card as docked and glass-surfaces treats it as floating.

The guard test then pins the fragile form. forkSidebarPeek.test.ts:87 asserts the floating selector does not mention data-fork-sidebar-peek, so the explicit exclusion that would fix this is forbidden.

Suggested fix: write one state predicate and make the two states exclusive. Docked becomes [data-slot="sidebar-wrapper"]:is(:not([data-fork-sidebar-overlay="true"]), [data-sidebar-state="collapsed"][data-fork-sidebar-peek="open"]). Floating becomes [data-fork-sidebar-overlay="true"]:not([data-sidebar-state="collapsed"][data-fork-sidebar-peek="open"]). Use the docked form in glass-surfaces too. Change the guard to assert that the two are exclusive, not that one string is missing.

There's a better option if it holds up. A collapsed panel never overlays anything, so narrowChatOverlay could stop stamping overlay="true" while the wrapper is collapsed. Then the peek needs no special arm in palettes or glass-surfaces at all, and the whole category disappears. Check the other overlay="true" consumers first (theme.custom.css:1314/1374, glass-stage :294-312). They all appear to pair it with expanded state already.

2. The tail triangle is drawn three times as hand-placed half-triangle gradient pairs

The same 18×8 isosceles triangle is rebuilt from two linear-gradient(to bottom right/left, transparent 50%, #000 50%) halves in three places, each with its own offset arithmetic:

  • ::before fill: 2 layers, plus 2 more for the edge, using calc(50% ± 0.25px / 0.75px / 1.25px) stop nudges (theme.custom.css:1516-1534)
  • ::after mask, layers 3–4 (:1488-1503)
  • the inset hole, layers 3–4 (:1541-1560)

That's about 12 gradient layers, and nothing checks that the three copies describe the same shape. The tail size is a constant, so define the shape once as a token, e.g. --fork-peek-tail-shape: url("data:image/svg+xml,…<polygon points='0,8 9,0 18,8'/>…"). Use it as one mask layer in ::after and in the inset hole. The ::before tail becomes background: var(--sidebar) masked by the same token. That leaves only two things that are specific to the tail: the 1px gap rectangle in ::after, and the stroked edge. The half-pair mask-position arithmetic and the sub-pixel stop nudges go away.

Related: the geometry is still tied to another component's class names. + 1px mirrors AppSidebarLayout's ml-px, and + 3px + tail-height reverse-engineers the glyph box. The code did this before (+ 4px), but the tail makes the coupling tighter. If the layout publishes a --workspace-controls-center-x and --workspace-controls-bottom, these lines read as intent rather than arithmetic.

3. rx='12' in the hole silently duplicates --fork-peek-radius

theme.custom.css:1546 hardcodes rx='12' inside a data URL, because var() can't reach into url(). The card uses --fork-peek-radius: 12px (:1430). If someone changes the variable, the corners stop matching and the transcript shows through at the card's corners under Glass. No guard covers it. Either make the radius a literal in both places with one comment pointing at the other, or extend the existing guard to assert the two match. A custom property that only looks like the single source of truth is worse than either.

4. Move the feature out of a 2.8k-line stylesheet

theme.custom.css is already 2,881 lines. This PR adds about 125 lines of peek geometry, pseudo-elements and the Glass cutout, so the fork-sidebar-peek fence is now about 180 lines of one feature. The repo already splits theme files by surface (theme.custom.glass-stage.css, theme.custom.glass-surfaces.css). A theme.custom.sidebar-peek.css would keep the geometry variables, both pseudos and the inset hole together, and the manifest's files: entry becomes exact. Not a hard blocker, since the file was already past 1k, but this is the PR where that split is cheapest.


Verdict: not approving. #1 is a real inconsistency, and its fix (exclusive predicates, or ideally no overlay stamp while collapsed) deletes the special arm rather than moving it. #2 and #3 are code-judo simplifications that would roughly halve the new CSS and remove a silent drift hazard. #4 is a cheap decomposition worth doing while the block is fresh.

Reviewed by Claude Opus 5.5 via Claude Code.

@NoahHendrickson NoahHendrickson left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Reviewed commit 19d9fc1245a77fcb954d692a088faf70183d8f5a against base eb30739340305b897c884d4a925aa5307e19fc9b. One actionable finding below.

Validation: traced the desktop browser host and mini-player stacking, and reproduced the stacking regression in a standalone Chromium fixture using the new mask rule: hit testing reaches the z-49 control before opening the peek and the sibling z-48 webview afterward. git diff --check passes. The full application and guard tests were not run because this isolated worktree has no installed dependencies.

Comment thread apps/web/src/theme.custom.css Outdated
[data-slot="sidebar-inset"] {
--fork-peek-tail-x: calc(var(--fork-peek-left) + var(--fork-peek-tail-left));
--fork-peek-tail-top: calc(var(--fork-peek-top) - var(--fork-peek-tail-height));
mask-image:

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

[P2] Preserve the floating browser controls above the hosted webview

With a local desktop browser tab floating over the chat, opening the Glass sidebar peek applies this mask and creates a stacking context around the entire inset. ThreadPreviewMiniPlayer renders its drag handle, reopen/close controls, and resize handles inside that inset at z-index 49, while AppRoot renders ElectronBrowserHost outside it and the hosted webview at z-index 48. The new context traps those controls below the webview, so they disappear and clicks hit the page until the peek closes, even when the floating browser does not overlap the sidebar. A Chromium fixture using this rule reproduces the hit-target change from the z-49 control to the z-48 sibling. Please keep the cutout from isolating the mini-player chrome below its hosted surface, or coordinate the host and chrome stacking when the mask is active; increasing the descendant control z-index alone cannot escape this context.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 923e2f8. The cutout is skipped while [data-preview-mini-player] [data-browser-surface-slot] is present, which is the same exception the glass stage card uses for its isolation. In that state the peek gets the opaque floating fill instead of the translucent tint, so it never sits see-through over the transcript. The tint, the active-row wash and the hole all key off the same predicate.

@github-actions github-actions Bot added the size:L label Nov 3, 2026

@NoahHendrickson NoahHendrickson left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Code review of 19d9fc1245. I checked each finding against the source; four below. Nothing blocking. The ones that matter most are the hole appearing before the open animation finishes (Glass) and the tail missing the glyph off macOS.

Two more I looked at and left out: the mask's GPU cost (it only applies while the peek is hovered open) and comment length.

🤖 Generated with Claude Code

a stacking context, which keeps its z-indexed layers under the card. */
:root[data-fork="noahhendrickson-t3code"][data-fork-sidebar-vibrancy="true"].dark[data-fork-theme="cool-darker"]
[data-slot="sidebar-wrapper"][data-sidebar-state="collapsed"][data-fork-sidebar-peek="open"]
[data-slot="sidebar-inset"] {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Under Glass, the hole shows up before the card does. The inset mask switches on at full strength when data-fork-sidebar-peek="open" lands. The container is still running fork-sidebar-peek-in at that point (120ms, opacity: 0 → 1, translateY(-4px) → 0) whenever data-panel-animations="true".

So every time the peek opens with animations on, the transcript disappears in the card-and-tail shape and leaves bare window material. The card then fades in over that gap, starting 4px above it, so the hole's edges and the tail cutout visibly fail to line up for the first ~120ms.

Possible fixes: drop the animation under the vibrancy selector, or animate the inset's mask in step with the card (for example, fade the subtracted layer's alpha over the same 120ms).

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 923e2f8. Under Glass the card now opens without fork-sidebar-peek-in, so the hole and the card land in the same frame. Other palettes keep the fade, since they have no hole.

Comment thread apps/web/src/theme.custom.css Outdated
(var(--workspace-topbar-height) + var(--workspace-titlebar-control-size)) / 2 + 3px +
var(--fork-peek-tail-height)
);
--fork-peek-tail-left: max(

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Off macOS, the tail misses the toggle glyph. The tail only lines up when --workspace-controls-left is the traffic-light inset. With the default 0.75rem (web, Windows, Linux, and macOS fullscreen), the inner calc gives 12 + 1 + 14 − 9 − 14 = 4px, so max() falls back to the 12px radius.

The tail then covers window x 26–44 with its apex at 35. The toggle glyph is centred at 12 + 1 + 14 = 27, so the apex is 8px off centre, right at the glyph's edge. That contradicts the header comment's "The tail centres on the floating toggle".

The clamp makes sense, since the tail can't sit inside the 12px corner. But the geometry needs a non-mac answer: for example, derive --fork-peek-left from the toggle so the tail can stay centred, or shrink the radius at the tail corner. Otherwise, at least correct the comment.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 923e2f8. Instead of clamping the tail, the card moves: --fork-peek-left: min(14px, toggle-x − radius − tail/2), and the tail is always toggle-x − tail/2 − left. In Chromium at controls-left 12px the card sits at 6px with the tail centred on 27, and at the mac inset it's still 14px with the tail on 100.

[data-slot="sidebar-wrapper"]:not([data-fork-sidebar-overlay="true"]):not(
[data-sidebar-state="collapsed"][data-fork-sidebar-peek="open"]
)
[data-slot="sidebar-wrapper"]:not([data-fork-sidebar-overlay="true"])

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Active rows render differently depending on the chat column's width. The palettes docked block now has a separate peek arm, so the peek keeps the tint when narrowChatOverlay flags the collapsed wrapper. This 6% rule didn't get a matching arm, though.

With Glass, a narrow chat column (data-fork-sidebar-overlay="true" on the collapsed wrapper) and the peek open, the active thread and chrome rows fall back to the docked block's --sidebar-row-active (8% white). The same card in a wide window gets 6%.

Adding the same [data-sidebar-state="collapsed"][data-fork-sidebar-peek="open"] arm here would make them match. This file would then belong back in the manifest's files list for fork-sidebar-peek.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 923e2f8. This rule now takes both arms of the docked predicate, so the active rows get 6% white whenever the tint applies. glass-surfaces is back in the manifest's files.

Comment thread apps/web/src/theme.custom.css Outdated
--fork-peek-tail-top: calc(var(--fork-peek-top) - var(--fork-peek-tail-height));
mask-image:
linear-gradient(#000 0 0),
url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg'%3E%3Crect width='100%25' height='100%25' rx='12'/%3E%3C/svg%3E"),

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Nit: rx='12' in the SVG copies --fork-peek-radius as a literal. If the radius var changes, the corners of the hole and the card stop matching, and the stage shows at the corners (or a gap opens up). The guard wouldn't catch it. A short comment tying the two together would help. So would a guard assertion that the SVG's rx equals the var's value.

Related: because the mask makes the inset a stacking context while the peek is open, the comment above the z-index 45 rule ("Raising the panel, rather than isolating the inset…") is no longer strictly true in that state. In practice it's harmless: the lightbox can't open while you're hovering the toggle. Still, a single clause there noting the exception would keep that comment accurate.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 923e2f8. Both shapes are one SVG token each (--fork-peek-card-shape, --fork-peek-tail-shape), and a new guard asserts rx matches --fork-peek-radius and the polygon matches the tail width and height. I also added a clause to the z-index 45 comment noting that the peek cutout makes the inset a stacking context while it's open.

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB +6 B (+0.1%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.3 KiB +6 B (+0.5%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 21.6 KiB 21.6 KiB 0 B (0.0%) 24.4 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.1 KiB 5.0 KiB −6 B (−0.1%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.3 KiB 1.3 KiB −6 B (−0.5%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.9 KiB 21.9 KiB 0 B (0.0%) 24.4 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: eb30739 · PR result: 923e2f8 · Source CI: success

Scenario and decoded snapshot size

10 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.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

- The docked Glass tint, its 6% active-row wash and the inset cutout share
  one predicate for the peek, and the opaque floating fill excludes it, so
  the two can no longer both match a collapsed wrapper narrowChatOverlay has
  flagged.
- No cutout while a desktop tab floats over the chat: the mask's stacking
  context trapped the tab's z-49 chrome under its z-48 webview. The card
  takes the opaque floating fill instead.
- Glass opens the card without the fade, so the hole never shows ahead of it.
- The tail centres on the toggle off macOS too: the card moves left where the
  toggle is near the window edge instead of clamping the tail.
- Card and tail cutouts are one SVG each; a guard ties their sizes to the
  geometry variables.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@NoahHendrickson

Copy link
Copy Markdown
Owner Author

Addressed in 923e2f8. CI on the previous head was green; the guards (45) and vp run check:changed pass on this one.

Fixed

  • Docked and floating overlap (review 1, fix(fork): tolerate trailing newline in CLAUDE.md symlink target #1). The docked tint has two exclusive arms: non-overlay non-peek, and a glass peek with no floating desktop tab. The opaque floating rule is :is(overlay, peek):not(glass peek). The 6% row wash and the inset cutout use the same peek predicate. In Chromium, all six combinations of overlay, peek and floating tab resolve to exactly one fill. The guard now asserts the shared predicate in all four rules instead of the absence of a string.
  • Floating browser chrome trapped under the webview. No cutout while a desktop tab floats, and the opaque fill in that state.
  • Hole showing before the card. No open animation under Glass.
  • Tail off macOS. The card moves left instead of clamping the tail.
  • Triangle drawn three times, rx='12' drift (feat(web): Sidebar V2 tab design, pixel-rain working indicator, Phosphor icons #2, docs(agents): add change scope guidance for agent-authored work #3). The two masks use one SVG token each, with a guard tying their sizes to the variables. The ::before keeps its gradient halves, because they draw the tail's edge stroke over its fill, and a mask can't colour those two separately.

Not changed

  • Stop narrowChatOverlay stamping while collapsed. That stamp is deliberate. syncSidebarOverlay counts collapsed as not occupying, so the attribute already says what opening the panel will do (narrowChatOverlay.ts:124-129). Dropping it would make the panel open pushed and then snap to overlay.
  • Publishing --workspace-controls-center-x from AppSidebarLayout. That widens the fenced upstream hunk for one consumer. The derivation is named once now (--fork-peek-toggle-x), and its comment points at the controls row.
  • Splitting into theme.custom.sidebar-peek.css (ci(fork): gate the Release workflow to upstream only #4). It's a pure move of an existing fence, plus a new import in main.tsx. Better as its own PR than mixed into the visual change.

@NoahHendrickson NoahHendrickson left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Re-review of 923e2f8169: approved.

All five findings from the three earlier reviews are fixed in 923e2f8, and I checked each one against the diff:

  • The docked tint arms, the opaque floating rule and the 6% row wash now share one peek predicate. The six overlay × peek × floating-tab combinations each resolve to exactly one fill.
  • No cutout while a desktop tab floats, so the mini-player chrome stays above the webview.
  • The Glass animation: none (0,9,0) outranks the fork-sidebar-peek-in rule (0,7,0), so the hole and the card land together.
  • --fork-peek-left: min(14px, …) keeps the tail on the toggle when --workspace-controls-left is 0.75rem, and the guard ties the SVG rx and polygon to the variables.

The 'not changed' items are reasonable. CI is green on this head, and the branch merges cleanly alongside #274.

🤖 Generated with Claude Code

@NoahHendrickson
NoahHendrickson merged commit beb19e0 into custom Nov 3, 2026
27 checks passed
@NoahHendrickson
NoahHendrickson deleted the t3/glass-sidebar-tail-offset branch November 3, 2026 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants