Merge pull request #2463 from zed-industries/kb/reapply-modal-accessibility

Reintroduce more accesible modal keybindings

assets/keymaps/default.json

@@ -191,7 +191,7 @@
     }
   },
   {
-    "context": "BufferSearchBar > Editor",
+    "context": "BufferSearchBar",
     "bindings": {
       "escape": "buffer_search::Dismiss",
       "tab": "buffer_search::FocusEditor",
@@ -200,13 +200,13 @@
     }
   },
   {
-    "context": "ProjectSearchBar > Editor",
+    "context": "ProjectSearchBar",
     "bindings": {
       "escape": "project_search::ToggleFocus"
     }
   },
   {
-    "context": "ProjectSearchView > Editor",
+    "context": "ProjectSearchView",
     "bindings": {
       "escape": "project_search::ToggleFocus"
     }

crates/gpui/src/keymap_matcher/binding.rs

@@ -11,6 +11,19 @@ pub struct Binding {
     context_predicate: Option<KeymapContextPredicate>,
 }
 
+impl std::fmt::Debug for Binding {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            "Binding {{ keystrokes: {:?}, action: {}::{}, context_predicate: {:?} }}",
+            self.keystrokes,
+            self.action.namespace(),
+            self.action.name(),
+            self.context_predicate
+        )
+    }
+}
+
 impl Clone for Binding {
     fn clone(&self) -> Self {
         Self {

crates/gpui/src/platform/mac/window.rs

@@ -699,6 +699,31 @@ impl platform::Window for Window {
         msg: &str,
         answers: &[&str],
     ) -> oneshot::Receiver<usize> {
+        // macOs applies overrides to modal window buttons after they are added.
+        // Two most important for this logic are:
+        // * Buttons with "Cancel" title will be displayed as the last buttons in the modal
+        // * Last button added to the modal via `addButtonWithTitle` stays focused
+        // * Focused buttons react on "space"/" " keypresses
+        // * Usage of `keyEquivalent`, `makeFirstResponder` or `setInitialFirstResponder` does not change the focus
+        //
+        // See also https://developer.apple.com/documentation/appkit/nsalert/1524532-addbuttonwithtitle#discussion
+        // ```
+        // By default, the first button has a key equivalent of Return,
+        // any button with a title of “Cancel” has a key equivalent of Escape,
+        // and any button with the title “Don’t Save” has a key equivalent of Command-D (but only if it’s not the first button).
+        // ```
+        //
+        // To avoid situations when the last element added is "Cancel" and it gets the focus
+        // (hence stealing both ESC and Space shortcuts), we find and add one non-Cancel button
+        // last, so it gets focus and a Space shortcut.
+        // This way, "Save this file? Yes/No/Cancel"-ish modals will get all three buttons mapped with a key.
+        let latest_non_cancel_label = answers
+            .iter()
+            .enumerate()
+            .rev()
+            .find(|(_, &label)| label != "Cancel")
+            .filter(|&(label_index, _)| label_index > 0);
+
         unsafe {
             let alert: id = msg_send![class!(NSAlert), alloc];
             let alert: id = msg_send![alert, init];
@@ -709,10 +734,20 @@ impl platform::Window for Window {
             };
             let _: () = msg_send![alert, setAlertStyle: alert_style];
             let _: () = msg_send![alert, setMessageText: ns_string(msg)];
-            for (ix, answer) in answers.iter().enumerate() {
+
+            for (ix, answer) in answers
+                .iter()
+                .enumerate()
+                .filter(|&(ix, _)| Some(ix) != latest_non_cancel_label.map(|(ix, _)| ix))
+            {
                 let button: id = msg_send![alert, addButtonWithTitle: ns_string(answer)];
                 let _: () = msg_send![button, setTag: ix as NSInteger];
             }
+            if let Some((ix, answer)) = latest_non_cancel_label {
+                let button: id = msg_send![alert, addButtonWithTitle: ns_string(answer)];
+                let _: () = msg_send![button, setTag: ix as NSInteger];
+            }
+
             let (done_tx, done_rx) = oneshot::channel();
             let done_tx = Cell::new(Some(done_tx));
             let block = ConcreteBlock::new(move |answer: NSInteger| {

crates/workspace/src/workspace.rs

@@ -234,7 +234,6 @@ pub fn init(app_state: Arc<AppState>, cx: &mut AppContext) {
         },
     );
     cx.add_action(Workspace::toggle_sidebar_item);
-    cx.add_action(Workspace::focus_center);
     cx.add_action(|workspace: &mut Workspace, _: &ActivatePreviousPane, cx| {
         workspace.activate_previous_pane(cx)
     });
@@ -1415,11 +1414,6 @@ impl Workspace {
         cx.notify();
     }
 
-    pub fn focus_center(&mut self, _: &menu::Cancel, cx: &mut ViewContext<Self>) {
-        cx.focus_self();
-        cx.notify();
-    }
-
     fn add_pane(&mut self, cx: &mut ViewContext<Self>) -> ViewHandle<Pane> {
         let pane = cx.add_view(|cx| {
             Pane::new(

crates/zed/RELEASE_CHANNEL

@@ -1 +1 @@
-preview
+stable