Repository navigation
Conversation
…variant in the composer Toggle now renders data-toggle after its props spread, as Button renders data-button, and the phone glyph rule in styles.css keys on it, so a Toggle's bare icon draws at 20px on phones without a hand-written size-4. The composer and side-thread dictation Toggles drop theirs and share one class on the ghost variant. The phone sheet's Mode and Model rows and Effort pills use the ghost variant instead of strings that undid the default variant's pressed fill; the rows inset the ghost focus ring so the scrolling sheet doesn't clip it. The model rows drop a dark pressed-hover token that never applied: the dark fill already sorts after hover. button.tsx notes that borrowing buttonVariants doesn't bring data-button. Closes #2380 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Member
|
Hi, a heads-up from the review: GitHub reports this PR as conflicting with Could you rebase and push when you have a moment? I've read this PR and expect to approve it, with only non-blocking notes (mostly that Fully or partially written by an AI agent. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to Daniel's non-blocking review notes on #2362 and #2368.
Togglenow rendersdata-toggle=""after its props spread, the same shape fix(studio): keep the phone icon size for Buttons inside asChild triggers #2368 gaveButtonwithdata-button.data-slot="toggle"keeps naming the outermost part, so anasChildtrigger can still replace it. The phone rule instyles.cssadds[data-toggle] svg.lucide:not([class*="size-"]):not([class*="h-"]):not([class*="w-"]), so a Toggle's bare icon draws at 20px on phones. The hand-writtensize-4on the mic icons inchat-composer.tsxandside-thread-panel.tsxis gone.ghostToggle variant (refactor(studio): use the Tabs and Toggle primitives in two hand-rolled controls #2334). The dictation Toggles, the phone sheet's Mode rows (toggleRowClass), Model rows (model-effort-menu.tsx) and Effort pills (effortPillClass) no longer carry tokens that only undid the default variant's pressed fill and ink (data-[state=on]:bg-transparent,data-[state=on]:text-current,data-[state=on]:hover:bg-muted/50on the rows,rounded-fullon the pills). The dictation Toggles also stop borrowingbuttonVariantsalong with theborder-0 bg-transparent shadow-noneoverrides it needed.features/chat/dictation-toggle.tsexportsdictationToggleClass, used by both the composer and the side thread. The two copies had drifted: only the composer's hadborder-0 bg-transparent shadow-none, which had no visible effect.button.tsxdocuments thedata-buttonboundary. An element that only borrowsbuttonVariantsclasses has nodata-button, so the phone glyph rule doesn't size its icons.The dark pressed-hover on the model rows (Daniel's question on #2362)
The comment in #2362 was right, and the token was a no-op. Before #2362, a chosen row had
bg-zinc-100 dark:bg-zinc-800. The dark variant (&:is(.dark *)) has the same specificity ashover:and sorts after it, so the dark pressed-hover fill was already zinc-800, and the light one wasmuted/50.dark:data-[state=on]:hover:bg-zinc-800never changed anything: removing it changes nothing in the browser (computed-style diff below), and the new browser pin passes with or without it. This PR removes it and rewrites the comment to say why the dark fill stays. Light pressed-hover staysmuted/50and dark pressed-hover stays zinc-800, and both are now pinned.Visible changes
The comparison covers every state, not only screenshots. A throwaway Playwright probe read the computed style of every changed control on
origin/main(9f08f63) and on this branch:background,color,border,radius,box-shadow,outline,padding,size,font weightandglyph size. It covered rest, hover and keyboard focus, pressed and not pressed, light and dark, at 1280px and 390px. 150 states were compared. Only two things differ:ring-2 ring-offset-2focus ring, the same ring as the dictation Toggle and every Button, instead of the browser's auto outline. On the full-width rows,focus-visible:ring-insetkeeps the ring inside the row, because the sheet's scroll container would otherwise clip it on the left and right. The plain-button rows in the same sheet (Mode, Model, Back, Add a file) keep the browser outline.Everything else is identical. That includes both dictation Toggles in every state (focus too), the rows and pills at rest, hover, pressed and pressed-hover, and the desktop Model/Effort menu, which has no Toggles.
Before | After
Before is
origin/main9f08f63; After is 914645b. Red boxes mark real pixel differences (a channel delta of 8 or more). Amber boxes mark the Toggles that were reviewed and are unchanged. The few 1 to 3 level antialias pixels on unrelated rounded borders (the starter pill and the side thread's model pill) are deterministic rasterisation noise and are not boxed.Composer: dictation Toggle, idle and listening
Side thread: dictation Toggle, idle and listening
Model and effort menu (desktop dropdown, phone sheet), and the phone Mode sheet
Keyboard focus in the phone sheet (red: the ghost focus ring)
Canvas Preview/Edit switch (red: 18px to 20px glyphs on phones)
Tests
New pins were written first and passed on
origin/main. They pass on this branch and fail under mutation.e2e/composer-toggles.spec.ts(real browser):mainthe canvas pin fails on phones (18px), which is the intended change.muted/50in light and the zinc fill kept in dark.toggle.test.tsx:data-toggleis present and survives aTooltipTrigger asChildwhosedata-slotwins.composer-toggles.pinning.test.tsx: the composer's and the side thread's dictation Toggles render the same class, carrydata-toggle, and their glyph has no size class of its own.fonts.test.ts: the 20px phone rule lists[data-toggle] svg.lucide:not([class*="size-"]).Mutation results. Each mutation was applied, the pins were run, and the code was restored:
[data-toggle]line from the phone rulefonts.test.tsfails; the e2e glyph pin fails on phonesdata-togglefromToggletoggle.test.tsxand the pinning test failvariant="ghost"muted/50muted/50Gates
task studio:check: lint, typecheck, test (web 1656, server 252) and generated-check, all green.task studio:browser: 150 passed, 40 skipped (the project-specific skips), 0 failed.task docswas not needed.Closes #2380
🤖 Generated with Claude Code