From 7bc3f74cabe4116de230092af2084a4872928f60 Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Wed, 7 May 2025 14:33:13 +0200 Subject: [PATCH] Fix panel button context menu overlap with tooltip hint (#30108) This fix works by disabling the tooltip whenever the menu is being rendered. ### Before ![image](https://github.com/user-attachments/assets/0b275931-fd1f-4748-88dd-cb76a52f8810) ### After ![image](https://github.com/user-attachments/assets/7260eb48-3a24-43ee-8df7-b6c48e8a4024) Release Notes: - Fix panel button tooltip overlapping with the panel button's right click menu --- crates/ui/src/components/right_click_menu.rs | 10 +- .../ui/src/components/stories/context_menu.rs | 8 +- crates/workspace/src/dock.rs | 12 +- crates/workspace/src/pane.rs | 438 +++++++++--------- 4 files changed, 239 insertions(+), 229 deletions(-) diff --git a/crates/ui/src/components/right_click_menu.rs b/crates/ui/src/components/right_click_menu.rs index 72a907e2b0..bea79972e3 100644 --- a/crates/ui/src/components/right_click_menu.rs +++ b/crates/ui/src/components/right_click_menu.rs @@ -21,8 +21,14 @@ impl RightClickMenu { self } - pub fn trigger(mut self, e: E) -> Self { - self.child_builder = Some(Box::new(move |_| e.into_any_element())); + pub fn trigger(mut self, e: F) -> Self + where + F: FnOnce(bool) -> E + 'static, + E: IntoElement + 'static, + { + self.child_builder = Some(Box::new(move |is_menu_active| { + e(is_menu_active).into_any_element() + })); self } diff --git a/crates/ui/src/components/stories/context_menu.rs b/crates/ui/src/components/stories/context_menu.rs index 3a04e2449e..ee8a73ce0b 100644 --- a/crates/ui/src/components/stories/context_menu.rs +++ b/crates/ui/src/components/stories/context_menu.rs @@ -47,12 +47,12 @@ impl Render for ContextMenuStory { .justify_between() .child( right_click_menu("test2") - .trigger(Label::new("TOP LEFT")) + .trigger(|_| Label::new("TOP LEFT")) .menu(move |window, cx| build_menu(window, cx, "top left")), ) .child( right_click_menu("test1") - .trigger(Label::new("BOTTOM LEFT")) + .trigger(|_| Label::new("BOTTOM LEFT")) .anchor(Corner::BottomLeft) .attach(Corner::TopLeft) .menu(move |window, cx| build_menu(window, cx, "bottom left")), @@ -65,13 +65,13 @@ impl Render for ContextMenuStory { .justify_between() .child( right_click_menu("test3") - .trigger(Label::new("TOP RIGHT")) + .trigger(|_| Label::new("TOP RIGHT")) .anchor(Corner::TopRight) .menu(move |window, cx| build_menu(window, cx, "top right")), ) .child( right_click_menu("test4") - .trigger(Label::new("BOTTOM RIGHT")) + .trigger(|_| Label::new("BOTTOM RIGHT")) .anchor(Corner::BottomRight) .attach(Corner::TopRight) .menu(move |window, cx| build_menu(window, cx, "bottom right")), diff --git a/crates/workspace/src/dock.rs b/crates/workspace/src/dock.rs index 26612892cc..c2694fba02 100644 --- a/crates/workspace/src/dock.rs +++ b/crates/workspace/src/dock.rs @@ -889,7 +889,7 @@ impl Render for PanelButtons { }) .anchor(menu_anchor) .attach(menu_attach) - .trigger( + .trigger(move |is_active| { IconButton::new(name, icon) .icon_size(IconSize::Small) .toggle_state(is_active_button) @@ -899,10 +899,12 @@ impl Render for PanelButtons { window.dispatch_action(action.boxed_clone(), cx) } }) - .tooltip(move |window, cx| { - Tooltip::for_action(tooltip.clone(), &*action, window, cx) - }), - ), + .when(!is_active, |this| { + this.tooltip(move |window, cx| { + Tooltip::for_action(tooltip.clone(), &*action, window, cx) + }) + }) + }), ) }) .collect(); diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 1eb142cffe..9087f6c94f 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -2374,232 +2374,234 @@ impl Pane { let is_pinned = self.is_tab_pinned(ix); let pane = cx.entity().downgrade(); let menu_context = item.item_focus_handle(cx); - right_click_menu(ix).trigger(tab).menu(move |window, cx| { - let pane = pane.clone(); - let menu_context = menu_context.clone(); - ContextMenu::build(window, cx, move |mut menu, window, cx| { - if let Some(pane) = pane.upgrade() { - menu = menu - .entry( - "Close", - Some(Box::new(CloseActiveItem { - save_intent: None, - close_pinned: true, - })), - window.handler_for(&pane, move |pane, window, cx| { - pane.close_item_by_id(item_id, SaveIntent::Close, window, cx) - .detach_and_log_err(cx); - }), - ) - .item(ContextMenuItem::Entry( - ContextMenuEntry::new("Close Others") - .action(Box::new(CloseInactiveItems { + right_click_menu(ix) + .trigger(|_| tab) + .menu(move |window, cx| { + let pane = pane.clone(); + let menu_context = menu_context.clone(); + ContextMenu::build(window, cx, move |mut menu, window, cx| { + if let Some(pane) = pane.upgrade() { + menu = menu + .entry( + "Close", + Some(Box::new(CloseActiveItem { save_intent: None, - close_pinned: false, - })) - .disabled(total_items == 1) - .handler(window.handler_for(&pane, move |pane, window, cx| { - pane.close_items(window, cx, SaveIntent::Close, |id| { - id != item_id - }) - .detach_and_log_err(cx); + close_pinned: true, })), - )) - .separator() - .item(ContextMenuItem::Entry( - ContextMenuEntry::new("Close Left") - .action(Box::new(CloseItemsToTheLeft { - close_pinned: false, - })) - .disabled(!has_items_to_left) - .handler(window.handler_for(&pane, move |pane, window, cx| { - pane.close_items_to_the_left_by_id( - item_id, - &CloseItemsToTheLeft { - close_pinned: false, - }, - pane.get_non_closeable_item_ids(false), - window, - cx, - ) - .detach_and_log_err(cx); - })), - )) - .item(ContextMenuItem::Entry( - ContextMenuEntry::new("Close Right") - .action(Box::new(CloseItemsToTheRight { - close_pinned: false, - })) - .disabled(!has_items_to_right) - .handler(window.handler_for(&pane, move |pane, window, cx| { - pane.close_items_to_the_right_by_id( - item_id, - &CloseItemsToTheRight { - close_pinned: false, - }, - pane.get_non_closeable_item_ids(false), - window, - cx, - ) - .detach_and_log_err(cx); - })), - )) - .separator() - .entry( - "Close Clean", - Some(Box::new(CloseCleanItems { - close_pinned: false, - })), - window.handler_for(&pane, move |pane, window, cx| { - if let Some(task) = pane.close_clean_items( - &CloseCleanItems { - close_pinned: false, - }, - window, - cx, - ) { - task.detach_and_log_err(cx) - } - }), - ) - .entry( - "Close All", - Some(Box::new(CloseAllItems { - save_intent: None, - close_pinned: false, - })), - window.handler_for(&pane, |pane, window, cx| { - if let Some(task) = pane.close_all_items( - &CloseAllItems { + window.handler_for(&pane, move |pane, window, cx| { + pane.close_item_by_id(item_id, SaveIntent::Close, window, cx) + .detach_and_log_err(cx); + }), + ) + .item(ContextMenuItem::Entry( + ContextMenuEntry::new("Close Others") + .action(Box::new(CloseInactiveItems { save_intent: None, close_pinned: false, - }, - window, - cx, - ) { - task.detach_and_log_err(cx) - } - }), - ); - - let pin_tab_entries = |menu: ContextMenu| { - menu.separator().map(|this| { - if is_pinned { - this.entry( - "Unpin Tab", - Some(TogglePinTab.boxed_clone()), - window.handler_for(&pane, move |pane, window, cx| { - pane.unpin_tab_at(ix, window, cx); - }), - ) - } else { - this.entry( - "Pin Tab", - Some(TogglePinTab.boxed_clone()), - window.handler_for(&pane, move |pane, window, cx| { - pane.pin_tab_at(ix, window, cx); - }), - ) - } - }) - }; - if let Some(entry) = single_entry_to_resolve { - let project_path = pane - .read(cx) - .item_for_entry(entry, cx) - .and_then(|item| item.project_path(cx)); - let worktree = project_path.as_ref().and_then(|project_path| { - pane.read(cx) - .project - .upgrade()? - .read(cx) - .worktree_for_id(project_path.worktree_id, cx) - }); - let has_relative_path = worktree.as_ref().is_some_and(|worktree| { - worktree - .read(cx) - .root_entry() - .map_or(false, |entry| entry.is_dir()) - }); - - let entry_abs_path = pane.read(cx).entry_abs_path(entry, cx); - let parent_abs_path = entry_abs_path - .as_deref() - .and_then(|abs_path| Some(abs_path.parent()?.to_path_buf())); - let relative_path = project_path - .map(|project_path| project_path.path) - .filter(|_| has_relative_path); - - let visible_in_project_panel = relative_path.is_some() - && worktree.is_some_and(|worktree| worktree.read(cx).is_visible()); - - let entry_id = entry.to_proto(); - menu = menu - .separator() - .when_some(entry_abs_path, |menu, abs_path| { - menu.entry( - "Copy Path", - Some(Box::new(zed_actions::workspace::CopyPath)), - window.handler_for(&pane, move |_, _, cx| { - cx.write_to_clipboard(ClipboardItem::new_string( - abs_path.to_string_lossy().to_string(), - )); - }), - ) - }) - .when_some(relative_path, |menu, relative_path| { - menu.entry( - "Copy Relative Path", - Some(Box::new(zed_actions::workspace::CopyRelativePath)), - window.handler_for(&pane, move |_, _, cx| { - cx.write_to_clipboard(ClipboardItem::new_string( - relative_path.to_string_lossy().to_string(), - )); - }), - ) - }) - .map(pin_tab_entries) - .separator() - .when(visible_in_project_panel, |menu| { - menu.entry( - "Reveal In Project Panel", - Some(Box::new(RevealInProjectPanel { - entry_id: Some(entry_id), + })) + .disabled(total_items == 1) + .handler(window.handler_for(&pane, move |pane, window, cx| { + pane.close_items(window, cx, SaveIntent::Close, |id| { + id != item_id + }) + .detach_and_log_err(cx); })), - window.handler_for(&pane, move |pane, _, cx| { - pane.project - .update(cx, |_, cx| { - cx.emit(project::Event::RevealInProjectPanel( - ProjectEntryId::from_proto(entry_id), - )) - }) - .ok(); - }), - ) - }) - .when_some(parent_abs_path, |menu, parent_abs_path| { - menu.entry( - "Open in Terminal", - Some(Box::new(OpenInTerminal)), - window.handler_for(&pane, move |_, window, cx| { - window.dispatch_action( - OpenTerminal { - working_directory: parent_abs_path.clone(), - } - .boxed_clone(), + )) + .separator() + .item(ContextMenuItem::Entry( + ContextMenuEntry::new("Close Left") + .action(Box::new(CloseItemsToTheLeft { + close_pinned: false, + })) + .disabled(!has_items_to_left) + .handler(window.handler_for(&pane, move |pane, window, cx| { + pane.close_items_to_the_left_by_id( + item_id, + &CloseItemsToTheLeft { + close_pinned: false, + }, + pane.get_non_closeable_item_ids(false), + window, cx, - ); - }), - ) - }); - } else { - menu = menu.map(pin_tab_entries); - } - } + ) + .detach_and_log_err(cx); + })), + )) + .item(ContextMenuItem::Entry( + ContextMenuEntry::new("Close Right") + .action(Box::new(CloseItemsToTheRight { + close_pinned: false, + })) + .disabled(!has_items_to_right) + .handler(window.handler_for(&pane, move |pane, window, cx| { + pane.close_items_to_the_right_by_id( + item_id, + &CloseItemsToTheRight { + close_pinned: false, + }, + pane.get_non_closeable_item_ids(false), + window, + cx, + ) + .detach_and_log_err(cx); + })), + )) + .separator() + .entry( + "Close Clean", + Some(Box::new(CloseCleanItems { + close_pinned: false, + })), + window.handler_for(&pane, move |pane, window, cx| { + if let Some(task) = pane.close_clean_items( + &CloseCleanItems { + close_pinned: false, + }, + window, + cx, + ) { + task.detach_and_log_err(cx) + } + }), + ) + .entry( + "Close All", + Some(Box::new(CloseAllItems { + save_intent: None, + close_pinned: false, + })), + window.handler_for(&pane, |pane, window, cx| { + if let Some(task) = pane.close_all_items( + &CloseAllItems { + save_intent: None, + close_pinned: false, + }, + window, + cx, + ) { + task.detach_and_log_err(cx) + } + }), + ); - menu.context(menu_context) + let pin_tab_entries = |menu: ContextMenu| { + menu.separator().map(|this| { + if is_pinned { + this.entry( + "Unpin Tab", + Some(TogglePinTab.boxed_clone()), + window.handler_for(&pane, move |pane, window, cx| { + pane.unpin_tab_at(ix, window, cx); + }), + ) + } else { + this.entry( + "Pin Tab", + Some(TogglePinTab.boxed_clone()), + window.handler_for(&pane, move |pane, window, cx| { + pane.pin_tab_at(ix, window, cx); + }), + ) + } + }) + }; + if let Some(entry) = single_entry_to_resolve { + let project_path = pane + .read(cx) + .item_for_entry(entry, cx) + .and_then(|item| item.project_path(cx)); + let worktree = project_path.as_ref().and_then(|project_path| { + pane.read(cx) + .project + .upgrade()? + .read(cx) + .worktree_for_id(project_path.worktree_id, cx) + }); + let has_relative_path = worktree.as_ref().is_some_and(|worktree| { + worktree + .read(cx) + .root_entry() + .map_or(false, |entry| entry.is_dir()) + }); + + let entry_abs_path = pane.read(cx).entry_abs_path(entry, cx); + let parent_abs_path = entry_abs_path + .as_deref() + .and_then(|abs_path| Some(abs_path.parent()?.to_path_buf())); + let relative_path = project_path + .map(|project_path| project_path.path) + .filter(|_| has_relative_path); + + let visible_in_project_panel = relative_path.is_some() + && worktree.is_some_and(|worktree| worktree.read(cx).is_visible()); + + let entry_id = entry.to_proto(); + menu = menu + .separator() + .when_some(entry_abs_path, |menu, abs_path| { + menu.entry( + "Copy Path", + Some(Box::new(zed_actions::workspace::CopyPath)), + window.handler_for(&pane, move |_, _, cx| { + cx.write_to_clipboard(ClipboardItem::new_string( + abs_path.to_string_lossy().to_string(), + )); + }), + ) + }) + .when_some(relative_path, |menu, relative_path| { + menu.entry( + "Copy Relative Path", + Some(Box::new(zed_actions::workspace::CopyRelativePath)), + window.handler_for(&pane, move |_, _, cx| { + cx.write_to_clipboard(ClipboardItem::new_string( + relative_path.to_string_lossy().to_string(), + )); + }), + ) + }) + .map(pin_tab_entries) + .separator() + .when(visible_in_project_panel, |menu| { + menu.entry( + "Reveal In Project Panel", + Some(Box::new(RevealInProjectPanel { + entry_id: Some(entry_id), + })), + window.handler_for(&pane, move |pane, _, cx| { + pane.project + .update(cx, |_, cx| { + cx.emit(project::Event::RevealInProjectPanel( + ProjectEntryId::from_proto(entry_id), + )) + }) + .ok(); + }), + ) + }) + .when_some(parent_abs_path, |menu, parent_abs_path| { + menu.entry( + "Open in Terminal", + Some(Box::new(OpenInTerminal)), + window.handler_for(&pane, move |_, window, cx| { + window.dispatch_action( + OpenTerminal { + working_directory: parent_abs_path.clone(), + } + .boxed_clone(), + cx, + ); + }), + ) + }); + } else { + menu = menu.map(pin_tab_entries); + } + } + + menu.context(menu_context) + }) }) - }) } fn render_tab_bar(&mut self, window: &mut Window, cx: &mut Context) -> AnyElement {