Skip to content

Tighten Download for Mac divider padding - #7257

Merged
lawrencecchen merged 12 commits into
mainfrom
feat-download-btn-tuner
Jul 4, 2026
Merged

lawrencecchen merged 12 commits into
mainfrom
feat-download-btn-tuner

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Jul 3, 2026 •

Copy link
Copy Markdown
Contributor

Tighten the "Download for Mac" split-button divider spacing and fix a Safari-only hover jump. Layout only, no copy/behavior change.

What changed

  • Divider padding is now a per-size PILL_PADDING config applied as inline px (arbitrary Tailwind values like pr-[9px] did not resolve on the base-ui Menu.Trigger, so the caret rendered 0 right-padding). Values: default download 20/9, caret 7/11; sm download 12/7, caret 5/9.
  • Removed the hover CSS transitions (transition-colors on the zones, transition-opacity on the caret) and did not add a forced translateZ(0) GPU layer. Either one made WebKit composite the sub-pixel-positioned zones and snap them to the device-pixel grid on hover, so the label/caret visibly jumped (Safari only, worst at the small size). Instant hover keeps the repaint on the main thread and matches Chrome.

Testing

  • Verified computed padding on the live button matches the config: default 20px/9px download, 7px/11px caret; sm 12px/7px, 5px/9px.
  • Dogfooded both sizes at 1x in the cmux WebKit browser (same engine as Safari) against a live tuner; final values chosen there and baked into the config.
  • web-typecheck green; Vercel preview built.

Single file: web/app/[locale]/components/download-button.tsx.

Pull the split-button divider closer to the caret across both sizes.
Default: caret zone px-2.5 -> px-2, download zone pr-3 -> pr-[11px].
Small: caret zone px-2 -> pl-1.5 pr-2, download zone pr-2.5 -> pr-2.
Layout only; no copy or behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 3, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Jul 4, 2026 4:01pm
cmux-staging Building Building Preview, Comment Jul 4, 2026 4:01pm

@coderabbitai

coderabbitai Bot commented Jul 3, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

DownloadButton now accepts per-zone padding overrides, and a new debug page renders live previews with controls, derived class output, and clipboard copy support.

Changes

Download Button Padding Overrides

Layer / File(s) Summary
DownloadButton padding overrides
web/app/[locale]/components/download-button.tsx
The component props, inline styles, split-zone class strings, caret icon opacity classes, and render bindings are updated to apply optional left/right padding overrides.

DownloadButtonDebug Preview Page

Layer / File(s) Summary
DownloadButtonDebug preview page
web/app/[locale]/debug/page.tsx
The debug page defines padding helpers and state, generates derived class snippets, injects diagnostic CSS, and renders the preview grid with controls and copy output.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed PR only changes TSX files; no Swift files appear in the merge-base diff, so Swift actor-isolation rules are not applicable.
Cmux Swift Blocking Runtime ✅ Passed No Swift files changed; the PR only modifies a Next.js TSX debug page, so the Swift blocking-runtime rule is not applicable.
Cmux Browser Automation Off-Main ✅ Passed The diff only edits web/app/[locale]/debug/page.tsx; no Swift browser-socket routing or worker-policy code is changed.
Cmux Expensive Synchronous Load ✅ Passed PASS — This PR only changes a Next.js TSX debug page; no Swift files or agent-history loads were added or moved, so the Swift sync-load rule doesn’t apply.
Cmux Cache Substitution Correctness ✅ Passed The change only adds transient UI padding overrides/debug tuning; it does not replace any fresh read in a persistence/history/undo/snapshot path or trust a stale cache.
Cmux No Hacky Sleeps ✅ Passed The only new timeout is the /debug copy-label reset; it’s presentation timing, and the PR adds no new fixed sleeps/polling in production paths.
Cmux Algorithmic Complexity ✅ Passed Changed loops are over fixed tiny lists (2 sizes, 4 fields, 3 waitlist platforms), with no nested scans over scalable user data.
Cmux Swift Concurrency ✅ Passed Diff only changes a TSX debug page; no Swift code or concurrency patterns were introduced, so the Swift rule is not applicable.
Cmux Swift @Concurrent ✅ Passed Diff only changes a TSX debug page; no Swift files or @concurrent/nonisolated async sites were touched, so the Swift rule doesn’t apply.
Cmux Swift File And Package Boundaries ✅ Passed PR diff only changes two TSX files; no Swift/package-boundary files are touched, so the Swift boundary rule is not applicable.
Cmux Swiftpm Lockfiles ✅ Passed Only submodule gitlinks changed; ghostty and bonsplit sub-commits touch source files only, with no Package.swift, Package.resolved, .gitignore, or Xcode lockfile changes.
Cmux Swift Logging ✅ Passed This PR only changes a TSX debug page; no Swift production/runtime code or logging was added or modified.
Cmux User-Facing Error Privacy ✅ Passed Only submodule pointer bumps are present; the nested diff removes internal log.warn strings and adds no user-facing error, alert, output, or recovery copy.
Cmux Full Internationalization ✅ Passed Only layout/style changes plus a hidden /debug tuner; no production-facing copy or locale entries changed.
Cmux Swiftui State Layout ✅ Passed Diff is web TSX only; no Swift files or SwiftUI state/layout patterns were introduced.
Cmux Architecture Rethink ✅ Passed This PR only changes TypeScript/Next.js UI files, not Swift architecture, so the Swift rethink rule does not apply.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR only changes a web TSX debug page; no Swift window code or cmuxAuxiliaryWindowIdentifiers changes, so the auxiliary-window close-shortcut rule is not implicated.
Cmux Source Artifacts ✅ Passed Only changed path is a hand-written source page, web/app/[locale]/debug/page.tsx; no logs, temp dirs, build output, or other artifacts entered the diff.
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS: The PR diff only touches web/app TypeScript files; no production Swift Sources/ files are changed, so the rule is not applicable.
Cmux No Ambient Global State ✅ Passed The diff adds only TSX preview/component code; no new Swift global mutable state, singleton, or static-namespace API was introduced.
Title check ✅ Passed The title clearly summarizes the main change: tightening the Mac download divider padding.
Description check ✅ Passed The description covers the summary and testing well, but it omits the required demo video, review trigger block, and checklist.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-download-btn-tuner

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 3, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR tightens the "Download for Mac" split-button spacing and fixes a Safari-only hover jump by removing CSS transitions from the hover tint and caret opacity. Padding is moved from Tailwind arbitrary-value classes to a per-size PILL_PADDING constant applied as inline pixel values, ensuring odd-pixel values (7, 9, 11 px) render exactly in both browsers and on @base-ui-components/react Menu.Trigger.

  • Introduces a module-level PILL_PADDING config object holding downloadLeft/Right and caretLeft/Right px per size; these are applied as style props on the download link and caret trigger.
  • Removes transition-colors from both hover zones and transition-opacity from the caret SVG to prevent WebKit from promoting the sub-pixel-positioned zones into their own compositing layers (which caused a visible label/caret snap on hover in Safari).

