Skip to content

REOPEN-F7: a hidden sidebar icon button registers its tooltip when it first shows (stacked on #33112) - #33113

Merged
lawrencecchen merged 16 commits into
feat-cmux-nextfrom
fix/reopen-f7-lazy-button-tooltip
Nov 3, 2026
Merged

lawrencecchen merged 16 commits into
feat-cmux-nextfrom
fix/reopen-f7-lazy-button-tooltip

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Was stacked on #33112, which merged at 14:43Z; feat-cmux-next is now merged in (head 1845245).

REOPEN-F7. Profile 8 (cmux19, job 72f71795e8b896e2f54c5ad4) puts TooltipTracker.setWhole (a tracker in the tooltip table and a tracking area) at about 1 ms (profiled) of the window build. It ran at init for every row's hidden hover-only icon buttons, which cannot be hovered while hidden.

Fix: the tooltip's text and shortcut are recorded at once, as before; only the hover tracking area waits. NSView.defersThemedToolTipTracking (opt-in; only SidebarIconButton sets it, the other 127 themedToolTip users do not change) keeps the tracking area off while the view is hidden or has no window, and themedToolTipCanShow() adds it when the button shows (viewDidUnhide, viewDidMoveToWindow, viewDidMoveToSuperview). VoiceOver's help is set with the text.

Review at e2c9ba9 found a P1 in the first version (it deferred the whole registration): the Priority bell, whose shortcut showPriority sets while the sidebar may have no window, lost its "⌥⌘K" (SidebarIconMeaningTests.thePriorityBellNamesItsDotAndKeepsTheShortcutApart). Fixed at 2e8574d. No look change (Sidebar-Look: no visual change).

Gain

Gain: 108-112 fewer tooltip tracking areas per launch (the same 4 hidden icon buttons per workspace row; 24-25 rows plus 3 spare rows in the realistic launch). Count tests: SidebarIconButtonLazyTooltipTests red on base (steps c2a86828, 6270511b), green on head (step 0b2c0164). Time gain 1-4 ms main-thread CPU, under the bench's noise (measured together with #33112); no regression (A/B cmux21, 20 launches per arm). Accepted under the HOT PATH AUDIT rule (handoff/hot-path-audit/README.md item 3).

Testing

Review

Review subagent: SHIP at 9c4e4b7 (comment 6601489848), after its P1 at e2c9ba9 was fixed. After #33112 merged, feat was merged in (conflict in SidebarIconButton.swift only, the #33112 lines next to this PR's): SHIP at 1845245 (comment 6607407510); the 3 changed files are byte-identical to 9c4e4b7.

Observed / Not run

  • Observed: the fleet runs above.
  • Not run: no hover check on a live app. The tooltip shows through the same tracker after the button shows; the test covers the registration.

Changelog

none

…OPEN-F7, red)

A reopened window's sidebar makes 5 icon buttons per workspace row,
hidden until hover, and each made its icon image at init: about 4 ms of
the profiled window build before the first frame (profile 8,
SidebarIconButton.renderIcon). These tests fail at the base: a hidden
button, or one in a hidden row, already has its image.
…t at init (REOPEN-F7)

Each workspace row makes 5 icon buttons (close, audio, pin, tree fold,
more) that stay hidden until the pointer is on the row; each made its
template image (an NSImage with a bitmap per scale) at init. Profile 8
(cmux19, job 72f71795): SidebarIconButton.renderIcon about 4 ms and the
buttons' own layout 2 ms of the window build before the first frame.

