Fix clipped titlebar shortcut-hint tooltip and floor sidebar width to its extent#5045
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughRefactors hint pill measurement to use SF Rounded, computes the rightmost hint extent, centralizes hint layout constants, reserves content width from that extent to avoid clipping, updates view padding/wiring, and enforces a sidebar minimum width to preserve tooltip clearance. ChangesTitlebar Shortcut Hint Rendering
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 16 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (16 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
Greptile SummaryThis PR fixes rightmost titlebar shortcut-hint pill clipping by correcting the font used for width measurement (SF Rounded instead of the default system font) and by driving the accessory frame reservation from the
Confidence Score: 5/5Safe to merge — the font-measurement and planner-driven frame-reservation changes are correct and tightly scoped to the titlebar accessory layout. Both changed files make additive, well-contained layout fixes with no mutations to shared state or concurrency boundaries. The sidebar minimum-width floor is a computed property driven by a runtime measurement and degrades gracefully to the user-defined floor when the measurement is unavailable. The two comments flagged are a missing safety-net masksToBounds opt-out and a maintenance risk from duplicated interval logic — neither causes incorrect behavior today. Sources/Update/UpdateTitlebarAccessory.swift — the interval-building duplication between titlebarHintLayoutRightmostExtent and titlebarHintIntervals is worth watching on any future spacing or offset change. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[TitlebarControlsLayoutMetrics.contentSize] --> B{max}
B --> C[buttonReservation
outerLeadingPadding + groupPadding
+ buttonRowWidth + hintTrailingInset]
B --> D[hintReservation
hintLeadingPadding + rightmostExtent
+ hintShadowMargin]
D --> E[titlebarHintLayoutRightmostExtent]
E --> F[For each TitlebarShortcutHintActionSlot
guard !isUnbound AND command]
F --> G[titlebarHintPillWidth
SF Rounded font at iconSize-5]
F --> H[Compute rightEdge
groupPadding + buttonRightEdge
+ xOffset + safetyShift + baseXShift]
G --> I[ShortcutHintHorizontalPlanner
.assignRightEdges]
H --> I
I --> J[max of assigned right edges]
J --> D
Reviews (3): Last reviewed commit: "Floor sidebar width to the leading title..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
The titlebar shortcut-hint pills (⌘B, ⌘N, ⌘[, ⌘], etc.) live in an NSTitlebarAccessoryViewController whose width AppKit allocates from preferredContentSize and clips to. The rightmost pill was being cut off on its right edge. Measured root cause (window coordinates): the accessory ended at x=224 but the rightmost pill rendered to x=230, a 6pt overflow that AppKit clipped. The reason the reservation fell short is that the hint reservation and the actual hint layout were two independent computations. The reservation was a fixed inset past the button row, but the real layout runs through ShortcutHintHorizontalPlanner, which shifts the whole pill row right by ~20pt to keep the leftmost pill from crossing the leading edge (the pills are wider than the button pitch). The fixed inset never accounted for that shift. Fix: derive the accessory's reserved width from the planner's actual rightmost hint edge via a shared titlebarHintLayoutRightmostExtent(), so contentSize reserves max(buttonReservation, leadingPad + rightmostHintExtent + shadowMargin). The pill-width measurement (SF Rounded, matching ShortcutHintPill) is extracted to a shared titlebarHintPillWidth() used by both the view and the reservation, so the two can't drift. After the fix the accessory ends at x=234 with the pill at x=230 (4pt clearance), and the pills keep their positions under the buttons. No automated test: this is a pixel-level titlebar layout fix verified by measuring the rendered frames; a unit test would only re-derive the same geometry. Verified with the always-show-hints test path.
azooz2003-bit
force-pushed
the
feat-titlebar-shortcut-hint-clip
branch
from
e736e6e to
c065214
Compare
The default minimum sidebar width (216) is narrower than the rightmost titlebar shortcut-hint tooltip's right edge (~234 in window coordinates), so shrinking the sidebar to its minimum let the last tooltip spill past the divider into the content area. Make minimumSidebarWidth take the max of the user's setting and a tooltip-derived floor: titlebarLeadingInset + a small clearance gap. titlebarLeadingInset is already measured at runtime (TitlebarLeadingInsetReader) as the traffic-light inset plus the width of every leading titlebar accessory, and the controls accessory's width now reserves the shortcut-hint tooltip extent (from the prior commit), so this reads "everything in the accessory area plus the tooltip" with no new measurement. Falls back to the user floor when the inset isn't measured yet.
azooz2003-bit
changed the title
This was referenced May 31, 2026
hhsw2015
pushed a commit
to hhsw2015/cmux
that referenced
this pull request
Jun 1, 2026
- Add Send Ctrl-F to Terminal passthrough (manaflow-ai#5011, force-stop CC agents) - Fix sidebar worktree spawn worktree-setup-as-input bug (manaflow-ai#5032) - Add boundary-aware ranking layer for command-palette fuzzy search - Open extension browser as pane tab + polish (manaflow-ai#5053) - Move sidebar kind selection to titlebar menu, fix clipped tooltip and floor sidebar width (manaflow-ai#5045) - Extract CmuxFoundation package — modular refactor wave 1 (manaflow-ai#5055) - Center empty sidebar-extension state, fade host bottom edge (manaflow-ai#5057) - Align titlebar accessory hints (manaflow-ai#5059) - Restore sidebar minimum width (manaflow-ai#5062) Conflicts resolved: - cmux.xcodeproj/project.pbxproj: merge fork's CMUXSettingsCore + CMUXSessionDaemon package refs with upstream's new CmuxFoundation package reference and product dependency.
austinywang
added a commit
that referenced
this pull request
Jun 1, 2026
Branch had diverged behind main, which reworked the titlebar shortcut-hint layout (#5045 clipping fix, #5059 hint alignment with defaultTitlebarHintX 4->0) and shrank header chrome sizing. Resolution preserves the branch's intent (restored release sizes 24/15/17/8, sidebar.left symbol, 3-slot sidebar chrome button-count plumbing) while adopting main's newer hint-layout machinery: - WindowChromeMetrics: keep restored sizes; add main's titlebarControlsLeadingPadding. - TitlebarControlsHitRegions: keep sidebarChromeButtonCount/allTitlebarButtonCount (referenced widely by auto-merged code); adopt named outerLeadingPadding. - contentSize: combine main's two-reservation clipping fix with the branch's buttonCount/reservesShortcutHintOverflow threading; bound titlebarHintLayoutRightmostExtent to the active slot count for sidebar chrome. - Drop branch's superseded hint positioning (titlebarButtonRightEdge, hintRightSafetyShift) in favor of main's planner (hintInterval/titlebarHintPillWidth). - Update regression anchors to merged behavior (defaultTitlebarHintX=0): classicWithShortcutHints 186->172, sidebar 118->104, roomy fitted spacing 7->14. vendor/bonsplit working tree checked out to the merged pointer (adds isAudioMuted, forkConversation* APIs that Workspace.swift now uses). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 1, 2026
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
1 participant
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.
Two related fixes for the titlebar shortcut-hint tooltips.
1. Rightmost tooltip clipped against the accessory
The titlebar shortcut-hint pills (⌘B, ⌘N, ⌘[, ⌘], …) live in an
NSTitlebarAccessoryViewControllerwhose width AppKit allocates frompreferredContentSizeand clips to. Measured in window coordinates: the accessory ended at x=224 but the rightmost pill rendered to x=230, so AppKit clipped 6pt off ⌘].Root cause: the accessory's reserved width and the actual pill layout were two independent computations. The reservation was a fixed inset past the button row, but the real layout runs through
ShortcutHintHorizontalPlanner, which shifts the whole pill row right by ~20pt to keep the leftmost pill from crossing the leading edge (the pills are wider than the button pitch). The fixed inset never accounted for that shift.Fix: derive the reserved width from the planner's actual rightmost hint edge via a shared
titlebarHintLayoutRightmostExtent(), socontentSizereservesmax(buttonReservation, leadingPad + rightmostHintExtent + shadowMargin). The pill-width measurement (SF Rounded, matchingShortcutHintPill) is extracted to a sharedtitlebarHintPillWidth()used by both the view and the reservation so they can't drift. After the fix the accessory ends at x=234 with the pill at x=230 (4pt clearance), pills unchanged under the buttons.2. Sidebar minimum width floored to the accessory + tooltip extent
The default minimum sidebar width (216) is narrower than the rightmost tooltip's right edge (~234), so shrinking the sidebar to its minimum let the last tooltip spill past the divider into the content area.
minimumSidebarWidthnow takes the max of the user setting andtitlebarLeadingInset + clearance.titlebarLeadingInsetis already measured at runtime (traffic-light inset + every leading accessory's width), and the accessory width now reserves the tooltip extent from fix #1, so it reads "everything in the accessory area plus the tooltip" with no new measurement.Verified by measuring the rendered frames with the always-show-hints test path (
CMUX_UI_TEST_SHORTCUT_HINTS_ALWAYS_SHOW=1): accessory right edge moved 224 → 234, rightmost pill 230, clearance 4pt. No automated test (pixel-level titlebar layout; a unit test would only re-derive the same geometry).🤖 Generated with Claude Code
Summary by CodeRabbit