chore: clean up

2026-11-04 08:57:07 +07:00
parent ded23ab2f2
commit fa73124ee4
5 changed files with 161 additions and 19 deletions
@@ -66,7 +66,10 @@ GPUI is a UI framework which also provides primitives for state and concurrency
 - Keep `render` declarative; use `.when(...)` / `.when_some(...)` for small conditional refinements.
 - Controlled components: the owner passes the current value, callbacks report intent, the owner updates state and calls `cx.notify()`. No second source of truth.
 - Call `cx.notify()` after a mutation that changes rendering; notify the narrowest owning entity. Never notify from `render`.
- Use `cx.emit(...)` for semantic events (declare `impl EventEmitter<EventType>`), `cx.subscribe(...)` / `cx.observe(...)` for reactions, and store returned `Subscription`s in the owner (commonly `_subscriptions: Vec<Subscription>`).
+- `render` is side-effect free: no `cx.notify()`, no `entity.update(...)`, no writing derived text or state back into other entities during render. A write there forces another frame, and a write that emits an event re-enters the event handler — an unbounded redraw loop. Reconcile state in event handlers instead, with silent setters (`set_value_silent`-style) so a control's own write does not re-enter the events it mirrors.
+- Use `cx.emit(...)` for semantic events (declare `impl EventEmitter<EventType>`), `cx.subscribe(...)` / `cx.observe(...)` for reactions, and store returned `Subscription`s in the owner (commonly `_subscriptions: Vec<Subscription>`). Never `.detach()` a component's own subscription — detached subscriptions outlive the component and fire into dropped state; detach is only for app-lifetime wiring registered once at startup.
+- To take a key a focused child has already bound, add bindings in a deeper context (`"Parent > Child"` predicate) registered after the child's — gpui breaks equal-depth ties by later registration. Do not intercept keystrokes globally for component-scoped keys; an interceptor runs for every keystroke in every window.
+- Animations schedule frames that re-render the whole window. Continuous animations (`repeat()`) must cap their frame rate (`with_max_fps`); skip entry animations on surfaces that redraw with typing (popups over text fields), where each frame is a full window redraw.
 - Use stable domain-derived `ElementId`s (`("delete-project", project.id)`); never list indexes or freshly generated random IDs for reorderable content.
 - `cx.listener(...)` adapts event handlers to update the current entity.
 - Actions: `actions!(namespace, [Action])` for data-free actions, the `Action` derive otherwise; register `on_action` on the focused region with a matching `key_context`.
@@ -191,12 +191,12 @@ impl RenderOnce for Alert {
                        .flex_none()
                        .cursor_pointer()
                        .p_0p5()
-                        .rounded(px(4.))
+                        .rounded(theme.radius.sm)
                        .hover(|style| style.bg(muted))
                        .child(
                            icon(IconRole::Close)
                                .size(theme.sizes.small.dense_icon_size)
-                                .color(foreground.alpha(0.55)),
+                                .color(theme.colors.muted_foreground),
                        )
                        .on_click(move |_, _window, cx| on_dismiss(cx)),
                )
@@ -1,6 +1,4 @@
-//! Toasts: succinct, temporary messages at the bottom center of the window
-//! — Adwaita Toast, the macOS banner. One toast at a time: content, an
-//! optional action, and a close button; fades in, then fades out.
+//! Toasts: succinct, temporary messages at the bottom center of the window.

 use std::rc::Rc;
 use std::time::Duration;
@@ -333,12 +331,12 @@ fn pill(entry: &ToastEntry, toasts: &Entity<Toasts>, cx: &App) -> gpui::Stateful
                .ml_4()
                .cursor_pointer()
                .p_0p5()