renderIcon now returns while the button is hidden or in a hidden row.
It runs when the button shows (viewDidUnhide, also sent for an
ancestor), joins a window, lays out, or is asked its intrinsic size. An
icon, label or symbol change on a hidden button draws when it shows.
The look does not change: a shown button has its image before it draws.
The shared glyph bitmaps (#32481) stay as they are.

Sidebar-Look: no visual change
…ws (REOPEN-F7, red)

Each workspace row's hover-only buttons registered their themed tooltip
(a tracker in the tooltip table, a tracking area) at init, before the
first window frame: about 1 ms profiled (profile 8). Fails at the base:
a hidden button already has its tracker.
…ws (REOPEN-F7)

Each workspace row's hover-only icon buttons put a tracker in the
tooltip table and a tracking area on themselves at init, before the
first window frame (TooltipTracker.setWhole, about 1 ms profiled in
profile 8), though a hidden button cannot be hovered.

applyToolTip registers the label as the themed tooltip once the button
shows (unhide, also of an ancestor, or joining a window). Until then it
sets only the VoiceOver help, so a hidden button still has its help.
A tooltip already registered follows a label change at once.

Sidebar-Look: no visual change
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 3, 2026
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 1845245: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr33113 -f ref=fix/reopen-f7-lazy-button-tooltip -f platform=macos -f backend_mode=local

web and size checks: green

Shipped in:

@blacksmith-sh

This comment has been minimized.

… placeholder

The green step 6bc3ff4c failed: a hidden button's image read as a
1 x 1 NSImage, the same object in every test. AppKit answers a button
that has no image of its own with a shared placeholder, so image == nil
never held. The test now counts an image as the icon only when it is
at least 2 points wide.

Sidebar-Look: no visual change
…wn state (green step 1844f51d)

aShownButtonHasItsImageInAWindowAndWithoutOne failed at d567840: a
button outside a window, asked its intrinsic size, kept AppKit's 1 x 1
placeholder image, because the check was image == nil and a button
with no image of its own answers that placeholder. A stack or a
constraint reading that size would size the button 1 x 1. The check is
now renderedSize == 0 (no icon made yet).

Sidebar-Look: no visual change
…tip reads back with no window (review P1, red)

Review of #33113 at e2c9ba9: showPriority sets the Priority bell's
themedToolTipShortcut on a sidebar that may have no window yet. With
the deferred registration, TooltipTracker.setWhole(nil, shortcut:)
returned early and the shortcut was lost (SidebarIconMeaningTests.
thePriorityBellNamesItsDotAndKeepsTheShortcutApart fails too). These
tests fail at e2c9ba9. The earlier tests now check the tooltip
tracking area, not the tracker table: the deferred part is the hover
tracking, not the text.

Sidebar-Look: no visual change
… only its tracking area (review P1)

Review of #33113 at e2c9ba9 (P1): showPriority sets the Priority
bell's themedToolTipShortcut on a sidebar that may have no window yet.
The deferred registration made TooltipTracker.setWhole(nil, shortcut:)
return early, so the bell lost its shortcut, and themedToolTip read nil
before the button showed (SidebarIconMeaningTests.thePriorityBell...).

Now the tracker always records the text and the shortcut. Only the
hover tracking area waits: NSView.defersThemedToolTipTracking (opt-in;
only SidebarIconButton sets it, the 127 other themedToolTip users do
not change) keeps the area off while the view is hidden or has no
window, and themedToolTipCanShow() adds it when the button shows
(viewDidUnhide, viewDidMoveToWindow, and now viewDidMoveToSuperview,
the review's P3: a move from a hidden to a shown superview in the same
window). VoiceOver's help is set with the text, as before.

Sidebar-Look: no visual change
…h no image (review P3, red)

Review of #33112 at bc142a2 (P3): the row's Unpin button was not
hidden at init, so it made its image when the row joined the window,
before the row's first layout hid it. Fails at bc142a2 on the pin.

Sidebar-Look: no visual change
It shows only on hover of an expanded pinned row (layoutPin through
setHoverShown, which unhides a hidden view the same way), and the row's
first layout hid it anyway. Hidden at init, it makes no image before
the first frame, as the close, audio and tree fold buttons.

Sidebar-Look: no visual change
@cursor

cursor Bot commented Nov 3, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (subagent opus): SHIP at 9c4e4b7

  • P1 fixed in every order: setWhole always stores text and shortcut, and each setter passes the other's current value. The cases that pass are: shortcut after text while hidden or with no window, a label change while hidden (the shortcut is kept), showsLabelToolTip off then on, and showPriority's order (unhide, then label, then shortcut). Only track(...) is gated.
  • Views that do not opt in keep the old behavior: TrackerSet.defers defaults to false, so the condition reduces to text != nil. defers(_:) reads the table and does not allocate. Regions ignore the flag (P3: say so in the doc).
  • Presenter is safe: current/inside only get a tracker through mouseEntered, so a tracker with no area cannot be shown. track(false) with no area does not reach the else if let area branch, so it never calls exit.
  • Nuance on (4): a shown-then-hidden button keeps its area unless its tooltip text changes or it moves while hidden. Then the area is removed (with exit) and added again on viewDidUnhide/viewDidMoveToWindow. This is harmless. Removal from a superview has window nil, so the new viewDidMoveToSuperview returns early. An ancestor unhide reaches the button through viewDidUnhide, and NSStackView re-attach reaches it through viewDidMoveToWindow.
  • Tests: the readback asserts were red against the first deferral and pass now. The !tracksTooltip asserts fail on REOPEN-F7: a hidden sidebar icon button makes its image when it first shows #33112's head, which tracks at init. No new stored static or global: one instance field on TrackerSet plus the existing trackers NSMapTable. No app_lines (CmuxNextApp) source changed. P3: the themedToolTipCanShow() doc lists two hooks, but the button uses three.
  • I did not compile or run anything. The fleet red and green results are the author's.

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 3, 2026
…f7-lazy-button-tooltip

# Conflicts:
#	Packages/macOS/CmuxNext/Sources/CmuxNextSidebar/Views/SidebarIconButton.swift
@lawrencecchen lawrencecchen removed the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 3, 2026
@lawrencecchen

lawrencecchen commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor Author

Review (subagent opus): SHIP at 1845245
Same reviewed change on the new base: the +/- lines of diff merge-base..184524511fb are identical to diff 8a8ba37..9c4e4b7 (3 files), and the 3 files at this head are byte-identical to 9c4e4b7.
Conflict resolution is right: feat's SidebarIconButton.swift at the merge base equals #33112's head 8a8ba37, git show --cc of the merge is empty (no hand-made lines), and init sets defersThemedToolTipTracking = true + applyToolTip(); themedToolTipCanShow() runs next to renderIcon() in viewDidMoveToWindow (window != nil), viewDidMoveToSuperview and viewDidUnhide.
No commit on feat since 8f963a6 (up to tip a34003d) touches CmuxNextDesign/Tooltip/ (ThemedToolTip.swift, TooltipPresenter.swift); #33112 changed only SidebarIconButton.swift, WorkspaceRowView.swift and its test. Not run: compile and tests (read-only review).

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 3, 2026
@lawrencecchen
lawrencecchen merged commit 63a8596 into feat-cmux-next Nov 3, 2026
215 of 225 checks passed
@lawrencecchen
lawrencecchen deleted the fix/reopen-f7-lazy-button-tooltip branch November 3, 2026 16:05
teamleaderleo added a commit that referenced this pull request Nov 3, 2026
…le (fixes base red from #33113) (#34033)

#33113 added super.viewDidMoveToSuperview in SidebarIconButton (an NSButton).
NSButton inherits it from NSView. Probed with check-objc-selectors.py --probe --write.


Agent-Session: https://cmux.com/deeplink/agent?id=aid-d750cf10922c785efa64

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 18452451 Deployed Nov 3, 2026 by lawrencecchen via wait for same-key tree publisher #106579
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant