From df23067c01931ebe3c06d46fb89fbf4e70bbd647 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Thu, 17 Sep 2026 16:39:46 +0700 Subject: [PATCH] migrate overlays and feedback --- crates/ui/src/lib.rs | 2 - crates/ui/src/modal.rs | 314 +++++++++++++++------------------- crates/ui/src/notification.rs | 273 ++++++++++++++++++----------- crates/ui/src/popover.rs | 262 ++++------------------------ crates/ui/src/root.rs | 3 +- crates/ui/src/tooltip.rs | 7 +- docs/gpui-base-migration.md | 110 ++++++++++-- 7 files changed, 445 insertions(+), 526 deletions(-) diff --git a/crates/ui/src/lib.rs b/crates/ui/src/lib.rs index 70515f82..fa211c81 100644 --- a/crates/ui/src/lib.rs +++ b/crates/ui/src/lib.rs @@ -42,7 +42,5 @@ mod window_ext; pub fn init(cx: &mut gpui::App) { gpui_base::init(cx); theme::sync_base(cx); - modal::init(cx); - popover::init(cx); menu::init(cx); } diff --git a/crates/ui/src/modal.rs b/crates/ui/src/modal.rs index a16d4b13..e7715416 100644 --- a/crates/ui/src/modal.rs +++ b/crates/ui/src/modal.rs @@ -2,28 +2,19 @@ use std::rc::Rc; use gpui::prelude::FluentBuilder; use gpui::{ - Animation, AnimationExt as _, AnyElement, App, Bounds, BoxShadow, ClickEvent, Div, FocusHandle, - InteractiveElement, IntoElement, KeyBinding, MouseButton, ParentElement, Pixels, Point, - RenderOnce, SharedString, StyleRefinement, Styled, Window, anchored, div, hsla, point, px, + Animation, AnimationExt as _, AnyElement, App, BoxShadow, ClickEvent, Div, FocusHandle, + InteractiveElement as _, IntoElement, ParentElement, Pixels, RenderOnce, SharedString, + StyleRefinement, Styled, Window, div, hsla, point, px, size, }; +use gpui_base::Dialog; use instant::Duration; use theme::ActiveTheme; -use crate::actions::{Cancel, Confirm}; use crate::animation::cubic_bezier; use crate::button::{Button, ButtonCustomVariant, ButtonVariant, ButtonVariants as _}; use crate::scroll::ScrollableElement; use crate::{IconName, Root, Sizable, StyledExt, WindowExtension, h_flex, v_flex}; -const CONTEXT: &str = "Modal"; - -pub fn init(cx: &mut App) { - cx.bind_keys([ - KeyBinding::new("escape", Cancel, Some(CONTEXT)), - KeyBinding::new("enter", Confirm { secondary: false }, Some(CONTEXT)), - ]); -} - type OnClose = Rc; type OnOk = Option bool + 'static>>; type OnCancel = Rc bool + 'static>; @@ -275,6 +266,8 @@ impl Styled for Modal { impl RenderOnce for Modal { fn render(self, window: &mut Window, cx: &mut App) -> impl gpui::IntoElement { let layer_ix = self.layer_ix; + let is_topmost = layer_ix + 1 == Root::read(window, cx).active_modals.len(); + let has_footer = self.footer.is_some(); let on_close = self.on_close.clone(); let on_ok = self.on_ok.clone(); let on_cancel = self.on_cancel.clone(); @@ -345,19 +338,16 @@ impl RenderOnce for Modal { let radius = cx.theme().radius_lg; let view_size = window.viewport_size() - - gpui::size( + - size( window_paddings.left + window_paddings.right, window_paddings.top + window_paddings.bottom, ); - let bounds = Bounds { - origin: Point::default(), - size: view_size, - }; - let offset_top = px(layer_ix as f32 * 16.); let y = self.margin_top.unwrap_or(view_size.height / 10.) + offset_top; - let x = bounds.center().x - self.width / 2.; + let x = view_size.width / 2. - self.width / 2.; + let card_top = window_paddings.top + y; + let card_left = window_paddings.left + x; let mut padding_right = px(16.); let mut padding_left = px(16.); @@ -373,168 +363,138 @@ impl RenderOnce for Modal { let animation = Animation::new(Duration::from_secs_f64(0.25)) .with_easing(cubic_bezier(0.32, 0.72, 0., 1.)); - anchored() - .position(point(window_paddings.left, window_paddings.top)) - .snap_to_window() + let backdrop = div() + .absolute() + .top(window_paddings.top) + .left(window_paddings.left) + .w(view_size.width) + .h(view_size.height) + .when(self.overlay_visible, |this| { + this.occlude().bg(cx.theme().overlay) + }) + .with_animation("fade-in", animation.clone(), move |this, delta| { + this.opacity(delta) + }); + + let card = v_flex() + .id(layer_ix) + .bg(cx.theme().background) + .border_1() + .border_color(cx.theme().border.alpha(0.4)) + .rounded(radius) + .when(cx.theme().shadow, |this| this.shadow_xl()) + .min_h_24() + .refine_style(&self.style) + // There style is high priority, can't be overridden. + .absolute() + .occlude() + .relative() + .left(card_left) + .top(card_top) + .w(self.width) + .when_some(self.max_width, |this, w| this.max_w(w)) .child( div() - .id("modal") - .w(view_size.width) - .h(view_size.height) - .when(self.overlay_visible, |this| { - this.occlude().bg(cx.theme().overlay) - }) - .when(self.overlay_closable, |this| { - // Only the last modal owns the `mouse down - close modal` event. - if (self.layer_ix + 1) != Root::read(window, cx).active_modals.len() { - return this; - } + .px_4() + .h_8() + .w_full() + .flex() + .items_center() + .justify_center() + .when_some(self.title, |this, title| { + this.h_10().font_semibold().text_center().child(title) + }), + ) + .when(self.show_close, |this| { + let on_cancel = on_cancel.clone(); + let on_close = on_close.clone(); - this.on_mouse_down(MouseButton::Left, { - let on_cancel = on_cancel.clone(); - let on_close = on_close.clone(); - move |_, window, cx| { - on_cancel(&ClickEvent::default(), window, cx); - on_close(&ClickEvent::default(), window, cx); - window.close_modal(cx); - } - }) - }) + this.child( + Button::new("close") + .icon(IconName::CloseCircleFill) + .absolute() + .top_1p5() + .right_2() + .custom( + ButtonCustomVariant::new(window, cx) + .foreground(cx.theme().icon_muted) + .color(cx.theme().ghost_element_background) + .hover(cx.theme().ghost_element_background) + .active(cx.theme().ghost_element_background), + ) + .on_click(move |_, window, cx| { + on_cancel(&ClickEvent::default(), window, cx); + on_close(&ClickEvent::default(), window, cx); + window.close_modal(cx); + }), + ) + }) + .child( + div() + .pt_px() + .w_full() + .h_auto() + .flex_1() + .overflow_hidden() .child( v_flex() - .id(layer_ix) - .bg(cx.theme().background) - .border_1() - .border_color(cx.theme().border.alpha(0.4)) - .rounded(radius) - .when(cx.theme().shadow, |this| this.shadow_xl()) - .min_h_24() - .key_context(CONTEXT) - .track_focus(&self.focus_handle) - .refine_style(&self.style) - .when(self.keyboard, |this| { - this.on_action({ - let on_cancel = on_cancel.clone(); - let on_close = on_close.clone(); - move |_: &Cancel, window, cx| { - // FIXME: - // - // Here some Modal have no focus_handle, so it will not work will Escape key. - // But by now, we `cx.close_modal()` going to close the last active model, so the Escape is unexpected to work. - on_cancel(&ClickEvent::default(), window, cx); - on_close(&ClickEvent::default(), window, cx); - window.close_modal(cx); - } - }) - .on_action({ - let on_ok = on_ok.clone(); - let on_close = on_close.clone(); - let has_footer = self.footer.is_some(); - move |_: &Confirm, window, cx| { - if let Some(on_ok) = &on_ok { - if on_ok(&ClickEvent::default(), window, cx) { - on_close(&ClickEvent::default(), window, cx); - window.close_modal(cx); - } - } else if has_footer { - window.close_modal(cx); - } - } - }) - }) - // There style is high priority, can't be overridden. - .absolute() - .occlude() - .relative() - .left(x) - .top(y) - .w(self.width) - .when_some(self.max_width, |this, w| this.max_w(w)) - .child( - div() - .px_4() - .h_8() - .w_full() - .flex() - .items_center() - .justify_center() - .when_some(self.title, |this, title| { - this.h_10().font_semibold().text_center().child(title) - }), - ) - .when(self.show_close, |this| { - this.child( - Button::new("close") - .icon(IconName::CloseCircleFill) - .absolute() - .top_1p5() - .right_2() - .custom( - ButtonCustomVariant::new(window, cx) - .foreground(cx.theme().icon_muted) - .color(cx.theme().ghost_element_background) - .hover(cx.theme().ghost_element_background) - .active(cx.theme().ghost_element_background), - ) - .on_click(move |_, window, cx| { - on_cancel(&ClickEvent::default(), window, cx); - on_close(&ClickEvent::default(), window, cx); - window.close_modal(cx); - }), - ) - }) - .child( - div() - .pt_px() - .w_full() - .h_auto() - .flex_1() - .overflow_hidden() - .child( - v_flex() - .pr(padding_right) - .pl(padding_left) - .size_full() - .overflow_y_scrollbar() - .child(self.content), - ), - ) - .when_none(&self.footer, |this| this.child(div().pt(padding_left))) - .when_some(self.footer, |this, footer| { - this.child( - h_flex() - .gap_2() - .pt(padding_left) - .pr(padding_right) - .pb(padding_left) - .pl(padding_right) - .justify_end() - .children(footer(render_ok, render_cancel, window, cx)), - ) - }) - .with_animation("slide-down", animation.clone(), move |this, delta| { - let y_offset = px(0.) + delta * px(30.); - // This is equivalent to `shadow_xl` with an extra opacity. - let shadow = vec![ - BoxShadow { - color: hsla(0., 0., 0., 0.1 * delta), - offset: point(px(0.), px(20.)), - blur_radius: px(25.), - spread_radius: px(-5.), - inset: false, - }, - BoxShadow { - color: hsla(0., 0., 0., 0.1 * delta), - offset: point(px(0.), px(8.)), - blur_radius: px(10.), - spread_radius: px(-6.), - inset: false, - }, - ]; - this.top(y + y_offset).shadow(shadow) - }), - ) - .with_animation("fade-in", animation, move |this, delta| this.opacity(delta)), + .pr(padding_right) + .pl(padding_left) + .size_full() + .overflow_y_scrollbar() + .child(self.content), + ), ) + .when_none(&self.footer, |this| this.child(div().pt(padding_left))) + .when_some(self.footer, |this, footer| { + this.child( + h_flex() + .gap_2() + .pt(padding_left) + .pr(padding_right) + .pb(padding_left) + .pl(padding_right) + .justify_end() + .children(footer(render_ok, render_cancel, window, cx)), + ) + }) + .with_animation("slide-down", animation, move |this, delta| { + let y_offset = px(0.) + delta * px(30.); + // This is equivalent to `shadow_xl` with an extra opacity. + let shadow = vec![ + BoxShadow { + color: hsla(0., 0., 0., 0.1 * delta), + offset: point(px(0.), px(20.)), + blur_radius: px(25.), + spread_radius: px(-5.), + inset: false, + }, + BoxShadow { + color: hsla(0., 0., 0., 0.1 * delta), + offset: point(px(0.), px(8.)), + blur_radius: px(10.), + spread_radius: px(-6.), + inset: false, + }, + ]; + this.top(card_top + y_offset).shadow(shadow) + }); + + Dialog::new(cx) + .layer(layer_ix, is_topmost) + .focus_handle(self.focus_handle.clone()) + .close_on_escape(self.keyboard) + .close_on_backdrop_press(self.overlay_closable) + .on_ok(move |event, window, cx| match &on_ok { + Some(on_ok) => on_ok(event, window, cx), + None => has_footer, + }) + .on_cancel(move |event, window, cx| on_cancel(event, window, cx)) + .on_close(move |event, window, cx| { + on_close(event, window, cx); + window.close_modal(cx); + }) + .backdrop(backdrop) + .popup(card) } } diff --git a/crates/ui/src/notification.rs b/crates/ui/src/notification.rs index b297d4e5..026e0135 100644 --- a/crates/ui/src/notification.rs +++ b/crates/ui/src/notification.rs @@ -1,7 +1,7 @@ use std::any::TypeId; -use std::collections::{HashMap, VecDeque}; +use std::collections::HashMap; use std::rc::Rc; -use instant::Duration; +use std::time::Duration; use gpui::prelude::FluentBuilder; use gpui::{ @@ -10,12 +10,25 @@ use gpui::{ ParentElement as _, Render, SharedString, StatefulInteractiveElement, StyleRefinement, Styled, Subscription, Window, div, px, relative, }; +use gpui_base::{ + Toast as BaseToast, ToastManager, ToastMotion, ToastOptions, ToastStack, ToastStackState, + ToastTransitionStatus, +}; use theme::ActiveTheme; use crate::animation::cubic_bezier; use crate::button::{Button, ButtonVariants as _}; use crate::{Icon, IconName, Sizable as _, Size, StyledExt, h_flex, v_flex}; +/// How often the notification lifecycle clock is sampled. +const ADVANCE_INTERVAL: Duration = Duration::from_millis(50); + +/// How long a notification stays before it hides itself. +const AUTOHIDE_DURATION: Duration = Duration::from_secs(5); + +/// Request by a notification to be dismissed; the list owns the transition. +struct DismissRequest; + #[derive(Debug, Clone, Copy, Default)] pub enum NotificationKind { #[default] @@ -79,7 +92,7 @@ pub struct Notification { action_builder: Option) -> Button>>, content_builder: Option) -> AnyElement>>, on_click: Option>, - closing: bool, + transition_status: ToastTransitionStatus, } impl From for Notification { @@ -133,7 +146,7 @@ impl Notification { action_builder: None, content_builder: None, on_click: None, - closing: false, + transition_status: ToastTransitionStatus::Starting, } } @@ -238,29 +251,29 @@ impl Notification { } /// Dismiss the notification. - pub fn dismiss(&mut self, _: &mut Window, cx: &mut Context) { - if self.closing { - return; + pub fn dismiss(&mut self, _window: &mut Window, cx: &mut Context) { + cx.emit(DismissRequest); + } + + /// Begin the exit transition, driven by the notification list. + pub(crate) fn begin_close(&mut self, cx: &mut Context) { + if self.transition_status != ToastTransitionStatus::Ending { + self.transition_status = ToastTransitionStatus::Ending; + cx.notify(); } - self.closing = true; - cx.notify(); + } - // Dismiss the notification after 0.15s to show the animation. - cx.spawn(async move |view, cx| { - cx.background_executor() - .timer(Duration::from_secs_f32(0.15)) - .await; + /// Mark the enter transition as finished, driven by the notification list. + pub(crate) fn complete_enter(&mut self, cx: &mut Context) { + if self.transition_status == ToastTransitionStatus::Starting { + self.transition_status = ToastTransitionStatus::Present; + cx.notify(); + } + } - cx.update(|cx| { - if let Some(view) = view.upgrade() { - view.update(cx, |view, cx| { - view.closing = false; - cx.emit(DismissEvent); - }); - } - }) - }) - .detach(); + /// Finish the exit transition, driven by the notification list. + pub(crate) fn complete_close(&mut self, cx: &mut Context) { + cx.emit(DismissEvent); } /// Set the content of the notification. @@ -280,6 +293,7 @@ impl Default for Notification { } impl EventEmitter for Notification {} +impl EventEmitter for Notification {} impl FluentBuilder for Notification {} @@ -319,17 +333,19 @@ impl Render for Notification { _ => cx.theme().text, }; - let closing = self.closing; + let transition_status = self.transition_status; + let closing = transition_status == ToastTransitionStatus::Ending; let has_title = self.title.is_some(); let only_message = !has_title && content.is_none() && action.is_none(); let placement = cx.theme().notification.placement; - h_flex() - .id("notification") + BaseToast::new("notification") + .transition_status(transition_status) + .h_flex() .group("") .occlude() .relative() - .w_112() + .w_full() .border_1() .border_color(cx.theme().border) .bg(background) @@ -455,10 +471,13 @@ impl Render for Notification { /// A list of notifications. pub struct NotificationList { /// Notifications that will be auto hidden. - pub(crate) notifications: VecDeque>, + pub(crate) notifications: ToastManager>, - /// Whether the notification list is expanded. - expanded: bool, + /// Measured geometry and interaction state of the visible stack. + stack_state: ToastStackState, + + /// Whether the lifecycle clock is running. The loop clears it as it exits. + is_advancing: bool, /// Subscriptions _subscriptions: HashMap, @@ -467,12 +486,64 @@ pub struct NotificationList { impl NotificationList { pub fn new(_window: &mut Window, _cx: &mut Context) -> Self { Self { - notifications: VecDeque::new(), - expanded: false, + notifications: ToastManager::new(ToastMotion::default()), + stack_state: ToastStackState::default(), + is_advancing: false, _subscriptions: HashMap::new(), } } + /// Tick the toast lifecycle until the last notification is unmounted. + /// + /// The stack expansion is sampled here because it reaches the list through + /// no event, and an idle window should arm no timer. + fn start_advancing(&mut self, window: &mut Window, cx: &mut Context) { + if self.is_advancing { + return; + } + self.is_advancing = true; + cx.spawn_in(window, async move |view, cx| { + loop { + cx.background_executor().timer(ADVANCE_INTERVAL).await; + let running = view.update(cx, |view, cx| { + view.advance(cx); + view.is_advancing = !view.notifications.is_empty(); + view.is_advancing + }); + if !matches!(running, Ok(true)) { + break; + } + } + }) + .detach(); + } + + fn advance(&mut self, cx: &mut Context) { + let changes = self.notifications.advance( + cx.background_executor().now(), + self.stack_state.is_expanded(), + ); + + for id in changes.presented { + if let Some(note) = self.notifications.get(&id) { + note.update(cx, |note, cx| note.complete_enter(cx)); + } + } + for id in changes.ending { + if let Some(note) = self.notifications.get(&id) { + note.update(cx, |note, cx| note.begin_close(cx)); + } + } + for (id, note) in changes.removed { + self._subscriptions.remove(&id); + note.update(cx, |note, cx| note.complete_close(cx)); + } + + if changes.changed { + cx.notify(); + } + } + pub fn push( &mut self, notification: impl Into, @@ -483,102 +554,110 @@ impl NotificationList { let id = notification.id.clone(); let autohide = notification.autohide; - // Remove the notification by id, for keep unique. - self.notifications.retain(|note| note.read(cx).id != id); - let notification = cx.new(|_| notification); + let dismiss_id = id.clone(); self._subscriptions.insert( id.clone(), - cx.subscribe(¬ification, move |view, _, _: &DismissEvent, cx| { - view.notifications.retain(|note| id != note.read(cx).id); - view._subscriptions.remove(&id); + cx.subscribe(¬ification, move |view, _, _: &DismissRequest, cx| { + if view + .notifications + .dismiss(&dismiss_id, cx.background_executor().now()) + && let Some(note) = view.notifications.get(&dismiss_id) + { + note.update(cx, |note, cx| note.begin_close(cx)); + } }), ); - self.notifications.push_back(notification.clone()); - - if autohide { - // Sleep for 5 seconds to autohide the notification - cx.spawn_in(window, async move |_this, cx| { - cx.background_executor().timer(Duration::from_secs(5)).await; - - if let Err(err) = - notification.update_in(cx, |note, window, cx| note.dismiss(window, cx)) - { - log::error!("failed to auto hide notification: {:?}", err); - } - }) - .detach(); - } + self.notifications.push( + id, + notification, + ToastOptions { + timeout: autohide.then_some(AUTOHIDE_DURATION), + }, + cx.background_executor().now(), + ); + self.start_advancing(window, cx); cx.notify(); } pub(crate) fn close( &mut self, id: impl Into, - window: &mut Window, + _window: &mut Window, cx: &mut Context, ) { let id: NotificationId = id.into(); - if let Some(n) = self.notifications.iter().find(|n| n.read(cx).id == id) { - n.update(cx, |note, cx| note.dismiss(window, cx)) + if self + .notifications + .dismiss(&id, cx.background_executor().now()) + && let Some(note) = self.notifications.get(&id) + { + note.update(cx, |note, cx| note.begin_close(cx)); } cx.notify(); } - pub fn clear(&mut self, _: &mut Window, cx: &mut Context) { - self.notifications.clear(); + pub fn clear(&mut self, _window: &mut Window, cx: &mut Context) { + for id in self + .notifications + .dismiss_all(cx.background_executor().now()) + { + if let Some(note) = self.notifications.get(&id) { + note.update(cx, |note, cx| note.begin_close(cx)); + } + } cx.notify(); } pub fn notifications(&self) -> Vec> { - self.notifications.iter().cloned().collect() + self.notifications + .iter() + .map(|(_, note, _)| note.clone()) + .collect() } } impl Render for NotificationList { - fn render( - &mut self, - window: &mut gpui::Window, - cx: &mut gpui::Context, - ) -> impl IntoElement { + fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { let size = window.viewport_size(); - let items = self.notifications.iter().rev().take(10).rev().cloned(); + let settings = &cx.theme().notification; + let (placement, margins, max_items) = ( + settings.placement, + settings.margins.clone(), + settings.max_items, + ); - let placement = cx.theme().notification.placement; - let margins = &cx.theme().notification.margins; + let items = self + .notifications + .visible(max_items) + .map(|(id, note, _)| (id.clone(), note.clone())) + .collect::>(); - v_flex() - .id("notification-list") + let stack = items + .into_iter() + .fold( + ToastStack::new("notification-list", self.stack_state.clone()), + |stack, (id, note)| stack.item(format!("{id:?}"), note), + ) + .placement(placement) + .v_flex() + .w_112() .max_h(size.height) - .pt(margins.top) - .pb(margins.bottom) - .gap_3() - .when( - matches!(placement, Anchor::TopRight), - |this| this.pr(margins.right), // ignore left - ) - .when( - matches!(placement, Anchor::TopLeft), - |this| this.pl(margins.left), // ignore right - ) - .when( - matches!(placement, Anchor::BottomLeft), - |this| this.flex_col_reverse().pl(margins.left), // ignore right - ) - .when( - matches!(placement, Anchor::BottomRight), - |this| this.flex_col_reverse().pr(margins.right), // ignore left - ) - .when(matches!(placement, Anchor::BottomCenter), |this| { - this.flex_col_reverse() - }) - .on_hover(cx.listener(|view, hovered, _, cx| { - view.expanded = *hovered; - cx.notify() - })) - .children(items) + .absolute() + .map(|this| match placement { + Anchor::TopLeft => this.top(margins.top).left(margins.left), + Anchor::TopRight => this.top(margins.top).right(margins.right), + Anchor::TopCenter => this.top(margins.top).left_0().right_0().mx_auto(), + Anchor::BottomLeft => this.bottom(margins.bottom).left(margins.left), + Anchor::BottomRight => this.bottom(margins.bottom).right(margins.right), + Anchor::BottomCenter => this.bottom(margins.bottom).left_0().right_0().mx_auto(), + Anchor::LeftCenter => this.left(margins.left).top_0().bottom_0().my_auto(), + Anchor::RightCenter => this.right(margins.right).top_0().bottom_0().my_auto(), + }); + + div().size_full().child(stack) } } diff --git a/crates/ui/src/popover.rs b/crates/ui/src/popover.rs index 0893a701..c77ab263 100644 --- a/crates/ui/src/popover.rs +++ b/crates/ui/src/popover.rs @@ -2,20 +2,13 @@ use std::rc::Rc; use gpui::prelude::FluentBuilder as _; use gpui::{ - Anchor, AnyElement, App, Bounds, Context, Deferred, DismissEvent, Div, ElementId, EventEmitter, - FocusHandle, Focusable, InteractiveElement as _, IntoElement, KeyBinding, MouseButton, - ParentElement, Pixels, Point, Render, RenderOnce, Stateful, StyleRefinement, Styled, - Subscription, Window, anchored, deferred, div, px, + Anchor, AnyElement, App, Context, Div, ElementId, FocusHandle, InteractiveElement as _, + IntoElement, MouseButton, ParentElement, RenderOnce, Stateful, StyleRefinement, Styled, Window, }; +use gpui_base::Popover as BasePopover; +pub use gpui_base::PopoverState; -use crate::actions::Cancel; -use crate::{ElementExt, Selectable, StyledExt as _, v_flex}; - -const CONTEXT: &str = "Popover"; - -pub(crate) fn init(cx: &mut App) { - cx.bind_keys([KeyBinding::new("escape", Cancel, Some(CONTEXT))]) -} +use crate::{Selectable, StyledExt as _, v_flex}; /// A popover element that can be triggered by a button or any other element. #[derive(IntoElement)] @@ -173,28 +166,6 @@ impl Popover { self.tracked_focus_handle = Some(handle.clone()); self } - - pub(crate) fn resolved_corner(anchor: Anchor, trigger_bounds: Bounds) -> Point { - match anchor { - Anchor::TopLeft => trigger_bounds.origin, - Anchor::TopCenter => trigger_bounds.top_center(), - Anchor::TopRight => trigger_bounds.top_right(), - Anchor::BottomLeft => Point { - x: trigger_bounds.origin.x, - y: trigger_bounds.origin.y - trigger_bounds.size.height, - }, - Anchor::BottomCenter => Point { - x: trigger_bounds.top_center().x, - y: trigger_bounds.origin.y - trigger_bounds.size.height, - }, - Anchor::BottomRight => Point { - x: trigger_bounds.top_right().x, - y: trigger_bounds.origin.y - trigger_bounds.size.height, - }, - // Fallback for LeftCenter/RightCenter – adjust as needed. - _ => trigger_bounds.origin, - } - } } impl ParentElement for Popover { @@ -209,119 +180,7 @@ impl Styled for Popover { } } -pub struct PopoverState { - focus_handle: FocusHandle, - pub(crate) tracked_focus_handle: Option, - trigger_bounds: Bounds, - open: bool, - #[allow(clippy::type_complexity)] - on_open_change: Option>, - - _dismiss_subscription: Option, -} - -impl PopoverState { - pub fn new(default_open: bool, cx: &mut App) -> Self { - Self { - focus_handle: cx.focus_handle(), - tracked_focus_handle: None, - trigger_bounds: Bounds::default(), - open: default_open, - on_open_change: None, - _dismiss_subscription: None, - } - } - - /// Check if the popover is open. - pub fn is_open(&self) -> bool { - self.open - } - - /// Dismiss the popover if it is open. - pub fn dismiss(&mut self, window: &mut Window, cx: &mut Context) { - if self.open { - self.toggle_open(window, cx); - } - } - - /// Open the popover if it is closed. - pub fn show(&mut self, window: &mut Window, cx: &mut Context) { - if !self.open { - self.toggle_open(window, cx); - } - } - - fn toggle_open(&mut self, window: &mut Window, cx: &mut Context) { - self.open = !self.open; - if self.open { - let state = cx.entity(); - let focus_handle = if let Some(tracked_focus_handle) = self.tracked_focus_handle.clone() - { - tracked_focus_handle - } else { - self.focus_handle.clone() - }; - focus_handle.focus(window, cx); - - self._dismiss_subscription = - Some( - window.subscribe(&cx.entity(), cx, move |_, _: &DismissEvent, window, cx| { - state.update(cx, |state, cx| { - state.dismiss(window, cx); - }); - window.refresh(); - }), - ); - } else { - self._dismiss_subscription = None; - } - - if let Some(callback) = self.on_open_change.as_ref() { - callback(&self.open, window, cx); - } - cx.notify(); - } - - fn on_action_cancel(&mut self, _: &Cancel, window: &mut Window, cx: &mut Context) { - self.dismiss(window, cx); - } -} - -impl Focusable for PopoverState { - fn focus_handle(&self, _: &App) -> FocusHandle { - self.focus_handle.clone() - } -} - -impl Render for PopoverState { - fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { - div() - } -} - -impl EventEmitter for PopoverState {} - impl Popover { - pub(crate) fn render_popover( - anchor: Anchor, - trigger_bounds: Bounds, - content: E, - _: &mut Window, - _: &mut App, - ) -> Deferred - where - E: IntoElement + 'static, - { - deferred( - anchored() - .snap_to_window_with_margin(px(8.)) - .anchor(anchor) - .position(Self::resolved_corner(anchor, trigger_bounds)) - .child(div().relative().child(content)), - ) - .with_priority(1) - } - pub(crate) fn render_popover_content( anchor: Anchor, appearance: bool, @@ -342,91 +201,34 @@ impl Popover { } impl RenderOnce for Popover { - fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { - let force_open = self.open; - let default_open = self.default_open; - let tracked_focus_handle = self.tracked_focus_handle.clone(); - let state = window.use_keyed_state(self.id.clone(), cx, |_, cx| { - PopoverState::new(default_open, cx) - }); + fn render(self, _: &mut Window, _: &mut App) -> impl IntoElement { + let anchor = self.anchor; + let appearance = self.appearance; + let style = self.style; + let children = self.children; + let content = self.content; - state.update(cx, |state, _| { - if let Some(tracked_focus_handle) = tracked_focus_handle { - state.tracked_focus_handle = Some(tracked_focus_handle); - } - state.on_open_change = self.on_open_change.clone(); - if let Some(force_open) = force_open { - state.open = force_open; - } - }); - - let open = state.read(cx).open; - let focus_handle = state.read(cx).focus_handle.clone(); - let trigger_bounds = state.read(cx).trigger_bounds; - - let Some(trigger) = self.trigger else { - return div().id("empty"); - }; - - let parent_view_id = window.current_view(); - - let el = div() - .id(self.id) - .child((trigger)(open, window, cx)) - .on_mouse_down(self.mouse_button, { - let state = state.clone(); - move |_, window, cx| { - cx.stop_propagation(); - state.update(cx, |state, cx| { - // We force set open to false to toggle it correctly. - // Because if the mouse down out will toggle open first. - state.open = open; - state.toggle_open(window, cx); - }); - cx.notify(parent_view_id); - } + BasePopover::new(self.id) + .anchor(anchor) + .mouse_button(self.mouse_button) + .default_open(self.default_open) + .overlay_closable(self.overlay_closable) + .content(move |state, window, cx| { + Self::render_popover_content(anchor, appearance, window, cx) + .when_some(content, |this, content| { + this.child((content)(state, window, cx)) + }) + .children(children) + .refine_style(&style) }) - .on_prepaint({ - let state = state.clone(); - move |bounds, _, cx| { - state.update(cx, |state, _| { - state.trigger_bounds = bounds; - }) - } - }); - - if !open { - return el; - } - - let popover_content = - Self::render_popover_content(self.anchor, self.appearance, window, cx) - .track_focus(&focus_handle) - .key_context(CONTEXT) - .on_action(window.listener_for(&state, PopoverState::on_action_cancel)) - .when_some(self.content, |this, content| { - this.child(state.update(cx, |state, cx| (content)(state, window, cx))) - }) - .children(self.children) - .when(self.overlay_closable, |this| { - this.on_mouse_down_out({ - let state = state.clone(); - move |_, window, cx| { - state.update(cx, |state, cx| { - state.dismiss(window, cx); - }); - cx.notify(parent_view_id); - } - }) - }) - .refine_style(&self.style); - - el.child(Self::render_popover( - self.anchor, - trigger_bounds, - popover_content, - window, - cx, - )) + .when_some(self.trigger, |this, trigger| this.trigger_with(trigger)) + .when_some(self.open, |this, open| this.open(open)) + .when_some(self.tracked_focus_handle, |this, handle| { + this.track_focus(&handle) + }) + .when_some(self.on_open_change, |this, callback| { + this.on_open_change(move |open, window, cx| callback(open, window, cx)) + }) + .into_any_element() } } diff --git a/crates/ui/src/root.rs b/crates/ui/src/root.rs index 34f52fcb..d1e44182 100644 --- a/crates/ui/src/root.rs +++ b/crates/ui/src/root.rs @@ -93,8 +93,7 @@ impl Root { Some( div() .absolute() - .top_0() - .right_0() + .inset_0() .child(root.read(cx).notification.clone()), ) } diff --git a/crates/ui/src/tooltip.rs b/crates/ui/src/tooltip.rs index f997e5d1..e6dcc4aa 100644 --- a/crates/ui/src/tooltip.rs +++ b/crates/ui/src/tooltip.rs @@ -1,8 +1,9 @@ use gpui::prelude::FluentBuilder; use gpui::{ - div, relative, App, AppContext, Context, Entity, IntoElement, ParentElement, Render, - SharedString, Styled, Window, + App, AppContext, Context, Entity, IntoElement, ParentElement, Render, SharedString, Styled, + Window, div, relative, }; +use gpui_base::Tooltip as BaseTooltip; use theme::ActiveTheme; pub struct Tooltip { @@ -18,7 +19,7 @@ impl Tooltip { impl Render for Tooltip { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { div().child( - div() + BaseTooltip::new("tooltip") .font_family(".SystemUIFont") .m_3() .p_1p5() diff --git a/docs/gpui-base-migration.md b/docs/gpui-base-migration.md index 2673260f..5ef4c8e7 100644 --- a/docs/gpui-base-migration.md +++ b/docs/gpui-base-migration.md @@ -21,7 +21,11 @@ repository at `main`, and this workspace's `Cargo.lock` (zed at `4b47ceb`, - **Phase 2: landed.** `input/` runs on base's editing engine. 6,715 lines of engine and history are gone and 306 are written, taking `crates/ui/src/input` from 6,573 lines to 321. Six call sites changed, all named in phase 2 below. -- **Phases 3-5: not started.** +- **Phase 3: landed.** `tooltip`, `popover`, `modal`, and `notification` run on base's + overlay and feedback primitives. Those four modules are 1,434 lines where they + were 1,592, and nothing outside `crates/ui` changed. The behavioural differences + are named in phase 3 below; the largest is that base's toast stack replaces the + fork's notification list. - One pre-existing, unrelated breakage was found; see [A pre-existing wasm blocker](#a-pre-existing-wasm-blocker). @@ -109,6 +113,9 @@ dragging, both of which are projected. ## What each module becomes +LOC is the count before the work; a module whose phase has landed reads +`before → after`. + | `ui` module | LOC | Plan | `gpui-base` counterpart | | --- | --- | --- | --- | | `input/` (input, clear_button) | 6,573 | Replace; keep `ui::input::{Input, InputEvent, InputState}` as the import path. 321 lines remain, and the engine paints itself through `InputEditorStyle` | `InputState`/`TextareaState` (`InputBaseState` in two modes) plus the `InputBase` frame | @@ -116,10 +123,10 @@ dragging, both of which are projected. | `checkbox.rs` | 312 | Delete | `Checkbox` | | `scroll/` (scrollbar, scrollable, scrollable_mask) | 1,332 | Replace; keep the `ScrollableElement` and `Scrollbar` names | `Scrollbar`, `ScrollableMask` | | `resizable/` | 927 | Replace; base exports the same names (`h_resizable`, `v_resizable`, `resizable_panel`, `PANEL_MIN_SIZE`, `resize_handle`) | `Resizable` + `ResizeHandleRenderer` for the coop hairline | -| `modal.rs` | 540 | Port onto base parts; keep `Modal`, `ModalButtonProps`, and `window.open_modal` | `Dialog`, `AlertDialog` | -| `notification.rs` | 584 | Port; keep `Notification`, `NotificationKind`, and `window.push_notification` | `Toast`, `ToastManager`, `ToastStack` | -| `popover.rs` | 432 | Replace with a coop-styled wrapper | `Popover`, `Popup`, `Positioner` | -| `tooltip.rs` | 36 | Replace with a coop-styled wrapper | `Tooltip` | +| `modal.rs` | 540 → 500 | Port onto base parts; `Modal`, `ModalButtonProps`, and `window.open_modal` unchanged. `Root` still owns the stack | `Dialog` — focus trap, Escape/Enter/backdrop dispatch, layer priority, deferred host | +| `notification.rs` | 584 → 663 | Port; `Notification`, `NotificationKind`, and `window.push_notification` unchanged | `ToastManager` (storage, ids, timers, exit), `ToastStack` (geometry, motion), `Toast` (`Role::Alert`) | +| `popover.rs` | 432 → 234 | Coop's builder over base's element; `PopoverState` is base's, re-exported | `Popover`, `Popup`, `Positioner` | +| `tooltip.rs` | 36 → 37 | Coop's view rooted at base's element | `Tooltip` (`Role::Tooltip`) | | `button.rs` | 626 | Skin: base behavior plus coop's existing variant tables | `Button`, `StateStyle` | | `switch.rs` | 287 | Skin | `Switch`, `SwitchTrack`, `SwitchThumb` | | `avatar.rs` | 141 | Skin | `Avatar`, `AvatarImage`, `AvatarFallback` | @@ -310,18 +317,86 @@ Surfaces to re-verify by hand: the chat composer (auto-grow, Enter to send, IME) profile bio, the subject line, the settings dialog, the relay and messaging lists, the import/restore/backup dialogs, and the sidebar search field. -### Phase 3 — overlays and feedback +### Phase 3 — overlays and feedback — landed -`popover` becomes a wrapper over base `Popover`; `modal` composes base `Dialog` and -`AlertDialog` while keeping the `Modal` API and `window.open_modal`; `notification` -moves onto `Toast`/`ToastManager` (base owns the stack, timers, and motion; coop owns -the visual and the placement from `theme.notification`); `tooltip` becomes a wrapper -over base `Tooltip`. `Root` and `window_ext` keep their public API and host the new -layers. No call site changes. +All four modules keep their names, builders, and call sites. The four files go from +1,592 lines to 1,434, and no file outside `crates/ui` changed. + +| `ui` module | What stayed coop's | What is base's now | +| --- | --- | --- | +| `tooltip` | the whole look, `Tooltip::new(text, window, cx)` and the `Render` view | the element and `Role::Tooltip` | +| `popover` | every builder, the content styling, the anchor | open lifecycle, dismissal, focus capture and restore, deferred registration, trigger measurement and anchor math | +| `modal` | `Modal`, `ModalButtonProps`, `Root`'s stack, `window.open_modal`, the card, buttons, shadows and animations | focus trap, Escape/Enter/backdrop dispatch with a cancel veto, layer priority, the deferred host, `Role::Dialog` | +| `notification` | `Notification`, `NotificationKind`, `window.push_notification`, the card and the placement from `theme.notification` | id-replacing storage, auto-hide and exit timers, stack geometry and motion, `Role::Alert` | + +**`tooltip`.** The view and its `new` are unchanged; the styled box inside is +`gpui_base::Tooltip` instead of a bare `div`. That is what carries the role. Base's +window-level `TooltipOverlay` is deliberately not adopted — gpui's own `.tooltip()` +layer already provides the delay and the placement, and taking the overlay would mean +rewriting every `.tooltip(..)` call site onto `Popup` plus hover state. + +**`popover`.** `PopoverState` is `gpui_base::PopoverState`, re-exported so +`ui::popover::PopoverState` still resolves, and the hand-rolled `anchored`/`deferred` +layer, `resolved_corner` and `render_popover` are gone — base's `Popup` measures the +trigger, resolves the anchor and snaps to the window edge. The rest of the file is the +fork's builder, unchanged, including `trigger_style`, which the fork already stored +without ever reading. Two bindings changed hands: `popover::init` (escape → coop's +`Cancel` in the `Popover` context) is deleted, because `gpui_base::init` binds +escape/enter/space in that same context and coop's lone escape binding would have +shadowed base's `Confirm` — the one that opens a popover from its trigger. + +**`modal`.** `Modal` still assembles the card, title, close button, footer buttons, +the two shadows and the `fade-in`/`slide-down` animations; `Root` still owns the stack, +the focus restore, and the one-visible-overlay rule, now expressed as base's +`layer(index, topmost)`. What changed underneath: + +- Escape, Enter and the backdrop now run through base's `Dialog` decisions, so + `on_cancel`/`on_ok` returning `false` vetoes all three. The fork honored the veto on + the buttons and the backdrop but ignored it on Escape. +- Enter on a modal that has a footer but no `on_ok` now calls `on_close` before closing; + the fork closed silently. No caller combines the two, and `on_close` defaults to a + no-op. +- Tab is trapped inside the modal, and the dialog surface carries `Role::Dialog`. +- `modal::init` (escape/enter in the `Modal` context) is deleted; base binds them in + its own `Dialog` context, which the `Dialog` host installs when `keyboard` is on. +- The dim does not move: coop's backdrop element keeps the `window_paddings` inset and + the `view_size` that the fork used. Its hit area does move — base's host covers the + whole viewport, so a click in the client-side-decoration shadow band now dismisses + the modal instead of starting a window resize. + +`AlertDialog` turned out to be unnecessary. Coop's `alert()` and `confirm()` select a +button set, not an ARIA role, and they already opt out of backdrop dismissal, which is +the whole of what `AlertDialog` adds over `Dialog`. + +**`notification`.** `Notification` keeps its builder and its card. `closing: bool` +becomes base's `ToastTransitionStatus`, `dismiss` now emits a `DismissRequest` the list +turns into a `ToastManager::dismiss`, and the exit delay is base's 200 ms rather than +the fork's fixed 150 ms. `NotificationList` holds +`ToastManager>` plus one `ToastStackState`; its +`expanded` field and hover handler are gone, and a 50 ms lifecycle tick runs only while +something is mounted. The stack is base's: + +- It collapses to three layers with a 14 px peek and a 5% width step per layer, expands + on hover or focus, and pauses auto-hide while expanded. +- The newest notification sits nearest the window edge; the fork's list grew downwards + with the oldest first. +- Motion is `ToastMotion::default()`, base's shadcn/Sonner figures. Coop contributes the + width the fork's card had, the placement and the margins from `theme.notification`. + +That stack is the one visible change of the phase, and it is the one to judge by hand. +If it is not wanted, the smaller step is to keep the list's own `v_flex` and use only +`ToastManager` together with `Toast` — base separates the lifecycle from the geometry, +so nothing else has to come back. + +Surfaces to re-verify by hand: the settings dialog (its Escape and Enter paths), the +import, restore and screening modals (a modal with a textarea, and one with +`keyboard(false)`), the dropdown menus that ride the popover, and every +`push_notification` site — sending an empty message, a failed upload with its retry +action, and the device-approval notification that never auto-hides. ### Phase 4 — leaf controls, scroll, and resizable (one module per pull request) -Order: `tooltip`, `avatar`, `switch`, `button`, `scroll/`, `resizable/`. `button` is the +Order: `avatar`, `switch`, `button`, `scroll/`, `resizable/`. `button` is the largest skin: the `ButtonVariants` and `ButtonCustomVariant` tables, the `compact`, `loading`, and `caret` builders, and the variant names stay as they are, with styling supplied through base's semantic-state styles. `scroll/` keeps the `ScrollableElement` @@ -345,7 +420,12 @@ menu positioning and dismissal on base `Popup`/`Positioner` is optional and late There is no UI test suite to lean on, so each phase gets the same treatment: -- `cargo check` and `cargo build` at the workspace root. +- `cargo check --workspace` and `cargo build` (default members build `desktop`). + `cargo build --workspace` cannot link the web crate's host dylib: `coop_web` is + `crate-type = ["cdylib", "rlib"]` and depends on `wasm-bindgen`, `web-sys`, + `console_log` and `tracing-wasm` unconditionally, so its dylib is a wasm artifact. + That is a property of the manifest rather than of any migrated crate — + `cargo check -p coop_web` passes, and the desktop binary links the same crates. - `cargo check -p theme -p ui --target wasm32-unknown-unknown`. The web target cannot be checked end to end until the pre-existing blocker below is fixed, so the migrated crates are checked directly. @@ -402,7 +482,7 @@ compile for `wasm32-unknown-unknown`". | 1 | Phase 0: `gpui` moves to the `gpui-pre` package, `gpui_tokio` vendored | root `Cargo.toml`, `Cargo.lock`, `web/Cargo.toml`, new `crates/gpui_tokio`; `crates/state` needed no edit | landed | | 2 | Phase 1: base wiring, `sync_base`, deletions | `crates/theme` | landed | | 3 | Phase 2: input, plus `history.rs` and the `ropey`/`sum_tree`/`lsp-types`/`regex`/`unicode-segmentation`/`tree-sitter` pruning | `crates/workspace`, `crates/chat_ui` (six call sites); no manifest outside `crates/ui` | landed | -| 4 | Phase 3: popover, modal, notification, tooltip | none | not started | +| 4 | Phase 3: popover, modal, notification, tooltip | none | landed | | 5–10 | Phase 4: one leaf module each | none | not started | | later | Phase 5: dock, as its own plan | `crates/workspace`, `crates/chat_ui` | not started |