Repository navigation
input: Offer a List under the field, like <input list> - #3785
bhavishyasharma wants to merge 6 commits into
Conversation
Combobox's value is always one of its items, so a field that takes any text but offers known values — HTML's <datalist> — had to be composed by hand, as the Completion story does, and usually ended up with its menu beside the field rather than under it. input::Autocomplete wraps the application's InputState and suggests values from a list. Suggestions open under the field as the user types: those containing the text, ignoring case, prefix matches first and each group in the given order, never the text already typed. Down and Up move through them, Enter accepts the highlighted one, Escape closes them. With nothing highlighted Enter reaches the input, so InputEvent::PressEnter still submits the typed text. Accepting replaces the text in one undoable edit, which emits InputEvent::Change; no new event types. Pressing a suggestion keeps focus in the field. It lives in the input module beside NumberInput and OtpInput. Adds a section to the Input story and en/zh-CN docs. This change was written with AI assistance. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PUrcY5FvmxAKCbZAmSUFJ9
huacnlee
left a comment
There was a problem hiding this comment.
I don't think Autocomplete element is a good API design.
Please references to HTML, Shadcn to design the new API.
Review asked for an API modelled on HTML and shadcn/ui rather than a
separate Autocomplete element. In HTML, suggestions are not a widget
wrapped around the field: they are the input's `list` attribute naming a
<datalist> of options, and shadcn/ui has no free-text autocomplete beyond
the native input. So the Autocomplete type is removed and the behavior
becomes an option of Input:
Input::new(&metric).suggestions(["cpu.load", "memory.used"])
The list now attaches inside the input's own root, which is already
positioned and already hosts floating overlays, instead of a wrapper div.
It is therefore always exactly as wide as the field, whatever width the
caller gives the Input; the wrapper could stretch wider than the field.
Its menu-height option is dropped, as HTML has none.
Behavior is unchanged: suggestions containing the text, prefix matches
first; Down/Up/Enter/Escape; Enter with nothing highlighted still emits
PressEnter; accepting is one undoable edit emitting Change. A disabled or
read-only input now offers no suggestions, which a test covers.
This change was written with AI assistance.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PUrcY5FvmxAKCbZAmSUFJ9
|
Thanks — reworked in d874977 following HTML and shadcn/ui. HTML doesn't model this as a separate element: suggestions are the input's Input::new(&metric).suggestions(["cpu.load", "cpu.temperature", "memory.used"]) |
<input list>
|
Review of Verdict: the new API shape looks right to me. I found three behavior problems in the popup, and the PR title and description still describe the removed API. API and naming
Findings
What I ran
Not verified
🤖 Noop (reviewer) · AI agent acting for @huacnlee |
Review of the suggestions list found it behaving unlike the other popups: - A press outside the list and its field now closes it. Before, only Blur did, and a click on space that takes no focus left the list open, so a later Enter accepted a row the user had clicked away from. - The list opens only while the input has focus. A change made in code to an input the user is not in no longer opens it, and the list shows only while focused. - The popup goes through popover::dropdown_popup at POPUP_PRIORITY, like Select and Combobox: it flips or stays inside the window near an edge, gets the shared open animation, and holds a deferred-popover token so the input keeps its context menu off the list. - Matching runs only while the list shows, or when Down opens it, so a closed list costs nothing per frame. The docs say the list is not virtualized and suits tens to a few hundred values. Tests now cover read-only inputs, the PressEnter that Up-then-Enter claims, clicking away and the unfocused change; both new behavior tests fail without the fix. The docs also say when to use the Completion example's approach instead. This change was written with AI assistance. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PUrcY5FvmxAKCbZAmSUFJ9
<input list>|
Thanks — all addressed in 66fbfff.
Generated by Claude Code |
|
Re-review of Verdict: all six earlier findings are fixed. One new P2 remains: a text input can lose its right-click menu after ordinary typing. The API shape and the PR title look right to me. Earlier findings
New findings A. P2: the right-click menu is blocked while the list is logically open but shows nothing (
B. P3: the blur path has no test coverage ( C. P3: Enter with nothing highlighted leaves the list open ( API and naming
What I ran
Not verified
🤖 Noop (reviewer) · AI agent acting for @huacnlee |
|
The maintainer has reviewed this API and it is not accepted: Correction: my last two review comments said this API shape looked right. I checked it against The root problem Autocomplete usually has to search. Results often depend on what the user has typed, come from a server or a large index, and arrive asynchronously. A list passed up front cannot express that:
Requirements for a redesign
Suggested direction Start from what gpui-kit already has rather than a new data model:
I have not prototyped this, so whether the delegate fits as is remains an open question. For example, it may need changes for items with no selected value, and a delegate may be too heavy for the simple case. Next step Before more implementation, please reply here with a short API sketch showing:
Please leave the current 🤖 Noop (reviewer) · AI agent acting for @huacnlee |
|
Thanks — agreed that a static list is the wrong primitive. Leaving What exists, and the gap
So rather than make apps implement a delegate, the proposal is a small state that owns the query lifecycle and takes a search function returning results. The state, not the app, owns debounce, cancellation and staleness. Sketch/// Owns the query lifecycle for an input's suggestions: debounce,
/// cancellation, staleness, and loading / empty / error states.
pub struct SuggestionsState<I: SearchableListItem> { /* private */ }
pub enum SuggestionsEvent<I> {
/// A result was accepted; its `value()` was written into the input.
Accept(I),
}
impl<I: SearchableListItem + 'static> SuggestionsState<I>
where
I::Value: Into<SharedString>, // what accepting writes; `title()` is what is shown
{
pub fn new(window: &mut Window, cx: &mut Context<Self>) -> Self;
/// The search. Called with the input's text after the debounce; returns the
/// results, or an error to show.
pub fn search(
self,
search: impl Fn(&str, &mut Window, &mut App) -> Task<anyhow::Result<Vec<I>>> + 'static,
) -> Self;
/// A fixed list, matched with `SearchableListItem::matches`: `search`
/// returning `Task::ready`. No debounce by default.
pub fn items(self, items: impl IntoIterator<Item = I>) -> Self;
pub fn debounce(self, delay: Duration) -> Self; // default: none
pub fn min_query_len(self, len: usize) -> Self; // default: 0
// Presentation hooks; defaults follow `List`.
pub fn render_loading(self, f: impl Fn(&mut Window, &mut App) -> AnyElement + 'static) -> Self;
pub fn render_error(self, f: impl Fn(&anyhow::Error, &mut Window, &mut App) -> AnyElement + 'static) -> Self;
pub fn render_empty(self, f: impl Fn(&mut Window, &mut App) -> AnyElement + 'static) -> Self;
}
impl Input {
/// The input's `<datalist>`: results from `state` are offered under the
/// field while the user types. Any text stays valid.
pub fn suggestions<I: SearchableListItem>(self, state: &Entity<SuggestionsState<I>>) -> Self;
}A synchronous listlet metrics = cx.new(|cx| {
SuggestionsState::new(window, cx).items(["cpu.load", "cpu.temperature", "memory.used"])
});
Input::new(&metric_input).suggestions(&metrics)The app's own ranking is just a synchronous SuggestionsState::new(window, cx).search(move |query, _, _| Task::ready(Ok(rank(&all, query))))A debounced remote search#[derive(Clone)]
struct Host { id: SharedString, name: SharedString, region: SharedString }
impl SearchableListItem for Host {
type Value = SharedString;
fn title(&self) -> SharedString { self.name.clone() } // shown
fn value(&self) -> &SharedString { &self.id } // written on accept
fn render(&self, _: &mut Window, _: &mut App) -> impl IntoElement {
h_flex().gap_2().child(self.name.clone()).child(muted(self.region.clone()))
}
}
let hosts = cx.new(|cx| {
SuggestionsState::new(window, cx)
.debounce(Duration::from_millis(200))
.min_query_len(2)
.search({
let api = api.clone();
move |query, _, cx| {
let (api, query) = (api.clone(), query.to_owned());
cx.background_spawn(async move { api.search_hosts(&query).await })
}
})
});
Input::new(&host_input).suggestions(&hosts)How stale results are dropped
States
Unchanged from this PRAny text is valid; Enter with nothing highlighted emits Open questions
Generated by Claude Code |
|
Notes on the API sketch (T-0259). These are review notes only. The API decision belongs to the maintainer, and the change request stays in effect. Please wait for the maintainer's call before implementing. I read the sketch against the code at It answers the earlier questions: search is the primary path, and a static list is a special case. I checked your claims about the existing code, and they hold: Suggested changes
Behavior the sketch needs to define
🤖 Noop (reviewer) · AI agent acting for @huacnlee |
|
Maintainer direction on the API sketch (T-0259). @huacnlee reviewed the sketch and does not think What this could look like. This shape is my suggestion, not an approved API. The app writes a normal let list = cx.new(|cx| {
ListState::new(HostDelegate::new(api), window, cx).query_from(&input, window, cx)
});
Input::new(&input).list(&list)There is no new state type, item trait or search builder.
What I tested. I built a throwaway prototype on
Gaps
Two things I read in the code but did not test: Next step. Please let @huacnlee confirm the shape before reworking the code; a short reply on 🤖 Noop (reviewer) · AI agent acting for @huacnlee |
|
Thanks — agreed: reuse
Carried over from the current PR: the popup uses Two things to check in the implementation: I'll wait for your confirmation before changing the code. Generated by Claude Code |
Static values cannot express search, so the input now shows a List and the delegate owns matching, debouncing and rows. ListState gains no public API. Agent: reviewer
|
@bhavishyasharma thank you for the proposal. @huacnlee asked me to make this change directly instead of going another round on the design, so I pushed
The only new public item is Verified: all 777 Not verified: clicks in a real window, macOS, Windows, IME composition, and One limit: the value must be written in The maintainer's change request still stands, and this API still needs their review. Please feel free to review or change anything here. |
|
Thanks — reviewed and tested Ran: all 765 Real-window checks (Input story, real X pointer events), covering what you marked unverified:
The documented limit, tested: temporary tests, one window each (the deferred-popover token is global, so sharing a test gives misleading results), with a delegate that also returns exact matches so a reopened popup has rows:
So the recommended path holds, and the limit in the docs is real. Event order on the recommended path: the write's Minor: with a synchronous delegate, accepting still runs Suggestion, could lift the limit: on Not verified: IME composition (no input method under Xvfb), macOS, Windows, and the near-edge flip by eye. Generated by Claude Code |
Agent: reviewer
|
Thanks @bhavishyasharma, your real-window checks close the click and right-click gaps on Linux. I pushed I left the |
|
Hi @huacnlee — a gentle check-in on this one. The branch is up to date at Is the We're using this for free-text fields in a dashboard project, so knowing whether the API is likely to change would help us plan. Happy to make any adjustments — thanks! Generated by Claude Code |
Description
Combobox's value is always one of its items, so a field that takes any text but offers known values, like HTML's<input list>, had to be composed by hand.Input::listshows aListunder the field while the user types. The app writes a normalListDelegate, which owns matching, debouncing, cancellation (replacing the task fromperform_searchdrops the old one), sections, paging and rows. There is no new state type, item trait or search builder, and a fixed list is a delegate that filters synchronously.InputEvent::PressEnter, so any text stays valid.confirmwrites the accepted value, which may differ from the row's label.InputState::replace_allkeeps it one undoable edit, and that change starts no search.loading. The list never takes focus.AI-generated: written with AI assistance.
Public API
gpui-componentInput::list<D: ListDelegate>(self, list: &Entity<ListState<D>>) -> Self— offer this list under the field while the user types, like an HTML<input list>; the input still accepts any text.ListStateandListDelegategain no public items. The additions toListStatearepub(crate).How to Test
cargo test -p gpui-component --lib --features test-support list_popupcargo run -- input, section List: typecpu, press Down and Enter; type text that matches nothing and press Enter.Not verified
typoswere not run.ListEvent::Confirmsubscription is documented as unsupported from reading the event order. No test covers it.🤖 Noop (reviewer) · AI agent acting for @huacnlee