-                .rounded(px(4.))
+                .rounded(theme.radius.sm)
                .hover(|style| style.bg(muted))
                .child(
                    icon(IconRole::Close)
                        .size(dense_icon_size)
-                        .color(foreground.alpha(0.55)),
+                        .color(theme.colors.muted_foreground),
                )
                .on_click(move |_, _window, cx| {
                    toasts.update(cx, |toasts, cx| {
@@ -0,0 +1,113 @@
+use std::cell::RefCell;
+use std::rc::Rc;
+use std::time::Instant;
+
+use gpui::{
+    AppContext as _, Context, Entity, IntoElement, ParentElement as _, Render, SharedString,
+    Styled as _, TestAppContext, VisualTestContext, Window, div,
+};
+use gpui_native_core::common::v_flex;
+use gpui_native_core::field::Field;
+use gpui_native_core::init;
+use gpui_native_core::input::{Input, InputState};
+
+struct FieldHost {
+    url: Entity<InputState>,
+    renders: Rc<RefCell<usize>>,
+}
+
+impl Render for FieldHost {
+    fn render(&mut self, _window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
+        *self.renders.borrow_mut() += 1;
+        let url_error = self
+            .url
+            .read(cx)
+            .value()
+            .is_empty()
+            .then_some("Required")
+            .map(SharedString::from);
+
+        div().size_full().p_4().child(
+            v_flex().gap_3().child(
+                Field::new("bench-url", "Server URL")
+                    .description("Where your patches are sent.")
+                    .error(url_error)
+                    .child(Input::new("bench-url-input", self.url.clone())),
+            ),
+        )
+    }
+}
+
+/// A field page in a real window, with the url input focused.
+fn field_host() -> (
+    TestAppContext,
+    Entity<InputState>,
+    Rc<RefCell<usize>>,
+    VisualTestContext,
+) {
+    let cx = TestAppContext::single();
+    cx.update(init);
+    let (url, renders, window) = cx.update(|cx| {
+        let renders = Rc::new(RefCell::new(0));
+        let url = InputState::new(cx);
+        let url_for_host = url.clone();
+        let renders_for_host = renders.clone();
+        let window = cx
+            .open_window(gpui::WindowOptions::default(), |_, cx| {
+                cx.new(|_| FieldHost {
+                    url: url_for_host,
+                    renders: renders_for_host,
+                })
+            })
+            .unwrap();
+        (url, renders, window)
+    });
+    let mut visual = VisualTestContext::from_window(window.into(), &cx);
+    visual.update(|window, cx| {
+        let focus = url.read(cx).focus_handle();
+        window.focus(&focus, cx);
+        window.draw(cx).clear(cx);
+    });
+    (cx, url, renders, visual)
+}
+
+fn draw(visual: &mut VisualTestContext) {
+    visual.update(|window, cx| {
+        window.draw(cx).clear(cx);
+    });
+}
+
+fn type_and_time(visual: &mut VisualTestContext, text: &str) -> std::time::Duration {
+    let started = Instant::now();
+    for letter in text.chars() {
+        visual.simulate_keystrokes(&letter.to_string());
+        draw(visual);
+    }
+    let elapsed = started.elapsed();
+    println!(
+        "{} keystrokes: {elapsed:?} ({:.2} ms per keystroke)",
+        text.chars().count(),
+        elapsed.as_secs_f64() / text.chars().count() as f64 * 1000.
+    );
+    elapsed
+}
+
+#[test]
+fn an_idle_field_page_schedules_no_frames() {
+    let (_cx, _url, _renders, mut visual) = field_host();
+    let frames = visual.update(|window, cx| window.simulate_next_frame(cx));
+    assert_eq!(
+        frames, 0,
+        "an idle field page must not request animation frames"
+    );
+}
+
+#[test]
+fn field_typing_frame_cost() {
+    let (_cx, _url, _renders, mut visual) = field_host();
+    let elapsed = type_and_time(&mut visual, "dsdsadsadadadada");
+    assert!(
+        elapsed.as_millis() < 1000,
+        "typing 16 characters took {elapsed:?}"
+    );
+}
@@ -17,7 +17,7 @@ use gpui_native_core::context_menu::ContextMenu;
 use gpui_native_core::dialog::Dialog;
 use gpui_native_core::empty_state::EmptyState;
 use gpui_native_core::field::{Field, FieldVariant};
-use gpui_native_core::input::{Input, InputState, InputStateEvent};
+use gpui_native_core::input::{Input, InputState};
 use gpui_native_core::link::Link;
 use gpui_native_core::menu::{Menu, MenuItem};
 use gpui_native_core::popover::Popover;
@@ -389,7 +389,9 @@ impl Gallery {
            cx,
        );
        dock.register_panel(
-            PanelDescriptor::new("field", "Field", |_, cx| cx.new(FieldDemo::new).into()),
+            PanelDescriptor::new("field", "Field", |window, cx| {
+                cx.new(|cx| FieldDemo::new(window, cx)).into()
+            }),
            cx,
        );
        dock.register_panel(
@@ -822,6 +824,7 @@ struct InputDemo {
    text: Entity<InputState>,
    password: Entity<InputState>,
    notes: Entity<InputState>,
+    _subscriptions: Vec<gpui::Subscription>,
 }

 impl InputDemo {
@@ -831,10 +834,18 @@ impl InputDemo {
        password.update(cx, |state, cx| state.set_password(true, cx));
        let notes = InputState::new(cx);
        notes.update(cx, |state, cx| state.set_multiline(true, cx));
+        // The panel is a cached view in the dock: the value readouts re-render
+        // only when the inputs it reads are notified.
+        let subscriptions = vec![
+            cx.observe(&text, |_, _, cx| cx.notify()),
+            cx.observe(&password, |_, _, cx| cx.notify()),
+            cx.observe(&notes, |_, _, cx| cx.notify()),
+        ];
        Self {
            text,
            password,
            notes,
+            _subscriptions: subscriptions,
        }
    }
 }
@@ -871,24 +882,25 @@ struct FieldDemo {
    name: Entity<InputState>,
    notifications: bool,
    sync_on_open: bool,
-    _subscription: gpui::Subscription,
+    _subscriptions: (gpui::Subscription, gpui::Subscription),
 }

 impl FieldDemo {
-    fn new(cx: &mut Context<Self>) -> Self {
+    fn new(_window: &mut Window, cx: &mut Context<Self>) -> Self {
        let url = InputState::new(cx);
        let name = InputState::new_with_text("reya", cx);
-        let subscription = cx.subscribe(&url, |_, _, event, cx| {
-            if matches!(event, InputStateEvent::TextChanged) {
-                cx.notify();
-            }
-        });
+        // The panel is a cached view in the dock: it re-renders only when the
+        // entities it reads are notified. Both inputs are read at render time
+        // (validation, error text), so every change must bust the cache.
+        let url_observation = cx.observe(&url, |_, _, cx| cx.notify());
+        let name_observation = cx.observe(&name, |_, _, cx| cx.notify());
+
        Self {
            url,
            name,
            notifications: false,
            sync_on_open: true,
-            _subscription: subscription,
+            _subscriptions: (url_observation, name_observation),
        }
    }
 }
@@ -1036,6 +1048,7 @@ struct ComboboxDemo {
    branch: Entity<ComboboxState<SharedString>>,
    label: Entity<ComboboxState<SharedString>>,
    label_created: Option<SharedString>,
+    _subscriptions: Vec<gpui::Subscription>,
 }

 impl ComboboxDemo {
@@ -1087,10 +1100,17 @@ impl ComboboxDemo {
            });
        });
        let label = cx.new(|cx| ComboboxState::new(label_builder, window, cx));
+        // The panel is a cached view in the dock: the value readouts re-render
+        // only when the picker states it reads are notified.
+        let subscriptions = vec![
+            cx.observe(&branch, |_, _, cx| cx.notify()),
+            cx.observe(&label, |_, _, cx| cx.notify()),
+        ];
        Self {
            branch,
            label,
            label_created: None,
+            _subscriptions: subscriptions,
        }
    }
 }
@@ -2835,7 +2855,15 @@ fn main() {
                            gallery.downgrade(),
                            cx,
                        );
-                        dock.open_panel("welcome", window, cx);
+                        // TEMPORARY perf probe: open the field panel under the
+                        // autoplay env var so the probe types into a real window.
+                        let first = if std::env::var("GPUI_FIELD_AUTOPLAY").is_ok() {
+                            eprintln!("[field-probe] autoplay: opening field panel");
+                            "field"
+                        } else {
+                            "welcome"
+                        };
+                        dock.open_panel(first, window, cx);
                    });
                    gallery
                },