diff --git a/AGENTS.md b/AGENTS.md index caa81c1..f41bc05 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,5 +1,6 @@ # Coding guidelines +- Prefer existing components (e.g. build a close affordance out of `Button`) over custom elements; build a custom element only when no existing component fits. - Prioritize code correctness and clarity. Speed and efficiency are secondary priorities unless otherwise specified. - Do not write organizational or comments that summarize the code. Comments should only be written to explain "why" the code is written in some way in the case there is a reason that is tricky / non-obvious. - Keep doc comments and comments simple, one line, straightforward, and unbloated. diff --git a/crates/core/Cargo.toml b/crates/core/Cargo.toml index 61254eb..d555ad2 100644 --- a/crates/core/Cargo.toml +++ b/crates/core/Cargo.toml @@ -76,7 +76,7 @@ tooltip = [] menu = ["scroll_area"] dropdown_menu = ["menu", "button"] select = ["menu"] -dialog = [] +dialog = ["button"] empty_state = [] alert = [] toast = [] diff --git a/crates/core/src/dialog.rs b/crates/core/src/dialog.rs index 6aba2f4..3ce1a54 100644 --- a/crates/core/src/dialog.rs +++ b/crates/core/src/dialog.rs @@ -6,14 +6,17 @@ use gpui::{ IntoElement, KeyDownEvent, MouseButton, ParentElement as _, RenderOnce, Role, SharedString, StatefulInteractiveElement as _, Styled as _, Window, canvas, div, ease_out_quint, relative, }; +use gpui_native_icons::IconRole; use gpui_native_theme::ActiveTheme as _; -use crate::common::v_flex; +use crate::button::{Button, ButtonVariant}; +use crate::common::{Sizable, v_flex}; /// Requests the dialog's close: `false` closes. pub type ChangeHandler = Rc; type ContentFn = Box AnyElement>; type ActionsFn = Box Vec>; + /// A centred panel over a dim scrim: the platform's modal. #[derive(IntoElement)] pub struct Dialog { @@ -57,7 +60,7 @@ impl Dialog { self } - /// Requests close through `on_change` on escape and scrim clicks. + /// Requests close through `on_change` on escape, scrim clicks, and the close button. pub fn on_change(mut self, handler: F) -> Self where F: Fn(bool, &mut Window, &mut App) + 'static, @@ -154,6 +157,7 @@ impl RenderOnce for Dialog { let on_change = self.on_change.clone(); let alert = self.alert; + let show_close = !alert && on_change.is_some(); // Keyboard focus rides the panel across frames. let focus = gpui_native_utils::focus_handle(&self.id, window, cx); @@ -190,12 +194,40 @@ impl RenderOnce for Dialog { cx.stop_propagation(); } }) - .when_some(self.title, |panel, title| { + .when(self.title.is_some() || show_close, |panel| { panel.child( div() - .text_size(title_size) - .font_weight(FontWeight::SEMIBOLD) - .child(title), + .flex() + .items_center() + .justify_end() + .gap_2() + .when_some(self.title.clone(), |row, title| { + row.child( + div() + .flex_1() + .text_size(title_size) + .font_weight(FontWeight::SEMIBOLD) + .child(title), + ) + }) + .when(show_close, |row| { + row.child( + Button::new("dialog-close") + .icon(IconRole::Close) + .variant(ButtonVariant::Ghost) + .small() + .debug_selector(|| "dialog-close".to_string()) + .accessibility_label("Close") + .on_click({ + let on_change = on_change.clone(); + move |_, window, cx| { + if let Some(on_change) = &on_change { + on_change(false, window, cx); + } + } + }), + ) + }), ) }) .when_some(content, |panel, content| { diff --git a/crates/core/tests/dialog.rs b/crates/core/tests/dialog.rs index 06a5b09..ceece3b 100644 --- a/crates/core/tests/dialog.rs +++ b/crates/core/tests/dialog.rs @@ -158,3 +158,26 @@ fn clicking_the_panel_does_not_dismiss() { click(&mut visual, point(px(960.), px(540.))); assert!(changes.borrow().is_empty()); } + +/// Clicking the close button reports close like escape and the scrim; the +/// panel then unmounts the button with the rest of the dialog. +#[test] +fn the_close_button_reports_close_and_disappears() { + let (_cx, mut visual, changes) = dialog_window(true, false); + + let bounds = visual + .debug_bounds("dialog-close") + .expect("the close button rendered"); + click(&mut visual, bounds.center()); + assert_eq!(*changes.borrow(), vec![false]); + + draw(&mut visual); + assert!(visual.debug_bounds("dialog-close").is_none()); +} + +/// An alert answers only through its actions, so it renders no close button. +#[test] +fn an_alert_hides_the_close_button() { + let (_cx, mut visual, _changes) = dialog_window(true, true); + assert!(visual.debug_bounds("dialog-close").is_none()); +}