From 0c65669b3e9d44ccc21b7f698a57527222cedb07 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Fri, 20 Mar 2026 11:50:13 -0400 Subject: [PATCH] Revert "gpui: Add `disabled` state to app menu items (#44191)" This reverts commit 61c2a4334ca884ed0070bde239902ae3c81aba21. --- 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(-) diff --git a/crates/gpui/examples/image/image.rs b/crates/gpui/examples/image/image.rs index 45fce26d046c17b716f1644757ef26978f23b6d6..832cdf896a80e84c3ca8b591e0a0956af2cedcac 100644 --- a/crates/gpui/examples/image/image.rs +++ b/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 { diff --git a/crates/gpui/examples/image_gallery.rs b/crates/gpui/examples/image_gallery.rs index bc5cda396c3c37a1ac92bb11abf2f5d57673765e..9d8ac29ff8c9762417ff59acbfc83db6ad9c8346 100644 --- a/crates/gpui/examples/image_gallery.rs +++ b/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 { diff --git a/crates/gpui/examples/set_menus.rs b/crates/gpui/examples/set_menus.rs index a07f3c36abcc86390595c73f9c6eae55c3c370ef..683793c35fd4d356c068a3c36b041fba1dbc5ecf 100644 --- a/crates/gpui/examples/set_menus.rs +++ b/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) -> 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::(); - 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(); } diff --git a/crates/gpui/examples/text.rs b/crates/gpui/examples/text.rs index 418ebaabf69da8717dcdd6aa5960abd986b6d05d..50fdef9beaec03b5c6d6f8ed710ff992f6918018 100644 --- a/crates/gpui/examples/text.rs +++ b/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![], }]); diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 882f3532da4a75606335d70a1063a5aff5e320c0..ea1e294973d65a0b83329a20c067626755de6444 100644 --- a/crates/gpui/src/app.rs +++ b/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) { - let menus: Vec = menus.into_iter().collect(); + pub fn set_menus(&self, menus: Vec) { self.platform.set_menus(menus, &self.keymap.borrow()); } diff --git a/crates/gpui/src/platform/app_menu.rs b/crates/gpui/src/platform/app_menu.rs index 27c20c00badc50a965560073885f09a4e271ce5e..b1e0d82bb9f6d4ee265d047f562e088a8e48c1db 100644 --- a/crates/gpui/src/platform/app_menu.rs +++ b/crates/gpui/src/platform/app_menu.rs @@ -7,39 +7,14 @@ pub struct Menu { /// The items in the menu pub items: Vec, - - /// Whether this menu is disabled - pub disabled: bool, } impl Menu { - /// Create a new Menu with the given name - pub fn new(name: impl Into) -> Self { - Self { - name: name.into(), - items: vec![], - disabled: false, - } - } - - /// Set items to be in this menu - pub fn items(mut self, items: impl IntoIterator) -> 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, - - /// 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()); - } -} diff --git a/crates/gpui_macos/src/platform.rs b/crates/gpui_macos/src/platform.rs index 8a502612a37db00ce76a71bbdce3ac7238ca2c9a..d9c22cbea0354caff9bd5dd80d7ea98fa7e891de 100644 --- a/crates/gpui_macos/src/platform.rs +++ b/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 } diff --git a/crates/livekit_client/examples/test_app.rs b/crates/livekit_client/examples/test_app.rs index eb87aa6cae4530f31fa778b162d585de0cbb253b..959944d0830bc8270c4f0b85896cbfc9351e5197 100644 --- a/crates/livekit_client/examples/test_app.rs +++ b/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()); diff --git a/crates/storybook/src/app_menus.rs b/crates/storybook/src/app_menus.rs index c3045cf7999b851245a2f540c6318b7d0ef57b4f..4e84b4c85da8b7ce3d9227ae174f842b4b1f9ce4 100644 --- a/crates/storybook/src/app_menus.rs +++ b/crates/storybook/src/app_menus.rs @@ -3,5 +3,8 @@ use gpui::{Menu, MenuItem}; pub fn app_menus() -> Vec { use crate::actions::Quit; - vec![Menu::new("Storybook").items([MenuItem::action("Quit", Quit)])] + vec![Menu { + name: "Storybook".into(), + items: vec![MenuItem::action("Quit", Quit)], + }] } diff --git a/crates/title_bar/src/application_menu.rs b/crates/title_bar/src/application_menu.rs index b5cb07f757fe6b4fa26df7cc0f875025a0c08a81..579e4dadbd590981a4aee15019bbe73e2bb28d5c 100644 --- a/crates/title_bar/src/application_menu.rs +++ b/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 diff --git a/crates/ui/src/components/context_menu.rs b/crates/ui/src/components/context_menu.rs index 064b67a433f0d053db9552e8def1064237db3980..f055777faa7149ec46076ea42c565b65d3a1ed68 100644 --- a/crates/ui/src/components/context_menu.rs +++ b/crates/ui/src/components/context_menu.rs @@ -692,20 +692,10 @@ impl ContextMenu { } pub fn action_checked( - self, - label: impl Into, - action: Box, - checked: bool, - ) -> Self { - self.action_checked_with_disabled(label, action, checked, false) - } - - pub fn action_checked_with_disabled( mut self, label: impl Into, action: Box, 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, diff --git a/crates/zed/src/zed/app_menus.rs b/crates/zed/src/zed/app_menus.rs index 3edbcad2d81d63b56e777218a3db5e57a42de7bc..f73d703557f8f73ad380c0b7a2cb995b29f92cf1 100644 --- a/crates/zed/src/zed/app_menus.rs +++ b/crates/zed/src/zed/app_menus.rs @@ -31,7 +31,6 @@ pub fn app_menus(cx: &mut App) -> Vec { 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 { 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 { 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 { 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 { 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 { 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 { name: "Run".into(), - disabled: false, items: vec![ MenuItem::action( "Spawn Task", @@ -285,7 +286,6 @@ pub fn app_menus(cx: &mut App) -> Vec { }, 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 { name: "Help".into(), - disabled: false, items: vec![ MenuItem::action( "View Release Notes Locally",