select: run on_dismiss however the popup closes

crates/base/src/select.rs

@@ -129,7 +129,8 @@ impl Select {
         self
     }
 
-    /// Handles a dismissal requested through the Cancel action.
+    /// Handles a dismissal, however it was requested: the Cancel action, or
+    /// the accessible activation that closes an open control.
     ///
     /// This runs before the controlled open state is asked to close, so a
     /// caller that commits its pending value on dismissal can still read that
@@ -171,6 +172,26 @@ impl RenderOnce for Select {
         let on_dismiss = self.on_dismiss;
         let on_confirm = self.on_confirm;
 
+        // Every way of closing runs the same steps. A caller that tracks
+        // dismissal has to see one however the popup was closed, and the
+        // accessible activation closes exactly what Escape closes.
+        let close: ActionHandler = Rc::new({
+            let on_open_change = on_open_change.clone();
+            let on_dismiss = on_dismiss.clone();
+            let focus_handle = focus_handle.clone();
+            move |window: &mut Window, cx: &mut App| {
+                if let Some(handler) = on_dismiss.as_ref() {
+                    handler(window, cx);
+                }
+                if let Some(handler) = on_open_change.as_ref() {
+                    handler(false, window, cx);
+                }
+                if let Some(handle) = focus_handle.as_ref() {
+                    handle.focus(window, cx);
+                }
+            }
+        });
+
         div()
             .id(self.id)
             .role(Role::ComboBox)
@@ -189,21 +210,20 @@ impl RenderOnce for Select {
             .when(!disabled, |this| {
                 let on_open_change = on_open_change.clone();
                 let content_focus_handle = content_focus_handle.clone();
-                let focus_handle = focus_handle.clone();
+                let close = close.clone();
 
                 // Platform adapters may flatten the trigger child.
                 // Expose activation on the semantic root itself.
                 this.on_a11y_action(AccessibleAction::Click, move |_, window, cx| {
-                    if let Some(handler) = on_open_change.as_ref() {
-                        handler(!open, window, cx);
+                    if open {
+                        close(window, cx);
+                        return;
                     }
 
-                    let next_focus = if open {
-                        focus_handle.as_ref()
-                    } else {
-                        content_focus_handle.as_ref()
-                    };
-                    if let Some(handle) = next_focus {
+                    if let Some(handler) = on_open_change.as_ref() {
+                        handler(true, window, cx);
+                    }
+                    if let Some(handle) = content_focus_handle.as_ref() {
                         handle.focus(window, cx);
                     }
                 })
@@ -279,15 +299,7 @@ impl RenderOnce for Select {
                 }
 
                 cx.stop_propagation();
-                if let Some(handler) = on_dismiss.as_ref() {
-                    handler(window, cx);
-                }
-                if let Some(handler) = on_open_change.as_ref() {
-                    handler(false, window, cx);
-                }
-                if let Some(handle) = focus_handle.as_ref() {
-                    handle.focus(window, cx);
-                }
+                close(window, cx);
             })
             .children(self.children)
             .refine_style(&self.style)
@@ -308,6 +320,8 @@ mod tests {
         focus_handle: FocusHandle,
         content_focus_handle: FocusHandle,
         changes: Arc<Mutex<Vec<bool>>>,
+        /// Every step of a close, in the order it ran.
+        closing: Arc<Mutex<Vec<&'static str>>>,
     }
 
     impl SelectHarness {
@@ -318,6 +332,7 @@ mod tests {
                 focus_handle: cx.focus_handle(),
                 content_focus_handle: cx.focus_handle(),
                 changes: Arc::new(Mutex::new(Vec::new())),
+                closing: Arc::new(Mutex::new(Vec::new())),
             }
         }
     }
@@ -332,6 +347,8 @@ mod tests {
         fn render(&mut self, _: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
             let state = cx.entity();
             let changes = self.changes.clone();
+            let opened = self.closing.clone();
+            let dismissed = self.closing.clone();
 
             Select::new("select")
                 .open(self.open)
@@ -340,11 +357,16 @@ mod tests {
                 .content_focus_handle(&self.content_focus_handle)
                 .on_open_change(move |open, _, cx| {
                     changes.lock().unwrap().push(open);
+                    opened
+                        .lock()
+                        .unwrap()
+                        .push(if open { "open" } else { "close" });
                     state.update(cx, |state, cx| {
                         state.open = open;
                         cx.notify();
                     });
                 })
+                .on_dismiss(move |_, _| dismissed.lock().unwrap().push("dismiss"))
                 .child(div().track_focus(&self.content_focus_handle).size(px(20.)))
         }
     }
@@ -409,6 +431,31 @@ mod tests {
         );
     }
 
+    /// Closing runs `on_dismiss`, and runs it before the open state is asked
+    /// to close, so a caller that commits a pending value on dismissal can
+    /// still read that value.
+    ///
+    /// Every close shares one path, which is the point: the accessible
+    /// activation used to close by calling `on_open_change` alone, so a
+    /// consumer wiring `on_dismiss` — `crates/shell` forwards it to JS as
+    /// `onDismiss` — saw Escape but not a screen reader pressing the same
+    /// control. GPUI exposes no way to dispatch an accessibility action in a
+    /// test (`Window::handle_a11y_action` is `pub(crate)`), so this covers the
+    /// shared path through the route a test can reach.
+    #[gpui::test]
+    fn every_close_dismisses_before_it_closes(cx: &mut TestAppContext) {
+        let (cx, state) = harness(cx, false);
+
+        cx.simulate_keystrokes("down escape");
+        assert_eq!(
+            &*state
+                .read_with(cx, |state, _| state.closing.clone())
+                .lock()
+                .unwrap(),
+            &["open", "dismiss", "close"]
+        );
+    }
+
     #[gpui::test]
     fn disabled_select_is_not_keyboard_interactive(cx: &mut TestAppContext) {
         let (cx, state) = harness(cx, true);
@@ -442,12 +489,18 @@ mod tests {
                     .accessibility_label("Programming language")
                     .accessibility_value("Rust"),
             );
-            let disabled = info(Select::new("disabled").disabled(true));
+            // Open, so the expanded assertion below says something about
+            // `disabled` rather than about the default open state.
+            let disabled = info(Select::new("disabled").open(true).disabled(true));
 
             assert_eq!(enabled.label(), Some("Programming language"));
             assert_eq!(enabled.value(), Some("Rust"));
             assert_eq!(enabled.is_expanded(), Some(true));
-            assert_eq!(disabled.is_expanded(), Some(false));
+            assert_eq!(
+                disabled.is_expanded(),
+                Some(true),
+                "a disabled control still reports the state it is in"
+            );
             assert!(enabled.supports_action(accesskit::Action::Click));
             assert!(!disabled.supports_action(accesskit::Action::Click));
         });

crates/component/src/searchable_list/delegate.rs

@@ -9,11 +9,19 @@ pub trait SearchableListItem: Clone {
     type Value: Clone + PartialEq;
 
     /// Short display label shown in the dropdown row and in the trigger by default.
+    ///
+    /// This is also what assistive technology reads as the committed value, so
+    /// it has to stand on its own as text even when [`Self::display_title`]
+    /// draws something richer.
     fn title(&self) -> SharedString;
 
     /// Override the trigger display element (e.g. "Country (US)" instead of just "United States").
     ///
     /// Returns `None` to fall back to `title()`.
+    ///
+    /// This is presentation only. An element is not text, so the accessible
+    /// value keeps reporting [`Self::title`]; if the two would read
+    /// differently, put the meaning a listener needs in `title()`.
     fn display_title(&self) -> Option<AnyElement> {
         None
     }

website/zh-CN/base/primitives/select.md

@@ -40,7 +40,10 @@ use gpui_kit::base::{Select};
 
 ## 可访问性
 
-提供标签，暴露当前值，并保留上下键、Enter、Escape 与类型检索。
+在受控根节点上设置 `.accessibility_label(...)`，并把 `.accessibility_value(...)`
+设为已提交的选中项，而不是临时的搜索游标。根节点会暴露展开状态与可访问的激活操作。
+激活会请求切换展开状态，并在 trigger 与内容之间移动焦点。禁用的控件不暴露激活操作。
+带样式的 `Select` 会自动提供已提交的值，未选中时回退到 placeholder。
 
 ## 注意事项
 

website/zh-CN/docs/components/select.md

@@ -65,6 +65,20 @@ Select::new(&state)
     .placeholder("Select a language...")
 ```
 
+### 可访问性
+
+给控件一个不随选中项变化的名称：
+
+```rust
+Select::new(&state)
+    .accessibility_label("Programming language")
+    .placeholder("Choose a language")
+```
+
+可访问值取自已提交选项的 `title()` 以及 `title_prefix`。自定义的 `display_title()`
+仍然只用于视觉呈现。搜索不会改变这个已提交的值。未选中时，可访问值使用 placeholder。
+启用状态的控件会暴露可访问的激活操作，用于打开或关闭弹层。
+
 ### 可搜索
 
 启用 `searchable(true)` 后，下拉菜单中会出现搜索能力：