Collab · CommentSidebar: threads are a list; each row is a Tab stop, named, selected, picked with Enter or Space

src/collab/sidebar.rs

@@ -1,7 +1,7 @@
 use std::rc::Rc;
 
 use gpui::{
-    App, ElementId, Entity, InteractiveElement, IntoElement, ParentElement, RenderOnce,
+    App, ElementId, Entity, InteractiveElement, IntoElement, ParentElement, RenderOnce, Role,
     SharedString, StatefulInteractiveElement, Styled, Window, div, prelude::*,
 };
 
@@ -12,7 +12,7 @@ use crate::{
     forms::{OnValue, Run, TextInput},
     i18n,
     primitives::IconName,
-    theme::ControlSize,
+    theme::{ActiveTheme, ControlSize, Radius},
 };
 
 type OnThreadFlag = Rc<dyn Fn(&SharedString, bool, &mut Window, &mut App)>;
@@ -135,7 +135,15 @@ impl RenderOnce for CommentSidebar {
                 i18n::text(cx, none, &[]),
             )
         });
-        let threads = shown.into_iter().map(|thread| {
+        let count = shown.len();
+        let theme = cx.theme();
+        let (ring, radius) = (theme.colors.focus, theme.radius(Radius::Md));
+        let threads = shown.into_iter().enumerate().map(|(at, thread)| {
+            let name: SharedString = match (thread.comments.first(), &thread.quote) {
+                (Some(first), _) => format!("{}: {}", first.author.name, first.body).into(),
+                (None, Some(quote)) => quote.clone(),
+                (None, None) => thread.key.clone(),
+            };
             let key = thread.key.clone();
             let active = self.active.as_ref() == Some(&key);
             let mut item = CommentThread::new((self.id.clone(), format!("thread-{key}")), thread)
@@ -153,19 +161,43 @@ impl RenderOnce for CommentSidebar {
                 item =
                     item.on_react(move |ix, emoji, window, cx| react(&key, ix, emoji, window, cx));
             }
-            let select = self.on_select.clone();
+            let select = self.on_select.clone().filter(|_| !active);
+            let row = key.clone();
             div()
                 .id((self.id.clone(), format!("pick-{key}")))
-                .when_some(select.filter(|_| !active), |row, select| {
+                .debug_selector(move || format!("comment-{row}"))
+                .role(Role::ListItem)
+                .aria_label(name)
+                .aria_selected(active)
+                .aria_position_in_set(at + 1)
+                .aria_size_of_set(count)
+                .tab_index(0)
+                .rounded(radius)
+                .border_1()
+                .border_color(gpui::transparent_black())
+                .focus(move |style| style.border_color(ring))
+                .when_some(select, |row, select| {
+                    let (click, press, pressed) = (select.clone(), key.clone(), key.clone());
                     row.on_click(move |_, window, cx| {
-                        log::info!("comment sidebar: {key}");
-                        select(&key, window, cx)
+                        log::info!("comment sidebar: {press}");
+                        click(&press, window, cx)
+                    })
+                    .on_key_down(move |event, window, cx| {
+                        let key = event.keystroke.key.as_str();
+                        if matches!(key, "enter" | "space") && !event.keystroke.modifiers.modified()
+                        {
+                            cx.stop_propagation();
+                            log::info!("comment sidebar: {pressed}");
+                            select(&pressed, window, cx)
+                        }
                     })
                 })
                 .child(item)
         });
         div()
             .id(self.id.clone())
+            .role(Role::List)
+            .aria_label(i18n::text(cx, "comments.list", &[]))
             .size_full()
             .flex()
             .flex_col()

src/collab/tests.rs

@@ -98,3 +98,50 @@ fn following_begun_from_outside_still_stops_on_escape(cx: &mut TestAppContext) {
     let back = cx.update(|window, cx| desk.read(cx).outside.focus_handle(cx).is_focused(window));
     assert!(back, "focus returns to where it was");
 }
+
+/// Two threads, one resolved, and the key each pick asked for.
+struct Board {
+    picked: Vec<gpui::SharedString>,
+}
+
+impl Render for Board {
+    fn render(&mut self, _: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
+        let thread = |key: &str, resolved| super::Thread {
+            key: key.to_string().into(),
+            quote: None,
+            comments: vec![super::Comment {
+                author: Peer::new("ada", "Ada", 0),
+                at: jiff::Timestamp::UNIX_EPOCH,
+                body: format!("About {key}").into(),
+                reactions: Vec::new(),
+            }],
+            resolved,
+        };
+        let pick = cx.entity();
+        div().size_full().child(
+            super::CommentSidebar::new("remarks", [thread("a", false), thread("b", true)])
+                .active(None, move |key, _, cx| {
+                    pick.update(cx, |board, _| board.picked.push(key.clone()))
+                }),
+        )
+    }
+}
+
+#[gpui::test]
+fn without_resolving_one_list_takes_keyboard_picks(cx: &mut TestAppContext) {
+    cx.update(|cx| {
+        Theme::init(cx);
+        forms::bind_keys(cx);
+    });
+    let (board, cx) = cx.add_window_view(|_, _| Board { picked: Vec::new() });
+    cx.run_until_parked();
+    assert!(cx.debug_bounds("comment-a").is_some());
+    assert!(cx.debug_bounds("comment-b").is_some(), "resolved shows too");
+    cx.update(|window, cx| {
+        window.activate_window();
+        window.focus_next(cx);
+    });
+    cx.simulate_keystrokes("enter");
+    let picked = board.read_with(cx, |board, _| board.picked.clone());
+    assert_eq!(picked, vec![gpui::SharedString::from("a")]);
+}

src/i18n/builtin.rs

@@ -48,6 +48,7 @@ const EN: &[(&str, &str)] = &[
     ("comments.none_open", "No open comments"),
     ("comments.none_resolved", "Nothing resolved yet"),
     ("comments.none", "No comments"),
+    ("comments.list", "Comments"),
     ("time.now", "just now"),
     ("time.ago", "{span} ago"),
     ("time.in", "in {span}"),
@@ -107,6 +108,7 @@ const ZH_CN: &[(&str, &str)] = &[
     ("comments.none_open", "没有未解决的批注"),
     ("comments.none_resolved", "还没有已解决的批注"),
     ("comments.none", "没有批注"),
+    ("comments.list", "批注"),
     ("time.now", "刚刚"),
     ("time.ago", "{span}前"),
     ("time.in", "{span}后"),