Skip to content

Commit 7572d13

Browse files
committed
Top menu fix: disable items on other windows
- Sort - Command palette - Future-proofing: make sure no more menu items are forgotten in the future
1 parent 1d2fd4f commit 7572d13

4 files changed

Lines changed: 38 additions & 12 deletions

File tree

apps/desktop/src-tauri/src/commands/CLAUDE.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@ immediately to business-logic modules. No significant logic lives here.
3333
- **Platform gates.** `volumes` is macOS-only; `mtp` and `network` are macOS+Linux; `volumes_linux` is Linux-only. Individual functions also use `#[cfg]` where behaviour differs (e.g., `sync_status`).
3434
- **`start_selection_drag`** requires the main thread. It uses `app.run_on_main_thread()` plus a `std::sync::mpsc` channel to return the result synchronously.
3535
- **`list_shares_with_credentials`** has `#[allow(clippy::too_many_arguments)]` because Tauri command parameters must be top-level arguments — no struct bundling.
36+
- **`set_menu_context` and Close tab (⌘W).** When the main window loses focus, `set_menu_context("other")` disables all
37+
non-App menu items — but `CLOSE_TAB_ID` is explicitly excluded. On macOS, ⌘W means "close the front window," and the
38+
`on_menu_event` close-tab exception handles this: if main is focused it closes a tab, otherwise it closes the focused
39+
non-main window (Settings, viewer, debug). If `CLOSE_TAB_ID` were disabled, its accelerator wouldn't fire and ⌘W would
40+
stop working in non-main windows. This is the only item that needs this exemption — all other non-App items are
41+
correctly disabled because they only make sense in the explorer.
42+
3643
## Dependencies
3744

3845
All major subsystems: `file_system`, `volumes`, `mtp`, `network`, `font_metrics`, `icons`,

apps/desktop/src-tauri/src/commands/ui.rs

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::ignore_poison::IgnorePoison;
2-
use crate::menu::{CommandScope, MenuState, build_context_menu, build_tab_context_menu, menu_id_to_command};
2+
use crate::menu::{CLOSE_TAB_ID, CommandScope, MenuState, build_context_menu, build_tab_context_menu, menu_id_to_command};
33
#[cfg(any(target_os = "macos", target_os = "linux"))]
44
use std::process::Command;
55
use tauri::menu::ContextMenu;
@@ -229,22 +229,28 @@ pub fn update_pin_tab_menu<R: Runtime>(app: AppHandle<R>, is_pinned: bool) -> Re
229229
item.set_text(label).map_err(|e| e.to_string())
230230
}
231231

