tab: Use on_click instead of action for TabBar dropdown menu. (#1454)

Close #1453

crates/ui/src/menu/popup_menu.rs

@@ -168,6 +168,30 @@ impl PopupMenuItem {
         self
     }
 
+    /// Set checked state for the menu item by adding or removing check icon.
+    ///
+    /// If true, will set the icon to check icon, otherwise remove the icon.
+    pub fn checked(mut self, checked: bool) -> Self {
+        match &mut self {
+            PopupMenuItem::Item { icon: i, .. } => {
+                if checked {
+                    *i = Some(IconName::Check.into());
+                } else {
+                    *i = None;
+                }
+            }
+            PopupMenuItem::ElementItem { icon: i, .. } => {
+                if checked {
+                    *i = Some(IconName::Check.into());
+                } else {
+                    *i = None;
+                }
+            }
+            _ => {}
+        }
+        self
+    }
+
     /// Add a click handler for the menu item.
     ///
     /// Only works for [`PopupMenuItem::Item`] and [`PopupMenuItem::ElementItem`].

crates/ui/src/tab/tab_bar.rs

@@ -1,22 +1,15 @@
-use std::sync::Arc;
-
-use crate::button::{Button, ButtonVariants as _};
-use crate::menu::DropdownMenu as _;
-use crate::{h_flex, ActiveTheme, IconName, Selectable, Sizable, Size, StyledExt};
-use gpui::prelude::FluentBuilder as _;
 use gpui::{
-    div, Action, AnyElement, App, Corner, Div, Edges, ElementId, IntoElement, ParentElement,
+    div, prelude::FluentBuilder as _, px, AnyElement, App, Corner, Div, Edges, ElementId,
-    Pixels, RenderOnce, ScrollHandle, Stateful, StatefulInteractiveElement as _, StyleRefinement,
+    InteractiveElement, IntoElement, ParentElement, Pixels, RenderOnce, ScrollHandle, Stateful,
-    Styled, Window,
+    StatefulInteractiveElement as _, StyleRefinement, Styled, Window,
 };
-use gpui::{px, InteractiveElement};
 use smallvec::SmallVec;
+use std::rc::Rc;
 
 use super::{Tab, TabVariant};
-
+use crate::button::{Button, ButtonVariants as _};
-#[derive(Action, Debug, Clone, Copy, PartialEq, Eq)]
+use crate::menu::{DropdownMenu as _, PopupMenuItem};
-#[action(namespace = tab_bar, no_json)]
+use crate::{h_flex, ActiveTheme, IconName, Selectable, Sizable, Size, StyledExt};
-pub struct SelectTab(usize);
 
 #[derive(IntoElement)]
 pub struct TabBar {
@@ -31,7 +24,7 @@ pub struct TabBar {
     variant: TabVariant,
     size: Size,
     menu: bool,
-    on_click: Option<Arc<dyn Fn(&usize, &mut Window, &mut App) + 'static>>,
+    on_click: Option<Rc<dyn Fn(&usize, &mut Window, &mut App) + 'static>>,
     /// Special for internal TabPanel to remove the top border.
     tab_item_top_offset: Pixels,
 }
@@ -138,8 +131,11 @@ impl TabBar {
     /// Set the on_click callback of the TabBar, the first parameter is the index of the clicked tab.
     ///
     /// When this is set, the children's on_click will be ignored.
-    pub fn on_click(mut self, on_click: impl Fn(&usize, &mut Window, &mut App) + 'static) -> Self {
+    pub fn on_click<F>(mut self, on_click: F) -> Self
-        self.on_click = Some(Arc::new(on_click));
+    where
+        F: Fn(&usize, &mut Window, &mut App) + 'static,
+    {
+        self.on_click = Some(Rc::new(on_click));
         self
     }
 
@@ -212,17 +208,10 @@ impl RenderOnce for TabBar {
 
         let mut item_labels = Vec::new();
         let selected_index = self.selected_index;
+        let on_click = self.on_click.clone();
 
         self.base
             .group("tab-bar")
-            .on_action({
-                let on_click = self.on_click.clone();
-                move |action: &SelectTab, window: &mut Window, cx: &mut App| {
-                    if let Some(on_click) = on_click.clone() {
-                        on_click(&action.0, window, cx);
-                    }
-                }
-            })
             .relative()
             .flex()
             .items_center()
@@ -286,12 +275,16 @@ impl RenderOnce for TabBar {
                         .dropdown_menu(move |mut this, _, _| {
                             this = this.scrollable();
                             for (ix, (label, disabled)) in item_labels.iter().enumerate() {
-                                this = this.menu_with_check_and_disabled(
+                                this = this.item(
-                                    label.clone().unwrap_or_default(),
+                                    PopupMenuItem::new(label.clone().unwrap_or_default())
-                                    selected_index == Some(ix),
+                                        .checked(selected_index == Some(ix))
-                                    Box::new(SelectTab(ix)),
+                                        .disabled(*disabled)
-                                    *disabled,
+                                        .when_some(on_click.clone(), |this, on_click| {
-                                );
+                                            this.on_click(move |_, window, cx| {
+                                                on_click(&ix, window, cx)
+                                            })
+                                        }),
+                                )
                             }
 
                             this