debugger: Improve variable list keyboard navigation (#27308)

Remco Smits created 

This PR improves the keyboard navigation for the variable list. 
Before this PR, if you want to open/close nested variables, you had to
use the right/left & up/down arrow keys.
Now you can step through with just only using your left/right arrow
keys, this feels a bit more natural and more similar to how other
editors allow you to navigate through variables.

This PR also fixes the following issues:
- Allow selecting a scope to be the start of your selection
- Allow selecting previous item if the first item is selected

-----


https://github.com/user-attachments/assets/aff0b133-97be-4c09-8ee6-b11495ad5568

Release Notes:

- N/A

Change summary

crates/debugger_ui/src/session/running/variable_list.rs |  69 ++-
crates/debugger_ui/src/tests/variable_list.rs           | 213 +++++++++++
2 files changed, 256 insertions(+), 26 deletions(-)

Detailed changes

crates/debugger_ui/src/session/running/variable_list.rs 🔗 

@@ -18,6 +18,7 @@ actions!(variable_list, [ExpandSelectedEntry, CollapseSelectedEntry]);
 pub(crate) struct EntryState {
     depth: usize,
     is_expanded: bool,
+    has_children: bool,
     parent_reference: VariableReference,
 }
 
@@ -246,6 +247,7 @@ impl VariableList {
                 .entry(path.clone())
                 .and_modify(|state| {
                     state.parent_reference = container_reference;
+                    state.has_children = variables_reference != 0;
                 })
                 .or_insert(EntryState {
                     depth: path.indices.len(),
@@ -258,6 +260,7 @@ impl VariableList {
                                 .unwrap_or(scope.name.to_lowercase().starts_with("local"))
                     }),
                     parent_reference: container_reference,
+                    has_children: variables_reference != 0,
                 });
 
             entries.push(ListEntry {
@@ -358,41 +361,45 @@ impl VariableList {
     fn select_prev(&mut self, _: &SelectPrevious, window: &mut Window, cx: &mut Context<Self>) {
         self.cancel_variable_edit(&Default::default(), window, cx);
         if let Some(selection) = &self.selection {
-            if let Some(var_ix) = self.entries.iter().enumerate().find_map(|(ix, var)| {
-                if &var.path == selection {
+            let index = self.entries.iter().enumerate().find_map(|(ix, var)| {
+                if &var.path == selection && ix > 0 {
                     Some(ix.saturating_sub(1))
                 } else {
                     None
                 }
-            }) {
-                if let Some(new_selection) = self.entries.get(var_ix).map(|var| var.path.clone()) {
-                    self.selection = Some(new_selection);
-                    cx.notify();
-                } else {
-                    self.select_first(&SelectFirst, window, cx);
-                }
+            });
+
+            if let Some(new_selection) =
+                index.and_then(|ix| self.entries.get(ix).map(|var| var.path.clone()))
+            {
+                self.selection = Some(new_selection);
+                cx.notify();
+            } else {
+                self.select_last(&SelectLast, window, cx);
             }
         } else {
-            self.select_first(&SelectFirst, window, cx);
+            self.select_last(&SelectLast, window, cx);
         }
     }
 
     fn select_next(&mut self, _: &SelectNext, window: &mut Window, cx: &mut Context<Self>) {
         self.cancel_variable_edit(&Default::default(), window, cx);
         if let Some(selection) = &self.selection {
-            if let Some(var_ix) = self.entries.iter().enumerate().find_map(|(ix, var)| {
+            let index = self.entries.iter().enumerate().find_map(|(ix, var)| {
                 if &var.path == selection {
                     Some(ix.saturating_add(1))
                 } else {
                     None
                 }
-            }) {
-                if let Some(new_selection) = self.entries.get(var_ix).map(|var| var.path.clone()) {
-                    self.selection = Some(new_selection);
-                    cx.notify();
-                } else {
-                    self.select_first(&SelectFirst, window, cx);
-                }
+            });
+
+            if let Some(new_selection) =
+                index.and_then(|ix| self.entries.get(ix).map(|var| var.path.clone()))
+            {
+                self.selection = Some(new_selection);
+                cx.notify();
+            } else {
+                self.select_first(&SelectFirst, window, cx);
             }
         } else {
             self.select_first(&SelectFirst, window, cx);
@@ -437,7 +444,7 @@ impl VariableList {
     fn collapse_selected_entry(
         &mut self,
         _: &CollapseSelectedEntry,
-        _window: &mut Window,
+        window: &mut Window,
         cx: &mut Context<Self>,
     ) {
         if let Some(ref selected_entry) = self.selection {
@@ -446,25 +453,33 @@ impl VariableList {
                 return;
             };
 
-            entry_state.is_expanded = false;
-            cx.notify();
+            if !entry_state.is_expanded || !entry_state.has_children {
+                self.select_prev(&SelectPrevious, window, cx);
+            } else {
+                entry_state.is_expanded = false;
+                cx.notify();
+            }
         }
     }
 
     fn expand_selected_entry(
         &mut self,
         _: &ExpandSelectedEntry,
-        _window: &mut Window,
+        window: &mut Window,
         cx: &mut Context<Self>,
     ) {
-        if let Some(ref selected_entry) = self.selection {
+        if let Some(selected_entry) = &self.selection {
             let Some(entry_state) = self.entry_states.get_mut(selected_entry) else {
                 debug_panic!("Trying to toggle variable in variable list that has an no state");
                 return;
             };
 
-            entry_state.is_expanded = true;
-            cx.notify();
+            if entry_state.is_expanded || !entry_state.has_children {
+                self.select_next(&SelectNext, window, cx);
+            } else {
+                entry_state.is_expanded = true;
+                cx.notify();
+            }
         }
     }
 
@@ -649,6 +664,7 @@ impl VariableList {
         } else {
             colors.default
         };
+        let path = entry.path.clone();
 
         div()
             .id(var_ref as usize)
@@ -661,7 +677,8 @@ impl VariableList {
             .h_full()
             .hover(|style| style.bg(bg_hover_color))
             .on_click(cx.listener({
-                move |_this, _, _window, cx| {
+                move |this, _, _window, cx| {
+                    this.selection = Some(path.clone());
                     cx.notify();
                 }
             }))

crates/debugger_ui/src/tests/variable_list.rs 🔗 

@@ -1103,6 +1103,219 @@ async fn test_keyboard_navigation(executor: BackgroundExecutor, cx: &mut TestApp
             });
     });
 
+    // select scope 2 backwards
+    cx.dispatch_action(SelectPrevious);
+    cx.run_until_parked();
+    running_state.update(cx, |debug_panel_item, cx| {
+        debug_panel_item
+            .variable_list()
+            .update(cx, |variable_list, _| {
+                variable_list.assert_visual_entries(vec!["> Scope 1", "> Scope 2 <=== selected"]);
+            });
+    });
+
+    // select scope 1 backwards
+    cx.dispatch_action(SelectNext);
+    cx.run_until_parked();
+    running_state.update(cx, |debug_panel_item, cx| {
+        debug_panel_item
+            .variable_list()
+            .update(cx, |variable_list, _| {
+                variable_list.assert_visual_entries(vec!["> Scope 1 <=== selected", "> Scope 2"]);
+            });
+    });
+
+    // test stepping through nested with ExpandSelectedEntry/CollapseSelectedEntry actions
+
+    cx.dispatch_action(ExpandSelectedEntry);
+    cx.run_until_parked();
+    running_state.update(cx, |debug_panel_item, cx| {
+        debug_panel_item
+            .variable_list()
+            .update(cx, |variable_list, _| {
+                variable_list.assert_visual_entries(vec![
+                    "v Scope 1 <=== selected",
+                    "    > variable1",
+                    "    > variable2",
+                    "> Scope 2",
+                ]);
+            });
+    });
+
+    cx.dispatch_action(ExpandSelectedEntry);
+    cx.run_until_parked();
+    running_state.update(cx, |debug_panel_item, cx| {
+        debug_panel_item
+            .variable_list()
+            .update(cx, |variable_list, _| {
+                variable_list.assert_visual_entries(vec![
+                    "v Scope 1",
+                    "    > variable1 <=== selected",
+                    "    > variable2",
+                    "> Scope 2",
+                ]);
+            });
+    });
+
+    cx.dispatch_action(ExpandSelectedEntry);
+    cx.run_until_parked();
+    running_state.update(cx, |debug_panel_item, cx| {
+        debug_panel_item
+            .variable_list()
+            .update(cx, |variable_list, _| {
+                variable_list.assert_visual_entries(vec![
+                    "v Scope 1",
+                    "    v variable1 <=== selected",
+                    "        > nested1",
+                    "        > nested2",
+                    "    > variable2",
+                    "> Scope 2",
+                ]);
+            });
+    });
+
+    cx.dispatch_action(ExpandSelectedEntry);
+    cx.run_until_parked();
+    running_state.update(cx, |debug_panel_item, cx| {
+        debug_panel_item
+            .variable_list()
+            .update(cx, |variable_list, _| {
+                variable_list.assert_visual_entries(vec![
+                    "v Scope 1",
+                    "    v variable1",
+                    "        > nested1 <=== selected",
+                    "        > nested2",
+                    "    > variable2",
+                    "> Scope 2",
+                ]);
+            });
+    });
+
+    cx.dispatch_action(ExpandSelectedEntry);
+    cx.run_until_parked();
+    running_state.update(cx, |debug_panel_item, cx| {
+        debug_panel_item
+            .variable_list()
+            .update(cx, |variable_list, _| {
+                variable_list.assert_visual_entries(vec![
+                    "v Scope 1",
+                    "    v variable1",
+                    "        > nested1",
+                    "        > nested2 <=== selected",
+                    "    > variable2",
+                    "> Scope 2",
+                ]);
+            });
+    });
+
+    cx.dispatch_action(ExpandSelectedEntry);
+    cx.run_until_parked();
+    running_state.update(cx, |debug_panel_item, cx| {
+        debug_panel_item
+            .variable_list()
+            .update(cx, |variable_list, _| {
+                variable_list.assert_visual_entries(vec![
+                    "v Scope 1",
+                    "    v variable1",
+                    "        > nested1",
+                    "        > nested2",
+                    "    > variable2 <=== selected",
+                    "> Scope 2",
+                ]);
+            });
+    });
+
+    cx.dispatch_action(CollapseSelectedEntry);
+    cx.run_until_parked();
+    running_state.update(cx, |debug_panel_item, cx| {
+        debug_panel_item
+            .variable_list()
+            .update(cx, |variable_list, _| {
+                variable_list.assert_visual_entries(vec![
+                    "v Scope 1",
+                    "    v variable1",
+                    "        > nested1",
+                    "        > nested2 <=== selected",
+                    "    > variable2",
+                    "> Scope 2",
+                ]);
+            });
+    });
+
+    cx.dispatch_action(CollapseSelectedEntry);
+    cx.run_until_parked();
+    running_state.update(cx, |debug_panel_item, cx| {
+        debug_panel_item
+            .variable_list()
+            .update(cx, |variable_list, _| {
+                variable_list.assert_visual_entries(vec![
+                    "v Scope 1",
+                    "    v variable1",
+                    "        > nested1 <=== selected",
+                    "        > nested2",
+                    "    > variable2",
+                    "> Scope 2",
+                ]);
+            });
+    });
+
+    cx.dispatch_action(CollapseSelectedEntry);
+    cx.run_until_parked();
+    running_state.update(cx, |debug_panel_item, cx| {
+        debug_panel_item
+            .variable_list()
+            .update(cx, |variable_list, _| {
+                variable_list.assert_visual_entries(vec![
+                    "v Scope 1",
+                    "    v variable1 <=== selected",
+                    "        > nested1",
+                    "        > nested2",
+                    "    > variable2",
+                    "> Scope 2",
+                ]);
+            });
+    });
+
+    cx.dispatch_action(CollapseSelectedEntry);
+    cx.run_until_parked();
+    running_state.update(cx, |debug_panel_item, cx| {
+        debug_panel_item
+            .variable_list()
+            .update(cx, |variable_list, _| {
+                variable_list.assert_visual_entries(vec![
+                    "v Scope 1",
+                    "    > variable1 <=== selected",
+                    "    > variable2",
+                    "> Scope 2",
+                ]);
+            });
+    });
+
+    cx.dispatch_action(CollapseSelectedEntry);
+    cx.run_until_parked();
+    running_state.update(cx, |debug_panel_item, cx| {
+        debug_panel_item
+            .variable_list()
+            .update(cx, |variable_list, _| {
+                variable_list.assert_visual_entries(vec![
+                    "v Scope 1 <=== selected",
+                    "    > variable1",
+                    "    > variable2",
+                    "> Scope 2",
+                ]);
+            });
+    });
+
+    cx.dispatch_action(CollapseSelectedEntry);
+    cx.run_until_parked();
+    running_state.update(cx, |debug_panel_item, cx| {
+        debug_panel_item
+            .variable_list()
+            .update(cx, |variable_list, _| {
+                variable_list.assert_visual_entries(vec!["> Scope 1 <=== selected", "> Scope 2"]);
+            });
+    });
+
     let shutdown_session = project.update(cx, |project, cx| {
         project.dap_store().update(cx, |dap_store, cx| {
             dap_store.shutdown_session(session.read(cx).session_id(), cx)