T23c round 2 fix

- a trigger opens only its word, so a path keeps its slashes in the query
- suggestions reveal the cursor once the list has its bounds: Popup takes a
  row to reveal, and a canvas inside the list asks for it and one more frame
This commit is contained in:
ZacharyZhang-NY committed 2026-09-26 07:10:53 -04:00
1 parent 4fafba4b20
commit 1a00e19591
9 files changed
+109 -31

No files matched your search

+1
View File
@@ -262,3 +262,4 @@ Per-component lines live in `tasks/`. Tags there:
| T23b | 2 | FAIL | the gallery's narrow passage built a vec of one range, which clippy refuses, so the checks failed | | T23b | 2 | FAIL | the gallery's narrow passage built a vec of one range, which clippy refuses, so the checks failed |
| T23b | 3 | PASS | | | T23b | 3 | PASS | |
| T23c | 1 | FAIL | suggestion arrows landed on disabled rows and left the cursor out of view; a dot ended a file mention; the drop veil never showed, since gpui styles a drag-over only on an element with a hitbox; a parameter's places came from its step alone; the gallery's prompt menu and Reset only logged | | T23c | 1 | FAIL | suggestion arrows landed on disabled rows and left the cursor out of view; a dot ended a file mention; the drop veil never showed, since gpui styles a drag-over only on an element with a hitbox; a parameter's places came from its step alone; the gallery's prompt menu and Reset only logged |
| T23c | 2 | FAIL | a slash inside a path read as a new trigger, closing the file suggestions; a list whose first rows were disabled opened with its cursor out of view |
+46 -11
View File
@@ -30,9 +30,13 @@ impl Render for Compose {
.commands(self.commands.clone(), move |value, _, cx| { .commands(self.commands.clone(), move |value, _, cx| {
command.update(cx, |compose, _| compose.picked.push(value.clone())) command.update(cx, |compose, _| compose.picked.push(value.clone()))
}) })
.context([Choice::new("lift", "lift.rs")], move |value, _, cx| { .context(
mention.update(cx, |compose, _| compose.picked.push(value.clone())) [
}) Choice::new("lift", "lift.rs"),
Choice::new("src", "src/lift.rs"),
],
move |value, _, cx| mention.update(cx, |compose, _| compose.picked.push(value.clone())),
)
.on_drop(|_, _, _| {}); .on_drop(|_, _, _| {});
let input = if self.busy { let input = if self.busy {
input.busy(|_, _| {}) input.busy(|_, _| {})
@@ -170,14 +174,19 @@ fn a_file_name_keeps_its_suggestions_past_a_dot(cx: &mut TestAppContext) {
} }
#[gpui::test] #[gpui::test]
fn arrows_pass_disabled_rows_and_keep_the_cursor_in_view(cx: &mut TestAppContext) { fn a_path_keeps_its_suggestions_past_a_slash(cx: &mut TestAppContext) {
let (compose, cx) = open(false, numbered(&[1]), cx); let (compose, cx) = open(false, summarize(), cx);
cx.simulate_input("/"); cx.simulate_input("see @src/");
frames(cx); settle(cx);
for _ in 0..16 { cx.simulate_keystrokes("enter");
cx.simulate_keystrokes("down"); settle(cx);
frames(cx); assert_eq!(text(&compose, cx), "see ");
} assert_eq!(picked(&compose, cx), ["src"]);
assert_eq!(compose.read_with(cx, |compose, _| compose.sent), 0);
}
/// Clicks near the foot of the command list that hangs under the slash.
fn click_list_foot(compose: &Entity<Compose>, cx: &mut VisualTestContext) {
let (anchor, height) = cx.update(|window, cx| { let (anchor, height) = cx.update(|window, cx| {
let anchor = compose.read(cx).field.read(cx).bounds_for(0); let anchor = compose.read(cx).field.read(cx).bounds_for(0);
let height = cx.theme().list_max_height().to_pixels(window.rem_size()); let height = cx.theme().list_max_height().to_pixels(window.rem_size());
@@ -189,6 +198,32 @@ fn arrows_pass_disabled_rows_and_keep_the_cursor_in_view(cx: &mut TestAppContext
); );
cx.simulate_mouse_move(foot, None, Modifiers::none()); cx.simulate_mouse_move(foot, None, Modifiers::none());
cx.simulate_click(foot, Modifiers::none()); cx.simulate_click(foot, Modifiers::none());
}
#[gpui::test]
fn the_list_opens_on_its_first_enabled_row_in_view(cx: &mut TestAppContext) {
let disabled: Vec<usize> = (0..17).collect();
let (compose, cx) = open(false, numbered(&disabled), cx);
cx.simulate_input("/");
frames(cx);
click_list_foot(&compose, cx);
assert_eq!(
picked(&compose, cx),
["c17"],
"the list opens scrolled to its cursor"
);
}
#[gpui::test]
fn arrows_pass_disabled_rows_and_keep_the_cursor_in_view(cx: &mut TestAppContext) {
let (compose, cx) = open(false, numbered(&[1]), cx);
cx.simulate_input("/");
frames(cx);
for _ in 0..16 {
cx.simulate_keystrokes("down");
frames(cx);
}
click_list_foot(&compose, cx);
assert_eq!( assert_eq!(
picked(&compose, cx), picked(&compose, cx),
["c17"], ["c17"],
+1
View File
@@ -256,6 +256,7 @@ impl RenderOnce for Combobox {
pick, pick,
dismiss: None, dismiss: None,
scroll: Some(&scroll), scroll: Some(&scroll),
reveal: None,
} }
.render(window, cx), .render(window, cx),
) )
+1
View File
@@ -303,6 +303,7 @@ impl RenderOnce for PhoneInput {
}) })
})), })),
scroll: None, scroll: None,
reveal: None,
} }
.render(window, cx), .render(window, cx),
) )
+36 -18
View File
@@ -7,7 +7,7 @@ use gpui::{
use super::{ use super::{
Highlight, Input, TextInput, Highlight, Input, TextInput,
options::{Choice, Pick, Popup, step}, options::{Choice, Pick, Popup, Run, step},
text::{Down, Enter, Up}, text::{Down, Enter, Up},
}; };
use crate::theme::ActiveTheme; use crate::theme::ActiveTheme;
@@ -19,7 +19,7 @@ pub(crate) fn one_word(ch: char) -> bool {
ch.is_alphanumeric() || ch == '_' ch.is_alphanumeric() || ch == '_'
} }
/// The trigger being typed, its query all `word` characters: its byte offset and char, ending at `caret`. /// The trigger that opens the word ending at `caret`, its query all `word` characters: its byte offset and char.
pub(crate) fn active_trigger( pub(crate) fn active_trigger(
text: &str, text: &str,
caret: usize, caret: usize,
@@ -27,17 +27,14 @@ pub(crate) fn active_trigger(
word: fn(char) -> bool, word: fn(char) -> bool,
) -> Option<(usize, char)> { ) -> Option<(usize, char)> {
let before = &text[..caret]; let before = &text[..caret];
let (ix, ch) = before let start = before
.char_indices() .char_indices()
.rev() .rev()
.take_while(|(_, ch)| !ch.is_whitespace()) .find(|(_, ch)| ch.is_whitespace())
.find(|(_, ch)| triggers.contains(ch))?; .map_or(0, |(ix, ch)| ix + ch.len_utf8());
let opens = before[..ix] let mut chars = before[start..].chars();
.chars() let trigger = chars.next().filter(|ch| triggers.contains(ch))?;
.next_back() chars.all(word).then_some((start, trigger))
.is_none_or(char::is_whitespace);
let query = before[ix + ch.len_utf8()..].chars().all(word);
(opens && query).then_some((ix, ch))
} }
/// Replaces the trigger and its query, `at..caret`, with `text`, as one undo step. /// Replaces the trigger and its query, `at..caret`, with `text`, as one undo step.
@@ -107,6 +104,7 @@ struct Picking {
highlighted: usize, highlighted: usize,
dismissed: Option<usize>, dismissed: Option<usize>,
scroll: ScrollHandle, scroll: ScrollHandle,
revealed: Option<usize>,
} }
/// The row the cursor rests on: `at` kept in range and off disabled rows. /// The row the cursor rests on: `at` kept in range and off disabled rows.
@@ -118,16 +116,15 @@ fn usable(rows: &[Choice], at: usize) -> usize {
} }
} }
/// Moves the cursor `by` rows past disabled ones and scrolls it into view. /// Moves the cursor `by` rows past disabled ones.
fn move_cursor(picking: &Entity<Picking>, rows: &[Choice], by: isize, cx: &mut App) { fn move_cursor(picking: &Entity<Picking>, rows: &[Choice], by: isize, cx: &mut App) {
picking.update(cx, |picking, cx| { picking.update(cx, |picking, cx| {
picking.highlighted = step(rows, picking.highlighted, by); picking.highlighted = step(rows, picking.highlighted, by);
picking.scroll.scroll_to_item(picking.highlighted);
cx.notify(); cx.notify();
}); });
} }
/// Rows offered at a trigger in a field's text, and what a pick does: arrows choose past disabled rows, Enter or a press picks, Escape dismisses until the trigger ends. /// Rows offered at a trigger in a field's text, and what a pick does: arrows choose past disabled rows, the cursor stays in view, Enter or a press picks, Escape dismisses until the trigger ends.
pub(crate) struct Suggestions { pub(crate) struct Suggestions {
pub id: ElementId, pub id: ElementId,
pub state: Entity<TextInput>, pub state: Entity<TextInput>,
@@ -152,10 +149,23 @@ impl Suggestions {
&& !self.rows.is_empty() && !self.rows.is_empty()
&& self.trigger.is_some_and(|ix| dismissed != Some(ix)); && self.trigger.is_some_and(|ix| dismissed != Some(ix));
let rows: Rc<[Choice]> = self.rows.into(); let rows: Rc<[Choice]> = self.rows.into();
let shown = open && anchor.is_some();
let highlighted = usable(&rows, picking.read(cx).highlighted); let highlighted = usable(&rows, picking.read(cx).highlighted);
if highlighted != picking.read(cx).highlighted { let revealed = picking.read(cx).revealed.filter(|_| shown);
picking.update(cx, |picking, _| picking.highlighted = highlighted); let stored = (picking.read(cx).highlighted, picking.read(cx).revealed);
if (highlighted, revealed) != stored {
picking.update(cx, |picking, _| {
picking.highlighted = highlighted;
picking.revealed = revealed;
});
} }
let reveal = (shown && revealed != Some(highlighted)).then(|| {
let done = picking.clone();
let mark: Run = Rc::new(move |_, cx| {
done.update(cx, |picking, _| picking.revealed = Some(highlighted))
});
(highlighted, mark)
});
let scroll = picking.read(cx).scroll.clone(); let scroll = picking.read(cx).scroll.clone();
log::debug!( log::debug!(
"suggestions {:?}: open {open}, {} rows, anchor {anchor:?}", "suggestions {:?}: open {open}, {} rows, anchor {anchor:?}",
@@ -214,6 +224,7 @@ impl Suggestions {
pick: self.pick, pick: self.pick,
dismiss: None, dismiss: None,
scroll: Some(&scroll), scroll: Some(&scroll),
reveal,
} }
.render(window, cx), .render(window, cx),
) )
@@ -333,8 +344,15 @@ mod tests {
assert_eq!(at("hi @ada lovelace"), None); assert_eq!(at("hi @ada lovelace"), None);
assert_eq!(at("go #rel"), Some((3, '#'))); assert_eq!(at("go #rel"), Some((3, '#')));
assert_eq!(at("see @lift."), None, "a handle is one word"); assert_eq!(at("see @lift."), None, "a handle is one word");
let file = active_trigger("see @lift.rs", 12, &marks, |ch| !ch.is_whitespace()); let file =
assert_eq!(file, Some((4, '@')), "a file name holds dots"); |text: &str| active_trigger(text, text.len(), &['/', '@'], |ch| !ch.is_whitespace());
assert_eq!(
file("see @lift.rs"),
Some((4, '@')),
"a file name holds dots"
);
assert_eq!(file("see @src/"), Some((4, '@')), "a path holds slashes");
assert_eq!(file("/usr/bin"), Some((0, '/')));
} }
#[test] #[test]
+1
View File
@@ -222,6 +222,7 @@ impl RenderOnce for MultiSelect {
Picker::show(&close, false, at, cx) Picker::show(&close, false, at, cx)
})), })),
scroll: Some(&scroll), scroll: Some(&scroll),
reveal: None,
} }
.render(window, cx), .render(window, cx),
) )
+21 -2
View File
@@ -3,7 +3,7 @@ use std::rc::Rc;
use gpui::{ use gpui::{
Animation, AnimationExt, AnyElement, App, Bounds, Corner, Div, ElementId, InteractiveElement, Animation, AnimationExt, AnyElement, App, Bounds, Corner, Div, ElementId, InteractiveElement,
IntoElement, MouseButton, ParentElement, Pixels, ScrollHandle, SharedString, Stateful, IntoElement, MouseButton, ParentElement, Pixels, ScrollHandle, SharedString, Stateful,
StatefulInteractiveElement, Styled, Window, anchored, deferred, div, prelude::*, StatefulInteractiveElement, Styled, Window, anchored, canvas, deferred, div, prelude::*,
}; };
use crate::{ use crate::{
@@ -221,6 +221,8 @@ pub(crate) struct Popup<'a> {
pub pick: Pick, pub pick: Pick,
pub dismiss: Option<Run>, pub dismiss: Option<Run>,
pub scroll: Option<&'a ScrollHandle>, pub scroll: Option<&'a ScrollHandle>,
/// A row to scroll into view once the list has its bounds, and what marks it done.
pub reveal: Option<(usize, Run)>,
} }
impl Popup<'_> { impl Popup<'_> {
@@ -259,7 +261,24 @@ impl Popup<'_> {
.when_some(self.dismiss, |list, dismiss| { .when_some(self.dismiss, |list, dismiss| {
list.on_mouse_down_out(move |_, window, cx| dismiss(window, cx)) list.on_mouse_down_out(move |_, window, cx| dismiss(window, cx))
}) })
.children(rows); .children(rows)
.when_some(
self.scroll.zip(self.reveal),
|list, (handle, (ix, done))| {
let handle = handle.clone();
list.child(
canvas(
move |_, window, cx| {
handle.scroll_to_item(ix);
window.request_animation_frame();
done(window, cx);
},
|_, _, _, _| {},
)
.absolute(),
)
},
);
float(anchor, self.rows.len(), list, window, cx) float(anchor, self.rows.len(), list, window, cx)
} }
} }
+1
View File
@@ -113,6 +113,7 @@ impl RenderOnce for SearchInput {
pick, pick,
dismiss: None, dismiss: None,
scroll: None, scroll: None,
reveal: None,
} }
.render(window, cx), .render(window, cx),
) )
+1
View File
@@ -224,6 +224,7 @@ pub(crate) fn listing(
Picker::show(&close, false, at, cx) Picker::show(&close, false, at, cx)
})), })),
scroll: Some(&scroll), scroll: Some(&scroll),
reveal: None,
} }
.render(window, cx), .render(window, cx),
) )