feat: add combobox filter

2026-11-06 21:27:31 +07:00
parent 265d2d129b
commit 0ad5c3fe90
8 changed files with 256 additions and 65 deletions
@@ -5,7 +5,8 @@ use std::time::Duration;

 use gpui::prelude::FluentBuilder as _;
 use gpui::{
-    AnyView, App, ClipboardItem, ElementId, Entity, IntoElement, RenderOnce, SharedString, Window,
+    AnyView, App, ClipboardItem, ElementId, Entity, IntoElement, RenderOnce, SharedString, Task,
+    Window,
 };
 use gpui_native_icons::IconRole;
 use gpui_native_theme::Size;
@@ -92,11 +93,22 @@ impl Sizable for ClipboardButton {
    }
 }

+/// The transient copied marker keyed per button.
+struct CopiedState {
+    generation: u64,
+    tasks: Vec<Task<()>>,
+}
+
 impl RenderOnce for ClipboardButton {
    fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement {
-        let copied: Entity<Option<u64>> =
-            window.use_keyed_state((self.id.clone(), "clipboard-copied"), cx, |_, _| None);
-        let is_copied = copied.read(cx).is_some();
+        let copied: Entity<CopiedState> =
+            window.use_keyed_state((self.id.clone(), "clipboard-copied"), cx, |_, _| {
+                CopiedState {
+                    generation: 0,
+                    tasks: Vec::new(),
+                }
+            });
+        let is_copied = copied.read(cx).generation != 0;

        let selector = if is_copied {
            "clipboard-copied"
@@ -126,30 +138,29 @@ impl RenderOnce for ClipboardButton {
                    cx.write_to_clipboard(ClipboardItem::new_string(value.to_string()));

                    let generation = copied.update(cx, |state, cx| {
-                        let generation = state.map_or(1, |generation| generation.wrapping_add(1));
-                        *state = Some(generation);
+                        state.generation += 1;
                        cx.notify();
-                        generation
+                        state.generation
                    });

-                    // Clear the copied state after the duration expires
-                    window
-                        .spawn(cx, {
-                            let copied = copied.clone();
-                            async move |cx| {
-                                cx.background_executor().timer(COPIED_DURATION).await;
-                                cx.update(|_, cx| {
-                                    copied.update(cx, |state, cx| {
-                                        if *state == Some(generation) {
-                                            *state = None;
-                                            cx.notify();
-                                        }
-                                    })
-                                })
-                                .ok();
-                            }
+                    let weak = copied.downgrade();
+                    let task = window.spawn(cx, async move |cx| {
+                        cx.background_executor().timer(COPIED_DURATION).await;
+                        cx.update(|_, cx| {
+                            weak.update(cx, |state, cx| {
+                                if state.generation == generation {
+                                    state.generation = 0;
+                                    cx.notify();
+                                }
+                            })
+                            .ok();
                        })
-                        .detach();
+                        .ok();
+                    });
+
+                    copied.update(cx, |state, _| {
+                        state.tasks.push(task);
+                    });

                    // Invoke the callback after copying
                    if let Some(on_copied) = &on_copied {
@@ -12,8 +12,8 @@ use gpui::prelude::FluentBuilder as _;
 use gpui::{
    App, Bounds, Context, ElementId, Entity, EventEmitter, InteractiveElement as _, IntoElement,
    KeyBinding, MouseButton, MouseDownEvent, ParentElement as _, Pixels, Render, Role,
-    ScrollHandle, SharedString, StatefulInteractiveElement as _, Styled as _, Subscription, Window,
-    actions, canvas, deferred, div, px,
+    ScrollHandle, SharedString, StatefulInteractiveElement as _, Styled as _, Subscription, Task,
+    Window, actions, canvas, deferred, div, px,
 };
 use gpui_native_icons::{IconRole, icon};
 use gpui_native_theme::{ActiveTheme as _, Size};
@@ -59,12 +59,12 @@ pub enum UnmatchedText {

 /// An option for a [`Combobox`]: a value and the label the filter matches on.
 pub type ComboboxOption<T> = (T, SharedString);
-/// Reports the value whenever it changes — including when typing clears it.
+/// Reports the value whenever it changes;
 pub type ComboboxHandler<T> = Rc<dyn Fn(Option<T>, &mut Window, &mut App)>;
 /// Receives unmatched text when the field loses focus in create mode.
 pub type ComboboxCreateHandler = Rc<dyn Fn(SharedString, &mut Window, &mut App)>;
-/// A filter over an option: the query, the value, the label.
-type Filter<T> = Rc<dyn Fn(&str, &T, &SharedString) -> bool>;
+/// Decides which options survive a query: the query, the value, the label.
+pub type ComboboxFilter<T> = Rc<dyn Fn(&str, &T, &SharedString) -> bool>;

 /// Builder for a combobox. Pass it to [`ComboboxState::new`].
 pub struct Combobox<T>
@@ -78,6 +78,7 @@ where
    placeholder: SharedString,
    on_change: Option<ComboboxHandler<T>>,
    on_create: Option<ComboboxCreateHandler>,
+    filter: Option<ComboboxFilter<T>>,
    unmatched: UnmatchedText,
    size: Size,
 }
@@ -99,6 +100,7 @@ where
            placeholder: SharedString::from("Select..."),
            on_change: None,
            on_create: None,
+            filter: None,
            unmatched: UnmatchedText::default(),
            size: Size::default(),
        }
@@ -138,6 +140,15 @@ where
        self
    }

+    /// Sets the matcher that decides which options survive a query.
+    pub fn filter<F>(mut self, filter: F) -> Self
+    where
+        F: Fn(&str, &T, &SharedString) -> bool + 'static,
+    {
+        self.filter = Some(Rc::new(filter));
+        self
+    }
+
    /// Leaves unmatched text in the field on blur.
    pub fn keep_unmatched_text(mut self) -> Self {
        self.unmatched = UnmatchedText::Keep;
@@ -175,9 +186,11 @@ where
    highlight: Option<usize>,
    on_change: Option<ComboboxHandler<T>>,
    on_create: Option<ComboboxCreateHandler>,
-    filter: Filter<T>,
+    filter: ComboboxFilter<T>,
    unmatched: UnmatchedText,
    size: Size,
+    /// Handler reports in flight; held so they die with the state.
+    tasks: Vec<Task<()>>,
    /// Keeps the query-event subscription tied to the state's lifetime.
    _subscription: Subscription,
 }
@@ -230,11 +243,12 @@ where
            highlight: None,
            on_change: combobox.on_change,
            on_create: combobox.on_create,
-            filter: Rc::new(|query, _value, label| {
-                label.to_lowercase().contains(&query.to_lowercase())
+            filter: combobox.filter.unwrap_or_else(|| {
+                Rc::new(|query, _value, label| label.to_lowercase().contains(&query.to_lowercase()))
            }),
            unmatched: combobox.unmatched,
            size: combobox.size,
+            tasks: Vec::new(),
            _subscription: subscription,
        }
    }
@@ -313,7 +327,11 @@ where
        }
        self.selected = value.clone();
        if let Some(on_change) = self.on_change.clone() {
-            on_change(value, window, cx);
+            let value = value.clone();
+            let task = window.spawn(cx, async move |cx| {
+                cx.update(|window, cx| on_change(value, window, cx)).ok();
+            });
+            self.tasks.push(task);
        }
        cx.emit(ComboboxChanged);
        cx.notify();
@@ -367,7 +385,11 @@ where
                if !text.is_empty()
                    && let Some(on_create) = self.on_create.clone()
                {
-                    on_create(SharedString::from(text), window, cx);
+                    let text = SharedString::from(text);
+                    let task = window.spawn(cx, async move |cx| {
+                        cx.update(|window, cx| on_create(text, window, cx)).ok();
+                    });
+                    self.tasks.push(task);
                }
            }
        }
@@ -11,7 +11,7 @@ use gpui::{
    DispatchPhase, ElementId, Entity, EntityInputHandler, EventEmitter, Font, HitboxBehavior,
    InteractiveElement as _, IntoElement, KeyBinding, KeyDownEvent, MouseButton, MouseDownEvent,
    MouseMoveEvent, MouseUpEvent, ParentElement as _, Pixels, RenderOnce, Role, ScrollDelta,
-    ScrollEvent, SharedString, StatefulInteractiveElement as _, Styled as _, Subscription,
+    ScrollEvent, SharedString, StatefulInteractiveElement as _, Styled as _, Subscription, Task,
    TextAlign, TextRun, UTF16Selection, Window, WrappedLine, actions, canvas, div, fill, point, px,
 };
 use gpui_native_icons::IconRole;
@@ -483,6 +483,8 @@ struct CaretBlink {
    visible: bool,
    active: bool,
    paused: bool,
+    /// Pending blink timers; held so they die with the controller.
+    tasks: Vec<Task<()>>,
 }

 impl CaretBlink {
@@ -493,6 +495,7 @@ impl CaretBlink {
            visible: true,
            active: false,
            paused: false,
+            tasks: Vec::new(),
        }
    }

@@ -550,7 +553,7 @@ impl CaretBlink {
    fn schedule_pause(&mut self, cx: &mut Context<Self>) {
        let generation = self.generation;
        let interval = self.interval;
-        cx.spawn(async move |this, cx| {
+        let task = cx.spawn(async move |this, cx| {
            cx.background_executor().timer(interval).await;
            this.update(cx, |this, cx| {
                if this.generation == generation {
@@ -558,8 +561,9 @@ impl CaretBlink {
                    this.tick(cx);
                }
            })
-        })
-        .detach();
+            .ok();
+        });
+        self.tasks.push(task);
    }
 }

@@ -7,7 +7,7 @@ use gpui::prelude::FluentBuilder as _;
 use gpui::{
    Animation, AnimationExt as _, App, Context, ElementId, Entity, InteractiveElement as _,
    IntoElement, ParentElement as _, RenderOnce, SharedString, StatefulInteractiveElement as _,
-    Styled as _, Window, deferred, div, ease_out_quint, px,
+    Styled as _, Task, Window, deferred, div, ease_out_quint, px,
 };
 use gpui_native_icons::{IconRole, icon};
 use gpui_native_theme::ActiveTheme as _;
@@ -107,6 +107,8 @@ pub struct Toasts {
    current: Option<ToastEntry>,
    exiting: Option<ToastEntry>,
    next_id: usize,
+    /// Pending dismiss and fade timers; held so they die with the store.
+    tasks: Vec<Task<()>>,
 }

 impl Toasts {
@@ -125,11 +127,11 @@ impl Toasts {
        cx.notify();

        if let Some(duration) = duration.filter(|duration| !duration.is_zero()) {
-            cx.spawn(async move |this, cx| {
+            let task = cx.spawn(async move |this, cx| {
                cx.background_executor().timer(duration).await;
                this.update(cx, |toasts, cx| toasts.dismiss(id, cx)).ok();
-            })
-            .detach();
+            });
+            self.tasks.push(task);
        }

        id
@@ -178,12 +180,11 @@ impl Toasts {
        cx.notify();

        let fade = cx.theme().motion.overlay;
-
-        cx.spawn(async move |this, cx| {
+        let task = cx.spawn(async move |this, cx| {
            cx.background_executor().timer(fade).await;
            this.update(cx, |toasts, cx| toasts.finish_exit(cx)).ok();
-        })
-        .detach();
+        });
+        self.tasks.push(task);
    }

    fn finish_exit(&mut self, cx: &mut Context<Self>) {
@@ -34,6 +34,25 @@ struct ComboHost {
 }

 fn combo_host(keep_unmatched: bool, create: bool) -> ComboHost {
+    combo_host_with(|builder, created| {
+        let builder = if keep_unmatched {
+            builder.keep_unmatched_text()
+        } else {
+            builder
+        };
+        if create {
+            builder.on_create(move |text, _, _| created.borrow_mut().push(text))
+        } else {
+            builder
+        }
+    })
+}
+
+/// Builds a host whose combobox the caller configures; the created-recorder
+/// is handed over for `on_create` wiring.
+fn combo_host_with(
+    configure: impl FnOnce(Combobox<SharedString>, CreatedRecorder) -> Combobox<SharedString>,
+) -> ComboHost {
    let cx = TestAppContext::single();
    // `init` registers the input bindings and then the combobox's, in the
    // load-bearing order.
@@ -47,19 +66,15 @@ fn combo_host(keep_unmatched: bool, create: bool) -> ComboHost {
        let created_for_builder = created.clone();
        let cell = state_cell.clone();
        cx.open_window(gpui::WindowOptions::default(), |window, cx| {
-            let mut builder = Combobox::new(
-                "test-combo",
-                "Fruit",
-                vec![option("main"), option("maintain"), option("release")],
-            )
-            .on_change(move |value, _, _| changes_for_builder.borrow_mut().push(value));
-            if keep_unmatched {
-                builder = builder.keep_unmatched_text();
-            }
-            if create {
-                builder = builder
-                    .on_create(move |text, _, _| created_for_builder.borrow_mut().push(text));
-            }
+            let builder = configure(
+                Combobox::new(
+                    "test-combo",
+                    "Fruit",
+                    vec![option("main"), option("maintain"), option("release")],
+                )
+                .on_change(move |value, _, _| changes_for_builder.borrow_mut().push(value)),
+                created_for_builder,
+            );
            let combobox = cx.new(|cx| ComboboxState::new(builder, window, cx));
            cell.borrow_mut().replace(combobox.clone());
            cx.new(|_| Host { combobox })
@@ -118,6 +133,7 @@ fn enter_commits_the_highlighted_option() {
    type_text(&mut host, "mai");

    host.visual.simulate_keystrokes("enter");
+    host.visual.run_until_parked();
    draw(&mut host.visual);

    assert_eq!(
@@ -143,6 +159,7 @@ fn typing_clears_the_value() {

    // Typing again first clears the value it no longer names.
    host.visual.simulate_keystrokes("r");
+    host.visual.run_until_parked();
    draw(&mut host.visual);
    assert_eq!(
        *host.changes.borrow(),
@@ -197,6 +214,7 @@ fn create_hands_unmatched_text_to_the_handler() {
    type_text(&mut host, "feature");

    host.visual.update(|window, cx| window.blur(cx));
+    host.visual.run_until_parked();
    draw(&mut host.visual);

    assert_eq!(
@@ -217,6 +235,7 @@ fn clicking_a_row_commits_it() {
        .expect("release row missing");
    host.visual
        .simulate_click(row.center(), gpui::Modifiers::default());
+    host.visual.run_until_parked();
    draw(&mut host.visual);

    assert_eq!(
@@ -225,3 +244,99 @@ fn clicking_a_row_commits_it() {
    );
    assert!(host.visual.debug_bounds("test-combo-panel").is_none());
 }
+
+/// Regression: handlers used to run inside an update of the combobox
+/// entity, so a handler that touched its own state re-entered and panicked.
+/// Both handlers now run on the executor and may update the state freely.
+#[test]
+fn handlers_may_update_their_own_combobox() {
+    let cx = TestAppContext::single();
+    cx.update(init);
+    let state_cell: Rc<RefCell<Option<Entity<ComboboxState<SharedString>>>>> =
+        Rc::new(RefCell::new(None));
+    let cell = state_cell.clone();
+    let window = cx.update(|cx| {
+        cx.open_window(gpui::WindowOptions::default(), |window, cx| {
+            let builder = Combobox::new(
+                "self-updating-combo",
+                "Label",
+                vec![option("bug"), option("docs")],
+            )
+            .on_create(move |text: SharedString, window, cx| {
+                let state = cell.borrow().clone().expect("state captured");
+                state.update(cx, |state, cx| {
+                    let mut options = state.options_snapshot();
+                    if !options.iter().any(|(value, _)| value == &text) {
+                        options.push((text.clone(), text.clone()));
+                        state.set_options(options, cx);
+                    }
+                    state.set_selected(Some(text), window, cx);
+                });
+            });
+            let combobox = cx.new(|cx| ComboboxState::new(builder, window, cx));
+            state_cell.borrow_mut().replace(combobox.clone());
+            cx.new(|_| Host { combobox })
+        })
+        .unwrap()
+    });
+    let state = state_cell.borrow().clone().expect("state captured");
+    let mut visual = VisualTestContext::from_window(window.into(), &cx);
+    visual.update(|window, cx| window.draw(cx).clear(cx));
+
+    // Typing an unmatched name and blurring hands it to `on_create`, which
+    // updates this very combobox — the crash the deferral fixes.
+    visual.update(|window, cx| {
+        state.update(cx, |state, cx| state.focus(window, cx));
+    });
+    for letter in "feature".chars() {
+        visual.simulate_keystrokes(&letter.to_string());
+    }
+    visual.update(|window, cx| window.blur(cx));
+    visual.run_until_parked();
+    visual.update(|window, cx| window.draw(cx).clear(cx));
+
+    let selected = cx.update(|cx| state.read(cx).selected().cloned());
+    assert_eq!(selected, Some(SharedString::from("feature")));
+    let options = cx.update(|cx| state.read(cx).options_snapshot());
+    assert!(options.iter().any(|(value, _)| value == "feature"));
+}
+
+/// Matches when the query's characters appear in the label in order, so
+/// "mn" finds "main" though the substring matcher would find nothing.
+fn subsequence(query: &str, _value: &SharedString, label: &SharedString) -> bool {
+    let label = label.to_lowercase();
+    let mut letters = label.chars();
+    query
+        .to_lowercase()
+        .chars()
+        .all(|letter| letters.any(|found| found == letter))
+}
+
+#[test]
+fn a_custom_filter_decides_which_options_survive() {
+    let mut host = combo_host_with(|builder, _| builder.filter(subsequence));
+    // "mn" is no substring of any option, so only the custom matcher finds
+    // "main" and "maintain"; "release" has no "n" and drops out.
+    type_text(&mut host, "mn");
+
+    assert!(host.visual.debug_bounds("test-combo-option-0").is_some());
+    assert!(host.visual.debug_bounds("test-combo-option-1").is_some());
+    assert!(host.visual.debug_bounds("test-combo-option-2").is_none());
+}
+
+#[test]
+fn replacing_the_options_reruns_the_custom_filter() {
+    let mut host = combo_host_with(|builder, _| builder.filter(subsequence));
+    type_text(&mut host, "mn");
+
+    host.cx.update(|cx| {
+        host.state.update(cx, |state, cx| {
+            state.set_options(vec![option("metro"), option("mainline")], cx)
+        })
+    });
+    draw(&mut host.visual);
+
+    // "metro" has no "n" after its "m"; "mainline" survives as the only row.
+    assert!(host.visual.debug_bounds("test-combo-option-0").is_some());
+    assert!(host.visual.debug_bounds("test-combo-option-1").is_none());
+}