From bcfc9e44376e8d8d9d81235727ba83597fd7f1ad Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Tue, 25 Mar 2025 00:36:09 +0100 Subject: [PATCH] debugger: Improve variable list keyboard navigation (#27308) 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 --- .../src/session/running/variable_list.rs | 69 +++--- crates/debugger_ui/src/tests/variable_list.rs | 213 ++++++++++++++++++ 2 files changed, 256 insertions(+), 26 deletions(-) diff --git a/crates/debugger_ui/src/session/running/variable_list.rs b/crates/debugger_ui/src/session/running/variable_list.rs index b931073bfd..f75d785f34 100644 --- a/crates/debugger_ui/src/session/running/variable_list.rs +++ b/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.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.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, ) { 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, ) { - 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(); } })) diff --git a/crates/debugger_ui/src/tests/variable_list.rs b/crates/debugger_ui/src/tests/variable_list.rs index 6c294447f0..136c48a150 100644 --- a/crates/debugger_ui/src/tests/variable_list.rs +++ b/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)