232-
/// Enables or disables file-scoped menu items based on the current context.
232+
/// Enables or disables explorer-scoped menu items based on the current context.
233233
/// - `"explorer"`: all menu items enabled (main file explorer has focus)
234-
/// - `"other"`: file-scoped items disabled (Settings, file viewer, or other window has focus)
234+
/// - `"other"`: all non-App items disabled except Close tab (⌘W), which doubles as
235+
/// "close the focused window" — standard macOS behavior
235236
#[tauri::command]
236237
pub fn set_menu_context<R: Runtime>(app: AppHandle<R>, context: String) -> Result<(), String> {
237238
let enabled = context == "explorer";
238239
let menu_state = app.state::<MenuState<R>>();
239-
let items = menu_state.items.lock_ignore_poison();
240240

241-
for (id, entry) in items.iter() {
242-
if let Some((_, CommandScope::FileScoped)) = menu_id_to_command(id) {
241+
for (id, entry) in menu_state.items.lock_ignore_poison().iter() {
242+
// Close tab stays enabled: on_menu_event has special logic to close the focused
243+
// non-main window when main isn't focused (standard ⌘W behavior on macOS).
244+
if id == &CLOSE_TAB_ID {
245+
continue;
246+
}
247+
let is_app = matches!(menu_id_to_command(id), Some((_, CommandScope::App)));
248+
if !is_app {
243249
let _ = entry.item.set_enabled(enabled);
244250
}
245251
}
246252

247-
// Items stored in separate MenuState fields (not in the HashMap) also need toggling
253+
// Items stored in separate MenuState fields (not in the HashMap)
248254
if let Some(ref item) = *menu_state.pin_tab.lock_ignore_poison() {
249255
let _ = item.set_enabled(enabled);
250256
}
@@ -257,6 +263,9 @@ pub fn set_menu_context<R: Runtime>(app: AppHandle<R>, context: String) -> Resul
257263
if let Some(ref item) = *menu_state.view_mode_brief.lock_ignore_poison() {
258264
let _ = item.set_enabled(enabled);
259265
}
266+
if let Some(ref submenu) = *menu_state.sort_submenu.lock_ignore_poison() {
267+
let _ = submenu.set_enabled(enabled);
268+
}
260269

261270
Ok(())
262271
}

apps/desktop/src-tauri/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,7 @@ pub fn run() {
290290
*menu_state.view_mode_brief_position.lock_ignore_poison() = menu_items.view_mode_brief_position;
291291
*menu_state.pin_tab.lock_ignore_poison() = Some(menu_items.pin_tab);
292292
*menu_state.items.lock_ignore_poison() = menu_items.items;
293+
*menu_state.sort_submenu.lock_ignore_poison() = Some(menu_items.sort_submenu);
293294
app.manage(menu_state);
294295

295296
// Set window title based on license status

apps/desktop/src-tauri/src/menu/mod.rs

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ pub fn menu_id_to_command(menu_id: &str) -> Option<(&'static str, CommandScope)>
7575
ABOUT_ID => Some(("app.about", CommandScope::App)),
7676
ENTER_LICENSE_KEY_ID => Some(("app.licenseKey", CommandScope::App)),
7777
SETTINGS_ID => Some(("app.settings", CommandScope::App)),
78-
COMMAND_PALETTE_ID => Some(("app.commandPalette", CommandScope::App)),
78+
COMMAND_PALETTE_ID => Some(("app.commandPalette", CommandScope::FileScoped)),
7979

8080
// Pane commands (file-scoped)
8181
SWITCH_PANE_ID => Some(("pane.switch", CommandScope::FileScoped)),
@@ -186,6 +186,8 @@ pub struct MenuState<R: Runtime> {
186186
pub pin_tab: Mutex<Option<MenuItem<R>>>,
187187
/// Generic menu items keyed by menu item ID, for accelerator and enable/disable updates.
188188
pub items: Mutex<HashMap<String, MenuItemEntry<R>>>,
189+
/// Sort by submenu (disabled when not in explorer context)
190+
pub sort_submenu: Mutex<Option<Submenu<R>>>,
189191
}
190192

191193
impl<R: Runtime> Default for MenuState<R> {
@@ -200,6 +202,7 @@ impl<R: Runtime> Default for MenuState<R> {
200202
view_mode_brief_position: Mutex::new(0),
201203
pin_tab: Mutex::new(None),
202204
items: Mutex::new(HashMap::new()),
205+
sort_submenu: Mutex::new(None),
203206
}
204207
}
205208
}
@@ -220,6 +223,8 @@ pub struct MenuItems<R: Runtime> {
220223
pub pin_tab: MenuItem<R>,
221224
/// Generic menu items for accelerator updates, keyed by menu item ID.
222225
pub items: HashMap<String, MenuItemEntry<R>>,
226+
/// Sort by submenu (disabled when not in explorer context)
227+
pub sort_submenu: Submenu<R>,
223228
}
224229

225230
/// View mode type that matches the frontend type.
@@ -720,6 +725,7 @@ fn build_menu_macos<R: Runtime>(
720725
view_mode_brief_position: view_brief_pos,
721726
pin_tab: pin_tab_item,
722727
items,
728+
sort_submenu,
723729
})
724730
}
725731

@@ -991,6 +997,7 @@ fn build_menu_linux<R: Runtime>(
991997
view_mode_brief_position: view_brief_pos,
992998
pin_tab: pin_tab_item,
993999
items,
1000+
sort_submenu,
9941001
})
9951002
}
9961003

@@ -1380,14 +1387,15 @@ mod tests {
13801387
menu_id_to_command(SETTINGS_ID),
13811388
Some(("app.settings", CommandScope::App))
13821389
);
1383-
assert_eq!(
1384-
menu_id_to_command(COMMAND_PALETTE_ID),
1385-
Some(("app.commandPalette", CommandScope::App))
1386-
);
13871390
assert_eq!(
13881391
menu_id_to_command(ENTER_LICENSE_KEY_ID),
13891392
Some(("app.licenseKey", CommandScope::App))
13901393
);
1394+
// Command palette is FileScoped — disabled when Settings/viewer has focus
1395+
assert_eq!(
1396+
menu_id_to_command(COMMAND_PALETTE_ID),
1397+
Some(("app.commandPalette", CommandScope::FileScoped))
1398+
);
13911399
}
13921400

13931401
#[test]
@@ -1504,4 +1512,5 @@ mod tests {
15041512
assert_eq!(command_id_to_menu_id("view.showHidden"), None);
15051513
assert_eq!(command_id_to_menu_id("unknown"), None);
15061514
}
1515+
15071516
}

0 commit comments

Comments
 (0)