Fix missing tooltips for selected buttons (#18435)

Bennet Bo Fenner created 1 year ago

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

Change summary

crates/storybook/src/story_selector.rs           |  4 ++
crates/title_bar/src/application_menu.rs         | 24 +++++++++++------
crates/title_bar/src/stories/application_menu.rs | 16 +++++++++--
crates/title_bar/src/title_bar.rs                | 22 ++++++++++------
crates/ui/src/components/button/button_like.rs   |  6 +---
5 files changed, 47 insertions(+), 25 deletions(-)

Detailed changes

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(),

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<ContextMenu>,
+}
 
 impl ApplicationMenu {
-    pub fn new() -> Self {
-        Self
+    pub fn new(_: &mut ViewContext<Self>) -> 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<Self>) -> 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()
     }
 }

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<ApplicationMenu>,
+}
+
+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<Self>) -> impl IntoElement {
@@ -13,7 +23,7 @@ impl Render for ApplicationMenuStory {
             .child(Story::title_for::<ApplicationMenu>())
             .child(StorySection::new().child(StoryItem::new(
                 "Application Menu",
-                h_flex().child(ApplicationMenu::new()),
+                h_flex().child(self.menu.clone()),
             )))
     }
 }

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<Client>,
     workspace: WeakView<Workspace>,
     should_move: bool,
+    application_menu: Option<View<ApplicationMenu>>,
     _subscriptions: Vec<Subscription>,
 }
 
@@ -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,

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)
     }