Skip to content

Commit

Permalink
Reveal the selected item when cycling a picker's selection (zed-indus…
Browse files Browse the repository at this point in the history
…tries#12950)

Release Notes:

- Fixed a bug where the selected tab was not always shown when cycling
between tabs with `ctrl-tab`.
  • Loading branch information
maxbrunsfeld authored and fallenwood committed Jun 18, 2024
1 parent 4584773 commit de83594
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 1 deletion.
1 change: 1 addition & 0 deletions crates/file_finder/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ editor = { workspace = true, features = ["test-support"] }
env_logger.workspace = true
gpui = { workspace = true, features = ["test-support"] }
language = { workspace = true, features = ["test-support"] }
picker = { workspace = true, features = ["test-support"] }
serde_json.workspace = true
theme = { workspace = true, features = ["test-support"] }
workspace = { workspace = true, features = ["test-support"] }
39 changes: 39 additions & 0 deletions crates/file_finder/src/file_finder_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1747,6 +1747,45 @@ async fn test_extending_modifiers_does_not_confirm_selection(cx: &mut gpui::Test
active_file_picker(&workspace, cx);
}

#[gpui::test]
async fn test_repeat_toggle_action(cx: &mut gpui::TestAppContext) {
let app_state = init_test(cx);
app_state
.fs
.as_fake()
.insert_tree(
"/test",
json!({
"00.txt": "",
"01.txt": "",
"02.txt": "",
"03.txt": "",
"04.txt": "",
"05.txt": "",
}),
)
.await;

let project = Project::test(app_state.fs.clone(), ["/test".as_ref()], cx).await;
let (workspace, cx) = cx.add_window_view(|cx| Workspace::test_new(project, cx));

cx.dispatch_action(Toggle::default());
let picker = active_file_picker(&workspace, cx);
picker.update(cx, |picker, _| {
assert_eq!(picker.delegate.selected_index, 0);
assert_eq!(picker.logical_scroll_top_index(), 0);
});

// When toggling repeatedly, the picker scrolls to reveal the selected item.
cx.dispatch_action(Toggle::default());
cx.dispatch_action(Toggle::default());
cx.dispatch_action(Toggle::default());
picker.update(cx, |picker, _| {
assert_eq!(picker.delegate.selected_index, 3);
assert_eq!(picker.logical_scroll_top_index(), 3);
});
}

async fn open_close_queried_buffer(
input: &str,
expected_matches: usize,
Expand Down
7 changes: 7 additions & 0 deletions crates/gpui/src/elements/uniform_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,13 @@ impl UniformListScrollHandle {
pub fn scroll_to_item(&mut self, ix: usize) {
self.deferred_scroll_to_item.replace(Some(ix));
}

/// Get the index of the topmost visible child.
pub fn logical_scroll_top_index(&self) -> usize {
self.deferred_scroll_to_item
.borrow()
.unwrap_or_else(|| self.base_handle.logical_scroll_top().0)
}
}

impl Styled for UniformList {
Expand Down
3 changes: 3 additions & 0 deletions crates/picker/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ workspace = true
path = "src/picker.rs"
doctest = false

[features]
test-support = []

[dependencies]
anyhow.workspace = true
editor.workspace = true
Expand Down
12 changes: 11 additions & 1 deletion crates/picker/src/picker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ impl<D: PickerDelegate> Picker<D> {
let count = self.delegate.match_count();
let index = self.delegate.selected_index();
let new_index = if index + 1 == count { 0 } else { index + 1 };
self.set_selected_index(new_index, false, cx);
self.set_selected_index(new_index, true, cx);
cx.notify();
}

Expand Down Expand Up @@ -538,6 +538,16 @@ impl<D: PickerDelegate> Picker<D> {
.into_any_element(),
}
}

#[cfg(any(test, feature = "test-support"))]
pub fn logical_scroll_top_index(&self) -> usize {
match &self.element_container {
ElementContainer::List(state) => state.logical_scroll_top().item_ix,
ElementContainer::UniformList(scroll_handle) => {
scroll_handle.logical_scroll_top_index()
}
}
}
}

impl<D: PickerDelegate> EventEmitter<DismissEvent> for Picker<D> {}
Expand Down

0 comments on commit de83594

Please sign in to comment.