Revert "gpui: Add `disabled` state to app menu items (#44191)"

Cole Miller created

This reverts commit 61c2a4334ca884ed0070bde239902ae3c81aba21.

Change summary

crates/gpui/examples/image/image.rs        |   1 
crates/gpui/examples/image_gallery.rs      |   5 
crates/gpui/examples/set_menus.rs          |  42 ++---
crates/gpui/examples/text.rs               |   1 
crates/gpui/src/app.rs                     |   3 
crates/gpui/src/platform/app_menu.rs       | 161 +----------------------
crates/gpui_macos/src/platform.rs          |  13 -
crates/livekit_client/examples/test_app.rs |  10 +
crates/storybook/src/app_menus.rs          |   5 
crates/title_bar/src/application_menu.rs   |   7 
crates/ui/src/components/context_menu.rs   |  12 -
crates/zed/src/zed/app_menus.rs            |  59 ++++----
12 files changed, 83 insertions(+), 236 deletions(-)

Detailed changes

crates/gpui/examples/image/image.rs 🔗

@@ -181,7 +181,6 @@ fn run_example() {
         cx.set_menus(vec![Menu {
             name: "Image".into(),
             items: vec![MenuItem::action("Quit", Quit)],
-            disabled: false,
         }]);
 
         let window_options = WindowOptions {

crates/gpui/examples/image_gallery.rs 🔗

@@ -273,7 +273,10 @@ fn run_example() {
         cx.activate(true);
         cx.on_action(|_: &Quit, cx| cx.quit());
         cx.bind_keys([KeyBinding::new("cmd-q", Quit, None)]);
-        cx.set_menus([Menu::new("Image Gallery").items([MenuItem::action("Quit", Quit)])]);
+        cx.set_menus(vec![Menu {
+            name: "Image Gallery".into(),
+            items: vec![MenuItem::action("Quit", Quit)],
+        }]);
 
         let window_options = WindowOptions {
             titlebar: Some(TitlebarOptions {

crates/gpui/examples/set_menus.rs 🔗

@@ -2,7 +2,7 @@
 
 use gpui::{
     App, Context, Global, Menu, MenuItem, SharedString, SystemMenuType, Window, WindowOptions,
-    actions, div, prelude::*,
+    actions, div, prelude::*, rgb,
 };
 use gpui_platform::application;
 
@@ -12,12 +12,12 @@ impl Render for SetMenus {
     fn render(&mut self, _window: &mut Window, _cx: &mut Context<Self>) -> impl IntoElement {
         div()
             .flex()
-            .bg(gpui::white())
+            .bg(rgb(0x2e7d32))
             .size_full()
             .justify_center()
             .items_center()
             .text_xl()
-            .text_color(gpui::black())
+            .text_color(rgb(0xffffff))
             .child("Set Menus Example")
     }
 }
@@ -28,8 +28,7 @@ fn run_example() {
 
         // Bring the menu bar to the foreground (so you can see the menu bar)
         cx.activate(true);
-        // Register the `quit` function so it can be referenced
-        // by the `MenuItem::action` in the menu bar
+        // Register the `quit` function so it can be referenced by the `MenuItem::action` in the menu bar
         cx.on_action(quit);
         cx.on_action(toggle_check);
         // Add menu items
@@ -92,24 +91,19 @@ impl Global for AppState {}
 
 fn set_app_menus(cx: &mut App) {
     let app_state = cx.global::<AppState>();
-    cx.set_menus([Menu::new("set_menus").items([
-        MenuItem::os_submenu("Services", SystemMenuType::Services),
-        MenuItem::separator(),
-        MenuItem::action("Disabled Item", gpui::NoAction).disabled(true),
-        MenuItem::submenu(Menu::new("Disabled Submenu").disabled(true)),
-        MenuItem::separator(),
-        MenuItem::action("List Mode", ToggleCheck).checked(app_state.view_mode == ViewMode::List),
-        MenuItem::submenu(
-            Menu::new("Mode").items([
-                MenuItem::action(ViewMode::List, ToggleCheck)
-                    .checked(app_state.view_mode == ViewMode::List),
-                MenuItem::action(ViewMode::Grid, ToggleCheck)
-                    .checked(app_state.view_mode == ViewMode::Grid),
-            ]),
-        ),
-        MenuItem::separator(),
-        MenuItem::action("Quit", Quit),
-    ])]);
+    cx.set_menus(vec![Menu {
+        name: "set_menus".into(),
+        items: vec![
+            MenuItem::os_submenu("Services", SystemMenuType::Services),
+            MenuItem::separator(),
+            MenuItem::action(ViewMode::List, ToggleCheck)
+                .checked(app_state.view_mode == ViewMode::List),
+            MenuItem::action(ViewMode::Grid, ToggleCheck)
+                .checked(app_state.view_mode == ViewMode::Grid),
+            MenuItem::separator(),
+            MenuItem::action("Quit", Quit),
+        ],
+    }]);
 }
 
 // Associate actions using the `actions!` macro (or `Action` derive macro)
@@ -117,7 +111,7 @@ actions!(set_menus, [Quit, ToggleCheck]);
 
 // Define the quit function that is registered with the App
 fn quit(_: &Quit, cx: &mut App) {
-    println!("Gracefully quitting the application...");
+    println!("Gracefully quitting the application . . .");
     cx.quit();
 }
 

crates/gpui/examples/text.rs 🔗

@@ -350,7 +350,6 @@ fn run_example() {
     application().run(|cx: &mut App| {
         cx.set_menus(vec![Menu {
             name: "GPUI Typography".into(),
-            disabled: false,
             items: vec![],
         }]);
 

crates/gpui/src/app.rs 🔗

@@ -2072,8 +2072,7 @@ impl App {
     }
 
     /// Sets the menu bar for this application. This will replace any existing menu bar.
-    pub fn set_menus(&self, menus: impl IntoIterator<Item = Menu>) {
-        let menus: Vec<Menu> = menus.into_iter().collect();
+    pub fn set_menus(&self, menus: Vec<Menu>) {
         self.platform.set_menus(menus, &self.keymap.borrow());
     }
 

crates/gpui/src/platform/app_menu.rs 🔗

@@ -7,39 +7,14 @@ pub struct Menu {
 
     /// The items in the menu
     pub items: Vec<MenuItem>,
-
-    /// Whether this menu is disabled
-    pub disabled: bool,
 }
 
 impl Menu {
-    /// Create a new Menu with the given name
-    pub fn new(name: impl Into<SharedString>) -> Self {
-        Self {
-            name: name.into(),
-            items: vec![],
-            disabled: false,
-        }
-    }
-
-    /// Set items to be in this menu
-    pub fn items(mut self, items: impl IntoIterator<Item = MenuItem>) -> Self {
-        self.items = items.into_iter().collect();
-        self
-    }
-
-    /// Set whether this menu is disabled
-    pub fn disabled(mut self, disabled: bool) -> Self {
-        self.disabled = disabled;
-        self
-    }
-
     /// Create an OwnedMenu from this Menu
     pub fn owned(self) -> OwnedMenu {
         OwnedMenu {
             name: self.name.to_string().into(),
             items: self.items.into_iter().map(|item| item.owned()).collect(),
-            disabled: self.disabled,
         }
     }
 }
@@ -97,9 +72,6 @@ pub enum MenuItem {
 
         /// Whether this action is checked
         checked: bool,
-
-        /// Whether this action is disabled
-        disabled: bool,
     },
 }
 
@@ -129,7 +101,6 @@ impl MenuItem {
             action: Box::new(action),
             os_action: None,
             checked: false,
-            disabled: false,
         }
     }
 
@@ -144,7 +115,6 @@ impl MenuItem {
             action: Box::new(action),
             os_action: Some(os_action),
             checked: false,
-            disabled: false,
         }
     }
 
@@ -158,13 +128,11 @@ impl MenuItem {
                 action,
                 os_action,
                 checked,
-                disabled,
             } => OwnedMenuItem::Action {
                 name: name.into(),
                 action,
                 os_action,
                 checked,
-                disabled,
             },
             MenuItem::SystemMenu(os_menu) => OwnedMenuItem::SystemMenu(os_menu.owned()),
         }
@@ -174,49 +142,19 @@ impl MenuItem {
     ///
     /// Only for [`MenuItem::Action`], otherwise, will be ignored
     pub fn checked(mut self, checked: bool) -> Self {
-        match &mut self {
-            MenuItem::Action { checked: old, .. } => {
-                *old = checked;
-            }
-            _ => {}
-        }
-        self
-    }
-
-    /// Returns whether this menu item is checked
-    ///
-    /// Only for [`MenuItem::Action`], otherwise, returns false
-    #[inline]
-    pub fn is_checked(&self) -> bool {
-        match self {
-            MenuItem::Action { checked, .. } => *checked,
-            _ => false,
-        }
-    }
-
-    /// Set whether this menu item is disabled
-    pub fn disabled(mut self, disabled: bool) -> Self {
-        match &mut self {
-            MenuItem::Action { disabled: old, .. } => {
-                *old = disabled;
-            }
-            MenuItem::Submenu(submenu) => {
-                submenu.disabled = disabled;
-            }
-            _ => {}
-        }
-        self
-    }
-
-    /// Returns whether this menu item is disabled
-    ///
-    /// Only for [`MenuItem::Action`] and [`MenuItem::Submenu`], otherwise, returns false
-    #[inline]
-    pub fn is_disabled(&self) -> bool {
         match self {
-            MenuItem::Action { disabled, .. } => *disabled,
-            MenuItem::Submenu(submenu) => submenu.disabled,
-            _ => false,
+            MenuItem::Action {
+                action,
+                os_action,
+                name,
+                ..
+            } => MenuItem::Action {
+                name,
+                action,
+                os_action,
+                checked,
+            },
+            _ => self,
         }
     }
 }
@@ -241,9 +179,6 @@ pub struct OwnedMenu {
 
     /// The items in the menu
     pub items: Vec<OwnedMenuItem>,
-
-    /// Whether this menu is disabled
-    pub disabled: bool,
 }
 
 /// The different kinds of items that can be in a menu
@@ -271,9 +206,6 @@ pub enum OwnedMenuItem {
 
         /// Whether this action is checked
         checked: bool,
-
-        /// Whether this action is disabled
-        disabled: bool,
     },
 }
 
@@ -287,13 +219,11 @@ impl Clone for OwnedMenuItem {
                 action,
                 os_action,
                 checked,
-                disabled,
             } => OwnedMenuItem::Action {
                 name: name.clone(),
                 action: action.boxed_clone(),
                 os_action: *os_action,
                 checked: *checked,
-                disabled: *disabled,
             },
             OwnedMenuItem::SystemMenu(os_menu) => OwnedMenuItem::SystemMenu(os_menu.clone()),
         }
@@ -357,70 +287,3 @@ pub(crate) fn init_app_menus(platform: &dyn Platform, cx: &App) {
         }
     }));
 }
-
-#[cfg(test)]
-mod tests {
-    use crate::Menu;
-
-    #[test]
-    fn test_menu() {
-        let menu = Menu::new("App")
-            .items(vec![
-                crate::MenuItem::action("Action 1", gpui::NoAction),
-                crate::MenuItem::separator(),
-            ])
-            .disabled(true);
-
-        assert_eq!(menu.name.as_ref(), "App");
-        assert_eq!(menu.items.len(), 2);
-        assert!(menu.disabled);
-    }
-
-    #[test]
-    fn test_menu_item_builder() {
-        use super::MenuItem;
-
-        let item = MenuItem::action("Test Action", gpui::NoAction);
-        assert_eq!(
-            match &item {
-                MenuItem::Action { name, .. } => name.as_ref(),
-                _ => unreachable!(),
-            },
-            "Test Action"
-        );
-        assert!(matches!(
-            item,
-            MenuItem::Action {
-                checked: false,
-                disabled: false,
-                ..
-            }
-        ));
-
-        assert!(
-            MenuItem::action("Test Action", gpui::NoAction)
-                .checked(true)
-                .is_checked()
-        );
-        assert!(
-            MenuItem::action("Test Action", gpui::NoAction)
-                .disabled(true)
-                .is_disabled()
-        );
-
-        let submenu = MenuItem::submenu(super::Menu {
-            name: "Submenu".into(),
-            items: vec![],
-            disabled: true,
-        });
-        assert_eq!(
-            match &submenu {
-                MenuItem::Submenu(menu) => menu.name.as_ref(),
-                _ => unreachable!(),
-            },
-            "Submenu"
-        );
-        assert!(!submenu.is_checked());
-        assert!(submenu.is_disabled());
-    }
-}

crates/gpui_macos/src/platform.rs 🔗

@@ -7,8 +7,8 @@ use block::ConcreteBlock;
 use cocoa::{
     appkit::{
         NSApplication, NSApplicationActivationPolicy::NSApplicationActivationPolicyRegular,
-        NSControl as _, NSEventModifierFlags, NSMenu, NSMenuItem, NSModalResponse, NSOpenPanel,
-        NSSavePanel, NSVisualEffectState, NSVisualEffectView, NSWindow,
+        NSEventModifierFlags, NSMenu, NSMenuItem, NSModalResponse, NSOpenPanel, NSSavePanel,
+        NSVisualEffectState, NSVisualEffectView, NSWindow,
     },
     base::{BOOL, NO, YES, id, nil, selector},
     foundation::{
@@ -297,7 +297,6 @@ impl MacPlatform {
                     action,
                     os_action,
                     checked,
-                    disabled,
                 } => {
                     // Note that this is intentionally using earlier bindings, whereas typically
                     // later ones take display precedence. See the discussion on
@@ -395,18 +394,13 @@ impl MacPlatform {
                     if *checked {
                         item.setState_(NSVisualEffectState::Active);
                     }
-                    item.setEnabled_(!disabled);
 
                     let tag = actions.len() as NSInteger;
                     let _: () = msg_send![item, setTag: tag];
                     actions.push(action.boxed_clone());
                     item
                 }
-                MenuItem::Submenu(Menu {
-                    name,
-                    items,
-                    disabled,
-                }) => {
+                MenuItem::Submenu(Menu { name, items }) => {
                     let item = NSMenuItem::new(nil).autorelease();
                     let submenu = NSMenu::new(nil).autorelease();
                     submenu.setDelegate_(delegate);
@@ -414,7 +408,6 @@ impl MacPlatform {
                         submenu.addItem_(Self::create_menu_item(item, delegate, actions, keymap));
                     }
                     item.setSubmenu_(submenu);
-                    item.setEnabled_(!disabled);
                     item.setTitle_(ns_string(name));
                     item
                 }

crates/livekit_client/examples/test_app.rs 🔗

@@ -35,7 +35,15 @@ fn main() {
         cx.activate(true);
         cx.on_action(quit);
         cx.bind_keys([KeyBinding::new("cmd-q", Quit, None)]);
-        cx.set_menus([Menu::new("Zed").items([MenuItem::action("Quit", Quit)])]);
+        cx.set_menus(vec![Menu {
+            name: "Zed".into(),
+            items: vec![MenuItem::Action {
+                name: "Quit".into(),
+                action: Box::new(Quit),
+                os_action: None,
+                checked: false,
+            }],
+        }]);
 
         let livekit_url = std::env::var("LIVEKIT_URL").unwrap_or("http://localhost:7880".into());
         let livekit_key = std::env::var("LIVEKIT_KEY").unwrap_or("devkey".into());

crates/storybook/src/app_menus.rs 🔗

@@ -3,5 +3,8 @@ use gpui::{Menu, MenuItem};
 pub fn app_menus() -> Vec<Menu> {
     use crate::actions::Quit;
 
-    vec![Menu::new("Storybook").items([MenuItem::action("Quit", Quit)])]
+    vec![Menu {
+        name: "Storybook".into(),
+        items: vec![MenuItem::action("Quit", Quit)],
+    }]
 }

crates/title_bar/src/application_menu.rs 🔗

@@ -114,9 +114,8 @@ impl ApplicationMenu {
                         name,
                         action,
                         checked,
-                        disabled,
                         ..
-                    } => menu.action_checked_with_disabled(name, action, checked, disabled),
+                    } => menu.action_checked(name, action, checked),
                     OwnedMenuItem::Submenu(submenu) => {
                         submenu
                             .items
@@ -127,10 +126,8 @@ impl ApplicationMenu {
                                     name,
                                     action,
                                     checked,
-                                    disabled,
                                     ..
-                                } => menu
-                                    .action_checked_with_disabled(name, action, checked, disabled),
+                                } => menu.action_checked(name, action, checked),
                                 OwnedMenuItem::Submenu(_) => menu,
                                 OwnedMenuItem::SystemMenu(_) => {
                                     // A system menu doesn't make sense in this context, so ignore it

crates/ui/src/components/context_menu.rs 🔗

@@ -692,20 +692,10 @@ impl ContextMenu {
     }
 
     pub fn action_checked(
-        self,
-        label: impl Into<SharedString>,
-        action: Box<dyn Action>,
-        checked: bool,
-    ) -> Self {
-        self.action_checked_with_disabled(label, action, checked, false)
-    }
-
-    pub fn action_checked_with_disabled(
         mut self,
         label: impl Into<SharedString>,
         action: Box<dyn Action>,
         checked: bool,
-        disabled: bool,
     ) -> Self {
         self.items.push(ContextMenuItem::Entry(ContextMenuEntry {
             toggle: if checked {
@@ -728,7 +718,7 @@ impl ContextMenu {
             icon_position: IconPosition::End,
             icon_size: IconSize::Small,
             icon_color: None,
-            disabled,
+            disabled: false,
             documentation_aside: None,
             end_slot_icon: None,
             end_slot_title: None,

crates/zed/src/zed/app_menus.rs 🔗

@@ -31,7 +31,6 @@ pub fn app_menus(cx: &mut App) -> Vec<Menu> {
         MenuItem::action("Toggle All Docks", workspace::ToggleAllDocks),
         MenuItem::submenu(Menu {
             name: "Editor Layout".into(),
-            disabled: false,
             items: vec![
                 MenuItem::action("Split Up", workspace::SplitUp::default()),
                 MenuItem::action("Split Down", workspace::SplitDown::default()),
@@ -61,31 +60,39 @@ pub fn app_menus(cx: &mut App) -> Vec<Menu> {
     vec![
         Menu {
             name: "Zed".into(),
-            disabled: false,
             items: vec![
                 MenuItem::action("About Zed", zed_actions::About),
                 MenuItem::action("Check for Updates", auto_update::Check),
                 MenuItem::separator(),
-                MenuItem::submenu(Menu::new("Settings").items([
-                    MenuItem::action("Open Settings", zed_actions::OpenSettings),
-                    MenuItem::action("Open Settings File", super::OpenSettingsFile),
-                    MenuItem::action("Open Project Settings", zed_actions::OpenProjectSettings),
-                    MenuItem::action("Open Project Settings File", super::OpenProjectSettingsFile),
-                    MenuItem::action("Open Default Settings", super::OpenDefaultSettings),
-                    MenuItem::separator(),
-                    MenuItem::action("Open Keymap", zed_actions::OpenKeymap),
-                    MenuItem::action("Open Keymap File", zed_actions::OpenKeymapFile),
-                    MenuItem::action("Open Default Key Bindings", zed_actions::OpenDefaultKeymap),
-                    MenuItem::separator(),
-                    MenuItem::action(
-                        "Select Theme...",
-                        zed_actions::theme_selector::Toggle::default(),
-                    ),
-                    MenuItem::action(
-                        "Select Icon Theme...",
-                        zed_actions::icon_theme_selector::Toggle::default(),
-                    ),
-                ])),
+                MenuItem::submenu(Menu {
+                    name: "Settings".into(),
+                    items: vec![
+                        MenuItem::action("Open Settings", zed_actions::OpenSettings),
+                        MenuItem::action("Open Settings File", super::OpenSettingsFile),
+                        MenuItem::action("Open Project Settings", zed_actions::OpenProjectSettings),
+                        MenuItem::action(
+                            "Open Project Settings File",
+                            super::OpenProjectSettingsFile,
+                        ),
+                        MenuItem::action("Open Default Settings", super::OpenDefaultSettings),
+                        MenuItem::separator(),
+                        MenuItem::action("Open Keymap", zed_actions::OpenKeymap),
+                        MenuItem::action("Open Keymap File", zed_actions::OpenKeymapFile),
+                        MenuItem::action(
+                            "Open Default Key Bindings",
+                            zed_actions::OpenDefaultKeymap,
+                        ),
+                        MenuItem::separator(),
+                        MenuItem::action(
+                            "Select Theme...",
+                            zed_actions::theme_selector::Toggle::default(),
+                        ),
+                        MenuItem::action(
+                            "Select Icon Theme...",
+                            zed_actions::icon_theme_selector::Toggle::default(),
+                        ),
+                    ],
+                }),
                 MenuItem::separator(),
                 #[cfg(target_os = "macos")]
                 MenuItem::os_submenu("Services", gpui::SystemMenuType::Services),
@@ -106,7 +113,6 @@ pub fn app_menus(cx: &mut App) -> Vec<Menu> {
         },
         Menu {
             name: "File".into(),
-            disabled: false,
             items: vec![
                 MenuItem::action("New", workspace::NewFile),
                 MenuItem::action("New Window", workspace::NewWindow),
@@ -154,7 +160,6 @@ pub fn app_menus(cx: &mut App) -> Vec<Menu> {
         },
         Menu {
             name: "Edit".into(),
-            disabled: false,
             items: vec![
                 MenuItem::os_action("Undo", editor::actions::Undo, OsAction::Undo),
                 MenuItem::os_action("Redo", editor::actions::Redo, OsAction::Redo),
@@ -175,7 +180,6 @@ pub fn app_menus(cx: &mut App) -> Vec<Menu> {
         },
         Menu {
             name: "Selection".into(),
-            disabled: false,
             items: vec![
                 MenuItem::os_action(
                     "Select All",
@@ -223,12 +227,10 @@ pub fn app_menus(cx: &mut App) -> Vec<Menu> {
         },
         Menu {
             name: "View".into(),
-            disabled: false,
             items: view_items,
         },
         Menu {
             name: "Go".into(),
-            disabled: false,
             items: vec![
                 MenuItem::action("Back", workspace::GoBack),
                 MenuItem::action("Forward", workspace::GoForward),
@@ -260,7 +262,6 @@ pub fn app_menus(cx: &mut App) -> Vec<Menu> {
         },
         Menu {
             name: "Run".into(),
-            disabled: false,
             items: vec![
                 MenuItem::action(
                     "Spawn Task",
@@ -285,7 +286,6 @@ pub fn app_menus(cx: &mut App) -> Vec<Menu> {
         },
         Menu {
             name: "Window".into(),
-            disabled: false,
             items: vec![
                 MenuItem::action("Minimize", super::Minimize),
                 MenuItem::action("Zoom", super::Zoom),
@@ -294,7 +294,6 @@ pub fn app_menus(cx: &mut App) -> Vec<Menu> {
         },
         Menu {
             name: "Help".into(),
-            disabled: false,
             items: vec![
                 MenuItem::action(
                     "View Release Notes Locally",