From 73ff8c0f1fd19ffcf93bc0bc2b8ba9d05f5eb935 Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Fri, 27 Sep 2024 14:16:14 +0200 Subject: [PATCH] Fix missing tooltips for selected buttons (#18435) Reverts #13857. Hiding tooltips for selected buttons prevents tooltips like "Close x dock" from showing up, see #14938 for an example. The intention of the original PR was to hide the "Show application menu" tooltip, while the context menu is open. In order to fix this without breaking other UI elements, we track the state of the context menu using `PopoverMenuHandle` now, which allows us to prevent the tooltip from showing up while the context menu is open. Closes #14938 Release Notes: - Fixed an issue where some tooltips would not show up --- crates/storybook/src/story_selector.rs | 4 +++- crates/title_bar/src/application_menu.rs | 24 ++++++++++++------- .../title_bar/src/stories/application_menu.rs | 16 ++++++++++--- crates/title_bar/src/title_bar.rs | 22 ++++++++++------- .../ui/src/components/button/button_like.rs | 6 ++--- 5 files changed, 47 insertions(+), 25 deletions(-) diff --git a/crates/storybook/src/story_selector.rs b/crates/storybook/src/story_selector.rs index 881fd83f8f..3a1c2f5630 100644 --- a/crates/storybook/src/story_selector.rs +++ b/crates/storybook/src/story_selector.rs @@ -46,7 +46,9 @@ pub enum ComponentStory { impl ComponentStory { pub fn story(&self, cx: &mut WindowContext) -> AnyView { match self { - Self::ApplicationMenu => cx.new_view(|_| title_bar::ApplicationMenuStory).into(), + Self::ApplicationMenu => cx + .new_view(|cx| title_bar::ApplicationMenuStory::new(cx)) + .into(), Self::AutoHeightEditor => AutoHeightEditorStory::new(cx).into(), Self::Avatar => cx.new_view(|_| ui::AvatarStory).into(), Self::Button => cx.new_view(|_| ui::ButtonStory).into(), diff --git a/crates/title_bar/src/application_menu.rs b/crates/title_bar/src/application_menu.rs index 47d4818da5..13ee10c141 100644 --- a/crates/title_bar/src/application_menu.rs +++ b/crates/title_bar/src/application_menu.rs @@ -1,16 +1,19 @@ -use ui::{prelude::*, ContextMenu, NumericStepper, PopoverMenu, Tooltip}; +use ui::{prelude::*, ContextMenu, NumericStepper, PopoverMenu, PopoverMenuHandle, Tooltip}; -#[derive(IntoElement)] -pub struct ApplicationMenu; +pub struct ApplicationMenu { + context_menu_handle: PopoverMenuHandle, +} impl ApplicationMenu { - pub fn new() -> Self { - Self + pub fn new(_: &mut ViewContext) -> Self { + Self { + context_menu_handle: PopoverMenuHandle::default(), + } } } -impl RenderOnce for ApplicationMenu { - fn render(self, _cx: &mut WindowContext) -> impl IntoElement { +impl Render for ApplicationMenu { + fn render(&mut self, _cx: &mut ViewContext) -> impl IntoElement { PopoverMenu::new("application-menu") .menu(move |cx| { ContextMenu::build(cx, move |menu, cx| { @@ -125,9 +128,12 @@ impl RenderOnce for ApplicationMenu { .trigger( IconButton::new("application-menu", ui::IconName::Menu) .style(ButtonStyle::Subtle) - .tooltip(|cx| Tooltip::text("Open Application Menu", cx)) - .icon_size(IconSize::Small), + .icon_size(IconSize::Small) + .when(!self.context_menu_handle.is_deployed(), |this| { + this.tooltip(|cx| Tooltip::text("Open Application Menu", cx)) + }), ) + .with_handle(self.context_menu_handle.clone()) .into_any_element() } } diff --git a/crates/title_bar/src/stories/application_menu.rs b/crates/title_bar/src/stories/application_menu.rs index 0b804209fd..c3f8c700ae 100644 --- a/crates/title_bar/src/stories/application_menu.rs +++ b/crates/title_bar/src/stories/application_menu.rs @@ -1,11 +1,21 @@ -use gpui::Render; +use gpui::{Render, View}; use story::{Story, StoryItem, StorySection}; use ui::prelude::*; use crate::application_menu::ApplicationMenu; -pub struct ApplicationMenuStory; +pub struct ApplicationMenuStory { + menu: View, +} + +impl ApplicationMenuStory { + pub fn new(cx: &mut WindowContext) -> Self { + Self { + menu: cx.new_view(ApplicationMenu::new), + } + } +} impl Render for ApplicationMenuStory { fn render(&mut self, _cx: &mut ViewContext) -> impl IntoElement { @@ -13,7 +23,7 @@ impl Render for ApplicationMenuStory { .child(Story::title_for::()) .child(StorySection::new().child(StoryItem::new( "Application Menu", - h_flex().child(ApplicationMenu::new()), + h_flex().child(self.menu.clone()), ))) } } diff --git a/crates/title_bar/src/title_bar.rs b/crates/title_bar/src/title_bar.rs index e2d45a923b..73a82e9ee0 100644 --- a/crates/title_bar/src/title_bar.rs +++ b/crates/title_bar/src/title_bar.rs @@ -15,7 +15,7 @@ use feature_flags::{FeatureFlagAppExt, ZedPro}; use gpui::{ actions, div, px, Action, AnyElement, AppContext, Decorations, Element, InteractiveElement, Interactivity, IntoElement, Model, MouseButton, ParentElement, Render, Stateful, - StatefulInteractiveElement, Styled, Subscription, ViewContext, VisualContext, WeakView, + StatefulInteractiveElement, Styled, Subscription, View, ViewContext, VisualContext, WeakView, }; use project::{Project, RepositoryEntry}; use recent_projects::RecentProjects; @@ -65,6 +65,7 @@ pub struct TitleBar { client: Arc, workspace: WeakView, should_move: bool, + application_menu: Option>, _subscriptions: Vec, } @@ -131,12 +132,7 @@ impl Render for TitleBar { .child( h_flex() .gap_1() - .children(match self.platform_style { - PlatformStyle::Mac => None, - PlatformStyle::Linux | PlatformStyle::Windows => { - Some(ApplicationMenu::new()) - } - }) + .when_some(self.application_menu.clone(), |this, menu| this.child(menu)) .children(self.render_project_host(cx)) .child(self.render_project_name(cx)) .children(self.render_project_branch(cx)) @@ -215,6 +211,15 @@ impl TitleBar { let user_store = workspace.app_state().user_store.clone(); let client = workspace.app_state().client.clone(); let active_call = ActiveCall::global(cx); + + let platform_style = PlatformStyle::platform(); + let application_menu = match platform_style { + PlatformStyle::Mac => None, + PlatformStyle::Linux | PlatformStyle::Windows => { + Some(cx.new_view(ApplicationMenu::new)) + } + }; + let mut subscriptions = Vec::new(); subscriptions.push( cx.observe(&workspace.weak_handle().upgrade().unwrap(), |_, _, cx| { @@ -227,9 +232,10 @@ impl TitleBar { subscriptions.push(cx.observe(&user_store, |_, _, cx| cx.notify())); Self { - platform_style: PlatformStyle::platform(), + platform_style, content: div().id(id.into()), children: SmallVec::new(), + application_menu, workspace: workspace.weak_handle(), should_move: false, project, diff --git a/crates/ui/src/components/button/button_like.rs b/crates/ui/src/components/button/button_like.rs index 625875e4c9..a22c27d241 100644 --- a/crates/ui/src/components/button/button_like.rs +++ b/crates/ui/src/components/button/button_like.rs @@ -523,10 +523,8 @@ impl RenderOnce for ButtonLike { }) }, ) - .when(!self.selected, |this| { - this.when_some(self.tooltip, |this, tooltip| { - this.tooltip(move |cx| tooltip(cx)) - }) + .when_some(self.tooltip, |this, tooltip| { + this.tooltip(move |cx| tooltip(cx)) }) .children(self.children) }