diff --git a/AGENTS.md b/AGENTS.md index 6bbfbed..71c14dd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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`), `cx.subscribe(...)` / `cx.observe(...)` for reactions, and store returned `Subscription`s in the owner (commonly `_subscriptions: Vec`). +- `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`), `cx.subscribe(...)` / `cx.observe(...)` for reactions, and store returned `Subscription`s in the owner (commonly `_subscriptions: Vec`). 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`. diff --git a/crates/core/src/alert.rs b/crates/core/src/alert.rs index 5bf85f1..8b2d19b 100644 --- a/crates/core/src/alert.rs +++ b/crates/core/src/alert.rs @@ -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)), ) diff --git a/crates/core/src/toast.rs b/crates/core/src/toast.rs index 01087fd..2020093 100644 --- a/crates/core/src/toast.rs +++ b/crates/core/src/toast.rs @@ -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, 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| { diff --git a/crates/core/tests/field_bench.rs b/crates/core/tests/field_bench.rs new file mode 100644 index 0000000..b3a99c5 --- /dev/null +++ b/crates/core/tests/field_bench.rs @@ -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, + renders: Rc>, +} + +impl Render for FieldHost { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> 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, + Rc>, + 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:?}" + ); +} diff --git a/examples/gallery/src/main.rs b/examples/gallery/src/main.rs index d75ebf0..17d2292 100644 --- a/examples/gallery/src/main.rs +++ b/examples/gallery/src/main.rs @@ -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, password: Entity, notes: Entity, + _subscriptions: Vec, } 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(¬es, |_, _, cx| cx.notify()), + ]; Self { text, password, notes, + _subscriptions: subscriptions, } } } @@ -871,24 +882,25 @@ struct FieldDemo { name: Entity, notifications: bool, sync_on_open: bool, - _subscription: gpui::Subscription, + _subscriptions: (gpui::Subscription, gpui::Subscription), } impl FieldDemo { - fn new(cx: &mut Context) -> Self { + fn new(_window: &mut Window, cx: &mut Context) -> 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>, label: Entity>, label_created: Option, + _subscriptions: Vec, } 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 },