From 203cb7a7a25583b814c0875d142c7e9d73e356b8 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 8 May 2025 17:10:30 +0300 Subject: [PATCH] Restyle notification close control (#30262) Follow-up of https://github.com/zed-industries/zed/pull/30015 Merges suppress and close buttons into one, with `shift` changing the state and showing different tooltips. Currently, there's no tooltip for notification suppress action, hence none is displayed in the video: https://github.com/user-attachments/assets/678c4d76-a86e-4fe9-8d7b-92996470a8a8 Release Notes: - N/A --- crates/auto_update_ui/src/auto_update_ui.rs | 1 + crates/collab_ui/src/notification_panel.rs | 55 +++++--- crates/workspace/src/notifications.rs | 134 +++++++++++--------- crates/workspace/src/workspace.rs | 10 ++ 4 files changed, 123 insertions(+), 77 deletions(-) diff --git a/crates/auto_update_ui/src/auto_update_ui.rs b/crates/auto_update_ui/src/auto_update_ui.rs index 044a6b2922..07c1821588 100644 --- a/crates/auto_update_ui/src/auto_update_ui.rs +++ b/crates/auto_update_ui/src/auto_update_ui.rs @@ -155,6 +155,7 @@ pub fn notify_if_app_was_updated(cx: &mut App) { } cx.emit(DismissEvent); }) + .show_suppress_button(false) }) }, ); diff --git a/crates/collab_ui/src/notification_panel.rs b/crates/collab_ui/src/notification_panel.rs index 817d15aa0e..166a383c6b 100644 --- a/crates/collab_ui/src/notification_panel.rs +++ b/crates/collab_ui/src/notification_panel.rs @@ -6,8 +6,8 @@ use collections::HashMap; use db::kvp::KEY_VALUE_STORE; use futures::StreamExt; use gpui::{ - AnyElement, App, AsyncWindowContext, Context, CursorStyle, DismissEvent, Element, Entity, - EventEmitter, FocusHandle, Focusable, InteractiveElement, IntoElement, ListAlignment, + AnyElement, App, AsyncWindowContext, ClickEvent, Context, CursorStyle, DismissEvent, Element, + Entity, EventEmitter, FocusHandle, Focusable, InteractiveElement, IntoElement, ListAlignment, ListScrollEvent, ListState, ParentElement, Render, StatefulInteractiveElement, Styled, Task, WeakEntity, Window, actions, div, img, list, px, }; @@ -22,7 +22,6 @@ use ui::{ Avatar, Button, Icon, IconButton, IconName, Label, Tab, Tooltip, h_flex, prelude::*, v_flex, }; use util::{ResultExt, TryFutureExt}; -use workspace::SuppressNotification; use workspace::notifications::{ Notification as WorkspaceNotification, NotificationId, SuppressEvent, }; @@ -812,32 +811,50 @@ impl NotificationToast { } impl Render for NotificationToast { - fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { + fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { let user = self.actor.clone(); + let suppress = window.modifiers().shift; + let (close_id, close_icon) = if suppress { + ("suppress", IconName::Minimize) + } else { + ("close", IconName::Close) + }; + h_flex() .id("notification_panel_toast") .elevation_3(cx) .p_2() - .gap_2() + .justify_between() .children(user.map(|user| Avatar::new(user.avatar_uri.clone()))) .child(Label::new(self.text.clone())) + .on_modifiers_changed(cx.listener(|_, _, _, cx| cx.notify())) .child( - IconButton::new("close", IconName::Close) - .tooltip(|window, cx| Tooltip::for_action("Close", &menu::Cancel, window, cx)) - .on_click(cx.listener(|_, _, _, cx| cx.emit(DismissEvent))), - ) - .child( - IconButton::new("suppress", IconName::SquareMinus) - .tooltip(|window, cx| { - Tooltip::for_action( - "Do not show until restart", - &SuppressNotification, - window, - cx, - ) + IconButton::new(close_id, close_icon) + .tooltip(move |window, cx| { + if suppress { + Tooltip::for_action( + "Suppress.\nClose with click.", + &workspace::SuppressNotification, + window, + cx, + ) + } else { + Tooltip::for_action( + "Close.\nSuppress with shift-click", + &menu::Cancel, + window, + cx, + ) + } }) - .on_click(cx.listener(|_, _, _, cx| cx.emit(SuppressEvent))), + .on_click(cx.listener(move |_, _: &ClickEvent, _, cx| { + if suppress { + cx.emit(SuppressEvent); + } else { + cx.emit(DismissEvent); + } + })), ) .on_click(cx.listener(|this, _, window, cx| { this.focus_notification_panel(window, cx); diff --git a/crates/workspace/src/notifications.rs b/crates/workspace/src/notifications.rs index c48fe9aeb9..3205a5dc86 100644 --- a/crates/workspace/src/notifications.rs +++ b/crates/workspace/src/notifications.rs @@ -1,7 +1,8 @@ use crate::{SuppressNotification, Toast, Workspace}; use gpui::{ - AnyView, App, AppContext as _, AsyncWindowContext, ClipboardItem, Context, DismissEvent, - Entity, EventEmitter, FocusHandle, Focusable, PromptLevel, Render, ScrollHandle, Task, svg, + AnyView, App, AppContext as _, AsyncWindowContext, ClickEvent, ClipboardItem, Context, + DismissEvent, Entity, EventEmitter, FocusHandle, Focusable, PromptLevel, Render, ScrollHandle, + Task, svg, }; use parking_lot::Mutex; use std::ops::Deref; @@ -263,6 +264,13 @@ impl Render for LanguageServerPrompt { PromptLevel::Critical => (IconName::XCircle, Color::Error), }; + let suppress = window.modifiers().shift; + let (close_id, close_icon) = if suppress { + ("suppress", IconName::Minimize) + } else { + ("close", IconName::Close) + }; + div() .id("language_server_prompt_notification") .group("language_server_prompt_notification") @@ -272,6 +280,7 @@ impl Render for LanguageServerPrompt { .elevation_3(cx) .overflow_y_scroll() .track_scroll(&self.scroll_handle) + .on_modifiers_changed(cx.listener(|_, _, _, cx| cx.notify())) .child( v_flex() .p_3() @@ -289,20 +298,6 @@ impl Render for LanguageServerPrompt { .child( h_flex() .gap_2() - .child( - IconButton::new("suppress", IconName::SquareMinus) - .tooltip(|window, cx| { - Tooltip::for_action( - "Do not show until restart", - &SuppressNotification, - window, - cx, - ) - }) - .on_click( - cx.listener(|_, _, _, cx| cx.emit(SuppressEvent)), - ), - ) .child( IconButton::new("copy", IconName::Copy) .on_click({ @@ -316,18 +311,33 @@ impl Render for LanguageServerPrompt { .tooltip(Tooltip::text("Copy Description")), ) .child( - IconButton::new("close", IconName::Close) - .tooltip(|window, cx| { - Tooltip::for_action( - "Close", - &menu::Cancel, - window, - cx, - ) + IconButton::new(close_id, close_icon) + .tooltip(move |window, cx| { + if suppress { + Tooltip::for_action( + "Suppress.\nClose with click.", + &SuppressNotification, + window, + cx, + ) + } else { + Tooltip::for_action( + "Close.\nSuppress with shift-click.", + &menu::Cancel, + window, + cx, + ) + } }) - .on_click(cx.listener(|_, _, _, cx| { - cx.emit(gpui::DismissEvent) - })), + .on_click(cx.listener( + move |_, _: &ClickEvent, _, cx| { + if suppress { + cx.emit(SuppressEvent); + } else { + cx.emit(DismissEvent); + } + }, + )), ), ), ) @@ -416,9 +426,8 @@ impl Render for ErrorMessagePrompt { }), ) .child( - ui::IconButton::new("close", ui::IconName::Close).on_click( - cx.listener(|_, _, _, cx| cx.emit(gpui::DismissEvent)), - ), + ui::IconButton::new("close", ui::IconName::Close) + .on_click(cx.listener(|_, _, _, cx| cx.emit(DismissEvent))), ), ) .child( @@ -456,8 +465,8 @@ pub mod simple_message_notification { use std::sync::Arc; use gpui::{ - AnyElement, DismissEvent, EventEmitter, FocusHandle, Focusable, ParentElement, Render, - SharedString, Styled, div, + AnyElement, ClickEvent, DismissEvent, EventEmitter, FocusHandle, Focusable, ParentElement, + Render, SharedString, Styled, div, }; use ui::{Tooltip, prelude::*}; @@ -637,6 +646,14 @@ pub mod simple_message_notification { impl Render for MessageNotification { fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { + let show_suppress_button = self.show_suppress_button; + let suppress = show_suppress_button && window.modifiers().shift; + let (close_id, close_icon) = if suppress { + ("suppress", IconName::Minimize) + } else { + ("close", IconName::Close) + }; + v_flex() .occlude() .p_3() @@ -655,42 +672,43 @@ pub mod simple_message_notification { }) .child(div().max_w_96().child((self.build_content)(window, cx))), ) - .child( - h_flex() - .gap_2() - .when(self.show_suppress_button, |this| { - this.child( - IconButton::new("suppress", IconName::SquareMinus) - .tooltip(|window, cx| { + .when(self.show_close_button, |this| { + this.on_modifiers_changed(cx.listener(|_, _, _, cx| cx.notify())) + .child( + IconButton::new(close_id, close_icon) + .tooltip(move |window, cx| { + if suppress { Tooltip::for_action( - "Do not show until restart", + "Suppress.\nClose with click.", &SuppressNotification, window, cx, ) - }) - .on_click(cx.listener(|_, _, _, cx| { - cx.emit(SuppressEvent); - })), - ) - }) - .when(self.show_close_button, |this| { - this.child( - IconButton::new("close", IconName::Close) - .tooltip(|window, cx| { + } else if show_suppress_button { Tooltip::for_action( - "Close", + "Close.\nSuppress with shift-click.", &menu::Cancel, window, cx, ) - }) - .on_click( - cx.listener(|this, _, _, cx| this.dismiss(cx)), - ), - ) - }), - ), + } else { + Tooltip::for_action( + "Close.", + &menu::Cancel, + window, + cx, + ) + } + }) + .on_click(cx.listener(move |_, _: &ClickEvent, _, cx| { + if suppress { + cx.emit(SuppressEvent); + } else { + cx.emit(DismissEvent); + } + })), + ) + }), ) .child( h_flex() diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index de65f3f3a1..e82d305c21 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -5729,6 +5729,11 @@ impl Render for Workspace { let theme = cx.theme().clone(); let colors = theme.colors(); + let notification_entities = self + .notifications + .iter() + .map(|(_, notification)| notification.entity_id()) + .collect::>(); client_side_decorations( self.actions(div(), window, cx) @@ -5744,6 +5749,11 @@ impl Render for Workspace { .text_color(colors.text) .overflow_hidden() .children(self.titlebar_item.clone()) + .on_modifiers_changed(move |_, _, cx| { + for &id in ¬ification_entities { + cx.notify(id); + } + }) .child( div() .size_full()