Skip to content

Disable titlebar page help tooltips #1038

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lawrencecchen wants to merge 1 commit into from
Closed

Disable titlebar page help tooltips #1038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 24 additions & 5 deletions Sources/ContentView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2020,6 +2020,13 @@ struct ContentView: View {
titlebarPageShortcutHintMonitor.isModifierPressed || alwaysShowShortcutHints
}

// AppKit tooltip tracking areas have been crashing during titlebar page churn
// (hover, drag/drop, close, and selection changes). Keep this strip tooltip-free
// until we have a safer non-AppKit tooltip implementation for it.
static func titlebarPageControlsShouldInstallHelpTooltips() -> Bool {
false
}

private func titlebarPageShortcutLabel(index: Int, pageCount: Int) -> String? {
commandPalettePageShortcutHint(index: index, pageCount: pageCount)
}
Expand Down Expand Up @@ -2196,7 +2203,8 @@ struct ContentView: View {
)

if isTitlebarHovered {
Button(action: {
let newPageLabel = String(localized: "workspace.page.new.tooltip", defaultValue: "New Page")
let newPageButton = Button(action: {
_ = workspace.newPage(select: true)
}) {
Image(systemName: "plus")
Expand All @@ -2209,9 +2217,15 @@ struct ContentView: View {
)
}
.buttonStyle(.plain)
.help(String(localized: "workspace.page.new.tooltip", defaultValue: "New Page"))
.accessibilityLabel(newPageLabel)
.accessibilityIdentifier("titlebarPageNewButton")
.transition(.opacity)

if Self.titlebarPageControlsShouldInstallHelpTooltips() {
newPageButton.help(newPageLabel)
} else {
newPageButton
}
}
}
.frame(maxWidth: .infinity, alignment: .leading)
Expand Down Expand Up @@ -2259,7 +2273,7 @@ struct ContentView: View {
let showTrailingDropIndicator = dragIndicator?.pageId == page.id && dragIndicator?.edge == .trailing

return ZStack(alignment: .trailing) {
Button(action: {
let pageButton = Button(action: {
workspace.selectPage(page.id)
}) {
HStack(spacing: 6) {
Expand All @@ -2279,12 +2293,17 @@ struct ContentView: View {
.background(
RoundedRectangle(cornerRadius: 6, style: .continuous)
.fill(fakeTitlebarTextColor.opacity(isActive ? 0.11 : (isHovered ? 0.06 : 0.001)))
)
)
}
.buttonStyle(.plain)
.help(page.title)
.accessibilityIdentifier(titlebarPageButtonAccessibilityIdentifier(pageId: page.id, isActive: isActive))

if Self.titlebarPageControlsShouldInstallHelpTooltips() {
pageButton.help(page.title)
} else {
pageButton
}

HStack(spacing: 4) {
if showsShortcutHint, let shortcutLabel {
titlebarPageShortcutHint(text: shortcutLabel)
Expand Down
9 changes: 9 additions & 0 deletions cmuxTests/WorkspaceContentViewVisibilityTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,15 @@ final class WorkspaceHandoffPolicyTests: XCTestCase {
}
}

final class TitlebarPageTooltipPolicyTests: XCTestCase {
func testTitlebarPageControlsNeverInstallAppKitHelpTooltips() {
XCTAssertFalse(
ContentView.titlebarPageControlsShouldInstallHelpTooltips(),
"Transient titlebar page controls must not register AppKit help tooltips."
)
}
}
Comment on lines +93 to +100

@coderabbitai coderabbitai Bot Mar 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

This guards the helper, not the tooltip behavior.

Line 96 only asserts that the policy method currently returns false, so this still passes if a future titlebar control forgets to honor that policy and installs .help(...) anyway. Please test through a small runtime seam closer to the rendered control/configuration so the assertion covers “no AppKit help tooltip gets installed” rather than the helper’s literal return value. As per coding guidelines, "Do not add tests that only verify source code text, method signatures, AST fragments, or grep-style patterns; tests must verify observable runtime behavior" and "If a behavior cannot be exercised end-to-end yet, add a small runtime seam or harness first, then test through that seam rather than testing implementation details."

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@cmuxTests/WorkspaceContentViewVisibilityTests.swift` around lines 93 - 100,
The test currently only asserts the policy method
ContentView.titlebarPageControlsShouldInstallHelpTooltips() returns false;
instead, render a titlebar control at runtime (via a small test seam or
test-only factory such as adding a TestTitlebarControl or a
ContentView.makeTitlebarControlForTesting() that produces the actual control
used in the titlebar) and assert no AppKit help tooltip gets registered on the
rendered view (e.g., verify the view and its subviews have no toolTip/helpTag
set and that addToolTip/removeToolTip were not applied) so the test checks
observable behavior rather than the policy return value.


@MainActor
final class WorkspacePageLifecycleTests: XCTestCase {
func testSwitchingPagesPreservesLivePanelIdentityAcrossDetachAndReattach() throws {
Expand Down
Loading