Confidence Score: 5/5

Safe to merge — changes are purely visual padding adjustments and a deliberate transition removal to fix a Safari rendering bug, with no behavioral or copy changes.

The diff only touches horizontal padding values and removes CSS transition properties. The PILL_PADDING constant is a pure immutable config, inline styles are handled correctly (React appends px to unitless numeric length values), and the transition removal is intentional and well-documented to address a confirmed Safari compositing-layer bug. No new strings, no logic changes, no accessibility regressions.

No files require special attention.

Important Files Changed

Filename Overview
web/app/[locale]/components/download-button.tsx Layout-only spacing and transition changes; logic, copy, and accessibility are unchanged. The inline-style approach for padding is well-reasoned and correctly handled by React's automatic px unit appending.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[DownloadButton render] --> B{size === 'sm'?}
    B -- yes --> C[pad = PILL_PADDING.sm]
    B -- no --> D[pad = PILL_PADDING.default]
    C --> E[downloadStyle inline px]
    D --> E
    C --> F[caretStyle inline px]
    D --> F
    E --> G{onConfirmationPage?}
    G -- yes --> H[a href=macHref style=downloadStyle]
    G -- no --> I[Link href=macHref style=downloadStyle]
    H --> J[Divider div]
    I --> J
    J --> K[Menu.Trigger style=caretStyle - no transition-colors - no transition-opacity on caret SVG]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[DownloadButton render] --> B{size === 'sm'?}
    B -- yes --> C[pad = PILL_PADDING.sm]
    B -- no --> D[pad = PILL_PADDING.default]
    C --> E[downloadStyle inline px]
    D --> E
    C --> F[caretStyle inline px]
    D --> F
    E --> G{onConfirmationPage?}
    G -- yes --> H[a href=macHref style=downloadStyle]
    G -- no --> I[Link href=macHref style=downloadStyle]
    H --> J[Divider div]
    I --> J
    J --> K[Menu.Trigger style=caretStyle - no transition-colors - no transition-opacity on caret SVG]
Loading

Reviews (8): Last reviewed commit: "Remove /debug tuner and padOverride prop..." | Re-trigger Greptile

@cubic-dev-ai cubic-dev-ai 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 issues found across 1 file

Re-trigger cubic

lawrencecchen and others added 4 commits July 3, 2026 16:18
Give the rounded pill its own backing layer via [transform:translateZ(0)].
Safari does not clip the zones' `transition-colors` hover tint to the
`rounded-full` corners once that tint animates into its own compositing
layer, so on hover the tint leaked past the corners as square edges
(Chrome clips it correctly, so it only reproduced in Safari). Promoting the
clip container makes the zones composite inside its clip. Verified in Safari
with a static repro forcing the composited-child state: square corner leak
without the transform, clean rounded corners with it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Padding: shave 1px from each side of the divider per size (default
label->divider 11->10px, divider->caret 8->7px; small 8->7px, 6->5px).

Hover jump: drop the CSS transitions on the hover tint (zones) and the
caret opacity. Animating them promoted the sub-pixel-positioned zone/icon
into a WebKit compositing layer that snapped to the device-pixel grid, so
the label/caret visibly jumped on hover (Safari only, worst at the small
size). Instant hover states never promote, so the pill stays stable. The
pill keeps its translateZ(0) backing layer to guard the rounded-corner
clip of the tint.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove the `[transform:translateZ(0)]` from the pill. Forcing the pill onto
a GPU layer makes WebKit re-rasterize the sub-pixel-positioned zones on every
hover repaint and snap them to the device-pixel grid, which is the label/caret
"jump" on hover (Safari only, worst small). The layer was added for the
corner-tint clip, but that clip only broke while a CSS transition promoted the
child; with the transitions already removed, the child never gets its own
layer, so overflow-hidden clips correctly without the forced layer. Verified a
promoted zone snaps its pixels (compare AE>0) vs an unpromoted one.

Add an opt-in `padOverride` prop (inline padding, undefined in prod so
rendering is unchanged) and a /debug page that renders both sizes live, drives
their divider padding with sliders, and has diagnostic toggles (re-add
transition / force translateZ) that should reintroduce the jump, to confirm
the cause with a real pointer.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@web/app/`[locale]/debug/page.tsx:
- Around line 1-40: The debug tuner page in DownloadButtonDebug is exposed as a
production route because it has no runtime guard. Add a development-only gate at
the start of the page component (or equivalent route exclusion) so the `/debug`
route returns notFound() in production, and keep the existing DownloadButton
debug UI available only in non-production environments.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: aa5b7a4e-48de-42bc-baa1-c967d280d603

📥 Commits

Reviewing files that changed from the base of the PR and between 9604684 and fdf67fa.

📒 Files selected for processing (2)
  • web/app/[locale]/components/download-button.tsx
  • web/app/[locale]/debug/page.tsx

Comment thread web/app/[locale]/debug/page.tsx Outdated
lawrencecchen and others added 3 commits July 3, 2026 17:51
default: pl-5 pr-2 / caret pl-1.5 pr-[7px]  (20/8, 6/7)
sm:      pl-3 pr-[7px] / caret pl-[5px] pr-[7px]  (12/7, 5/7)
Sync /debug initial values to match.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The tuned values include odd px (5,7) that map to arbitrary Tailwind classes
like pr-[7px]. That class silently did not resolve on the base-ui Menu.Trigger
button, so the caret rendered padding-right:0 and the live button did not match
the /debug tuner (which uses inline px via padOverride). Move padding into a
per-size PILL_PADDING config applied as inline styles on both zones, so the
exact values render identically in dev, prod, and /debug. padOverride now
merges over the config.

default: download 20/8, caret 6/7   sm: download 12/7, caret 5/7

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Default to 1x zoom and dark bg, and render each size next to its real
neighbors (View on GitHub for default, Docs/Blog nav for sm) so padding is
tuned at true on-screen scale.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lawrencecchen and others added 4 commits July 4, 2026 02:35
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
default: download 20/9, caret 7/11   sm: download 12/7, caret 5/9

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Tuning is done and the values live in PILL_PADDING. Drop the dev-only /debug
page and the padOverride prop so production ships just the download button
changes (padding config + hover-jump fix), no public /debug route.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lawrencecchen
lawrencecchen merged commit 65f3346 into main Jul 4, 2026
26 of 28 checks passed
@lawrencecchen
lawrencecchen deleted the feat-download-btn-tuner branch July 4, 2026 11:30
azooz2003-bit added a commit that referenced this pull request Jul 4, 2026
…ng, themes)

Delta: #7318/#7319 mouse cursor-shape + right/middle drag forwarding, #7320 ghostty
upstream, #6664 iOS themes, #7196 iOS scroll, #7257/#7307 web, #7329 CLI suggestions,
#7222 codex resume update-suppression. Conflicts resolved keeping HEAD structure:
- GhosttyTerminalView.swift: took main's viewDidEndLiveResize + resetCursorRects
  (cursor-shape #7318; ghosttyMouseShape/ghosttyMouseCursor already in HEAD).
- Mobile/MobileHostService.swift: union (kept HEAD sharedRequestActivity +
  main publicStatusPayload theme payload #6664).
- project.pbxproj: union-dedup, normalized; test-wiring green.
- swift-file-length-budget.tsv regenerated.

#7222 (codex update-check suppression) partially landed: the CMUXAgentLaunch package
part (AgentResumeArgv.codexUpdateCheckSuppressionOverride) auto-merged and is present;
the app-target files (+CodexUpdateCheck.swift + test) extend SurfaceResumeCommandCanonicalizer,
a type HEAD dissolved into the CmuxWorkspaces package, so they can't compile and the
suppression was never wired (call site was in the HEAD-deleted +PortableAgentExecutable).
Removed those two files; re-wire recorded in merge-deferred-gaps.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

This branch was successfully deployed

1 active deployment
Preview – cmux — c464b6d8 Deployed Jul 4, 2026 by vercel[bot]
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.

1 participant