Revise the `MessageNotification` component (#24287)

Danilo Leal created

This PR makes adding icons to the primary and secondary actions, in the
`MessageNotification` component, optional. Also took the opportunity to
remove a probably unnecessary "third action" from it; streamlining the
component API (we had added that for a design that we're not using
anymore). I did keep the "more info" possibility, which may be useful in
the future, though.

Release Notes:

- N/A

Change summary

crates/extensions_ui/src/extension_suggest.rs |  13 +
crates/workspace/src/notifications.rs         | 157 ++++++++++----------
crates/workspace/src/workspace.rs             |   5 
crates/zed/src/zed.rs                         |  15 +
4 files changed, 101 insertions(+), 89 deletions(-)

Detailed changes

crates/extensions_ui/src/extension_suggest.rs 🔗

@@ -7,6 +7,7 @@ use editor::Editor;
 use extension_host::ExtensionStore;
 use gpui::{AppContext as _, Context, Entity, SharedString, Window};
 use language::Buffer;
+use ui::prelude::*;
 use workspace::notifications::simple_message_notification::MessageNotification;
 use workspace::{notifications::NotificationId, Workspace};
 
@@ -172,8 +173,10 @@ pub(crate) fn suggest(buffer: Entity<Buffer>, window: &mut Window, cx: &mut Cont
                     "Do you want to install the recommended '{}' extension for '{}' files?",
                     extension_id, file_name_or_extension
                 ))
-                .with_click_message("Yes, install extension")
-                .on_click({
+                .primary_message("Yes, install extension")
+                .primary_icon(IconName::Check)
+                .primary_icon_color(Color::Success)
+                .primary_on_click({
                     let extension_id = extension_id.clone();
                     move |_window, cx| {
                         let extension_id = extension_id.clone();
@@ -183,8 +186,10 @@ pub(crate) fn suggest(buffer: Entity<Buffer>, window: &mut Window, cx: &mut Cont
                         });
                     }
                 })
-                .with_secondary_click_message("No, don't install it")
-                .on_secondary_click(move |_window, cx| {
+                .secondary_message("No, don't install it")
+                .secondary_icon(IconName::Close)
+                .secondary_icon_color(Color::Error)
+                .secondary_on_click(move |_window, cx| {
                     let key = language_extension_key(&extension_id);
                     db::write_and_log(cx, move || {
                         KEY_VALUE_STORE.write_kvp(key, "dismissed".to_string())

crates/workspace/src/notifications.rs 🔗

@@ -124,8 +124,8 @@ impl Workspace {
                 Some((click_msg, on_click)) => {
                     let on_click = on_click.clone();
                     simple_message_notification::MessageNotification::new(toast.msg.clone())
-                        .with_click_message(click_msg.clone())
-                        .on_click(move |window, cx| on_click(window, cx))
+                        .primary_message(click_msg.clone())
+                        .primary_on_click(move |window, cx| on_click(window, cx))
                 }
                 None => simple_message_notification::MessageNotification::new(toast.msg.clone()),
             })
@@ -375,12 +375,14 @@ pub mod simple_message_notification {
 
     pub struct MessageNotification {
         build_content: Box<dyn Fn(&mut Window, &mut Context<Self>) -> AnyElement>,
-        on_click: Option<Arc<dyn Fn(&mut Window, &mut Context<Self>)>>,
-        click_message: Option<SharedString>,
-        secondary_click_message: Option<SharedString>,
+        primary_message: Option<SharedString>,
+        primary_icon: Option<IconName>,
+        primary_icon_color: Option<Color>,
+        primary_on_click: Option<Arc<dyn Fn(&mut Window, &mut Context<Self>)>>,
+        secondary_message: Option<SharedString>,
+        secondary_icon: Option<IconName>,
+        secondary_icon_color: Option<Color>,
         secondary_on_click: Option<Arc<dyn Fn(&mut Window, &mut Context<Self>)>>,
-        tertiary_click_message: Option<SharedString>,
-        tertiary_on_click: Option<Arc<dyn Fn(&mut Window, &mut Context<Self>)>>,
         more_info_message: Option<SharedString>,
         more_info_url: Option<Arc<str>>,
         show_close_button: bool,
@@ -404,12 +406,14 @@ pub mod simple_message_notification {
         {
             Self {
                 build_content: Box::new(content),
-                on_click: None,
-                click_message: None,
+                primary_message: None,
+                primary_icon: None,
+                primary_icon_color: None,
+                primary_on_click: None,
+                secondary_message: None,
+                secondary_icon: None,
+                secondary_icon_color: None,
                 secondary_on_click: None,
-                secondary_click_message: None,
-                tertiary_on_click: None,
-                tertiary_click_message: None,
                 more_info_message: None,
                 more_info_url: None,
                 show_close_button: true,
@@ -417,51 +421,55 @@ pub mod simple_message_notification {
             }
         }
 
-        pub fn with_click_message<S>(mut self, message: S) -> Self
+        pub fn primary_message<S>(mut self, message: S) -> Self
         where
             S: Into<SharedString>,
         {
-            self.click_message = Some(message.into());
+            self.primary_message = Some(message.into());
             self
         }
 
-        pub fn on_click<F>(mut self, on_click: F) -> Self
-        where
-            F: 'static + Fn(&mut Window, &mut Context<Self>),
-        {
-            self.on_click = Some(Arc::new(on_click));
+        pub fn primary_icon(mut self, icon: IconName) -> Self {
+            self.primary_icon = Some(icon);
             self
         }
 
-        pub fn with_secondary_click_message<S>(mut self, message: S) -> Self
-        where
-            S: Into<SharedString>,
-        {
-            self.secondary_click_message = Some(message.into());
+        pub fn primary_icon_color(mut self, color: Color) -> Self {
+            self.primary_icon_color = Some(color);
             self
         }
 
-        pub fn on_secondary_click<F>(mut self, on_click: F) -> Self
+        pub fn primary_on_click<F>(mut self, on_click: F) -> Self
         where
             F: 'static + Fn(&mut Window, &mut Context<Self>),
         {
-            self.secondary_on_click = Some(Arc::new(on_click));
+            self.primary_on_click = Some(Arc::new(on_click));
             self
         }
 
-        pub fn with_tertiary_click_message<S>(mut self, message: S) -> Self
+        pub fn secondary_message<S>(mut self, message: S) -> Self
         where
             S: Into<SharedString>,
         {
-            self.tertiary_click_message = Some(message.into());
+            self.secondary_message = Some(message.into());
             self
         }
 
-        pub fn on_tertiary_click<F>(mut self, on_click: F) -> Self
+        pub fn secondary_icon(mut self, icon: IconName) -> Self {
+            self.secondary_icon = Some(icon);
+            self
+        }
+
+        pub fn secondary_icon_color(mut self, color: Color) -> Self {
+            self.secondary_icon_color = Some(color);
+            self
+        }
+
+        pub fn secondary_on_click<F>(mut self, on_click: F) -> Self
         where
             F: 'static + Fn(&mut Window, &mut Context<Self>),
         {
-            self.tertiary_on_click = Some(Arc::new(on_click));
+            self.secondary_on_click = Some(Arc::new(on_click));
             self
         }
 
@@ -529,66 +537,63 @@ pub mod simple_message_notification {
                 .child(
                     h_flex()
                         .gap_1()
-                        .children(self.click_message.iter().map(|message| {
-                            Button::new(message.clone(), message.clone())
+                        .children(self.primary_message.iter().map(|message| {
+                            let mut button = Button::new(message.clone(), message.clone())
                                 .label_size(LabelSize::Small)
-                                .icon(IconName::Check)
-                                .icon_position(IconPosition::Start)
-                                .icon_size(IconSize::Small)
-                                .icon_color(Color::Success)
                                 .on_click(cx.listener(|this, _, window, cx| {
-                                    if let Some(on_click) = this.on_click.as_ref() {
+                                    if let Some(on_click) = this.primary_on_click.as_ref() {
                                         (on_click)(window, cx)
                                     };
                                     this.dismiss(cx)
-                                }))
+                                }));
+
+                            if let Some(icon) = self.primary_icon {
+                                button = button
+                                    .icon(icon)
+                                    .icon_color(self.primary_icon_color.unwrap_or(Color::Muted))
+                                    .icon_position(IconPosition::Start)
+                                    .icon_size(IconSize::Small);
+                            }
+
+                            button
                         }))
-                        .children(self.secondary_click_message.iter().map(|message| {
-                            Button::new(message.clone(), message.clone())
+                        .children(self.secondary_message.iter().map(|message| {
+                            let mut button = Button::new(message.clone(), message.clone())
                                 .label_size(LabelSize::Small)
-                                .icon(IconName::Close)
-                                .icon_position(IconPosition::Start)
-                                .icon_size(IconSize::Small)
-                                .icon_color(Color::Error)
                                 .on_click(cx.listener(|this, _, window, cx| {
                                     if let Some(on_click) = this.secondary_on_click.as_ref() {
                                         (on_click)(window, cx)
                                     };
                                     this.dismiss(cx)
-                                }))
+                                }));
+
+                            if let Some(icon) = self.secondary_icon {
+                                button = button
+                                    .icon(icon)
+                                    .icon_position(IconPosition::Start)
+                                    .icon_size(IconSize::Small)
+                                    .icon_color(self.secondary_icon_color.unwrap_or(Color::Muted));
+                            }
+
+                            button
                         }))
                         .child(
-                            h_flex()
-                                .w_full()
-                                .gap_1()
-                                .justify_end()
-                                .children(self.tertiary_click_message.iter().map(|message| {
-                                    Button::new(message.clone(), message.clone())
-                                        .label_size(LabelSize::Small)
-                                        .on_click(cx.listener(|this, _, window, cx| {
-                                            if let Some(on_click) = this.tertiary_on_click.as_ref()
-                                            {
-                                                (on_click)(window, cx)
-                                            };
-                                            this.dismiss(cx)
-                                        }))
-                                }))
-                                .children(
-                                    self.more_info_message
-                                        .iter()
-                                        .zip(self.more_info_url.iter())
-                                        .map(|(message, url)| {
-                                            let url = url.clone();
-                                            Button::new(message.clone(), message.clone())
-                                                .label_size(LabelSize::Small)
-                                                .icon(IconName::ArrowUpRight)
-                                                .icon_size(IconSize::Indicator)
-                                                .icon_color(Color::Muted)
-                                                .on_click(cx.listener(move |_, _, _, cx| {
-                                                    cx.open_url(&url);
-                                                }))
-                                        }),
-                                ),
+                            h_flex().w_full().justify_end().children(
+                                self.more_info_message
+                                    .iter()
+                                    .zip(self.more_info_url.iter())
+                                    .map(|(message, url)| {
+                                        let url = url.clone();
+                                        Button::new(message.clone(), message.clone())
+                                            .label_size(LabelSize::Small)
+                                            .icon(IconName::ArrowUpRight)
+                                            .icon_size(IconSize::Indicator)
+                                            .icon_color(Color::Muted)
+                                            .on_click(cx.listener(move |_, _, _, cx| {
+                                                cx.open_url(&url);
+                                            }))
+                                    }),
+                            ),
                         ),
                 )
         }

crates/workspace/src/workspace.rs 🔗

@@ -5207,8 +5207,9 @@ fn notify_if_database_failed(workspace: WindowHandle<Workspace>, cx: &mut AsyncA
                     |cx| {
                         cx.new(|_| {
                             MessageNotification::new("Failed to load the database file.")
-                                .with_click_message("File an issue")
-                                .on_click(|_window, cx| cx.open_url(REPORT_ISSUE_URL))
+                                .primary_message("File an Issue")
+                                .primary_icon(IconName::Plus)
+                                .primary_on_click(|_window, cx| cx.open_url(REPORT_ISSUE_URL))
                         })
                     },
                 );

crates/zed/src/zed.rs 🔗

@@ -49,7 +49,7 @@ use std::time::Duration;
 use std::{borrow::Cow, ops::Deref, path::Path, sync::Arc};
 use terminal_view::terminal_panel::{self, TerminalPanel};
 use theme::{ActiveTheme, ThemeSettings};
-use ui::PopoverMenuHandle;
+use ui::{prelude::*, PopoverMenuHandle};
 use util::markdown::MarkdownString;
 use util::{asset_str, ResultExt};
 use uuid::Uuid;
@@ -1177,8 +1177,8 @@ fn show_keymap_file_json_error(
     show_app_notification(notification_id, cx, move |cx| {
         cx.new(|_cx| {
             MessageNotification::new(message.clone())
-                .with_click_message("Open keymap file")
-                .on_click(|window, cx| {
+                .primary_message("Open Keymap File")
+                .primary_on_click(|window, cx| {
                     window.dispatch_action(zed_actions::OpenKeymap.boxed_clone(), cx);
                     cx.emit(DismissEvent);
                 })
@@ -1220,8 +1220,8 @@ fn show_keymap_file_load_error(
                             ))
                             .into_any()
                     })
-                    .with_click_message("Open keymap file")
-                    .on_click(|window, cx| {
+                    .primary_message("Open Keymap File")
+                    .primary_on_click(|window, cx| {
                         window.dispatch_action(zed_actions::OpenKeymap.boxed_clone(), cx);
                         cx.emit(DismissEvent);
                     })
@@ -1273,8 +1273,9 @@ pub fn handle_settings_changed(error: Option<anyhow::Error>, cx: &mut App) {
             show_app_notification(id, cx, move |cx| {
                 cx.new(|_cx| {
                     MessageNotification::new(format!("Invalid user settings file\n{error}"))
-                        .with_click_message("Open settings file")
-                        .on_click(|window, cx| {
+                        .primary_message("Open Settings File")
+                        .primary_icon(IconName::Settings)
+                        .primary_on_click(|window, cx| {
                             window.dispatch_action(zed_actions::OpenSettings.boxed_clone(), cx);
                             cx.emit(DismissEvent);
                         })