From 76d58ac2952c3e136b52092c8cb43b5ed6d3a479 Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Thu, 8 Aug 2024 07:12:36 -0300 Subject: [PATCH] Add design tweaks to the AI configuration panel (#15894) This PR polishes elements around setting up LLM providers on the Assistant panel, including: - [x] Adding banners for promoting Zed AI and to deal with the "No provider set up" scenario - [x] Tweaking the error popover whenever there's no API key added - [ ] Making configuration panel scrollable --- Release Notes: - N/A --------- Co-authored-by: Thorsten Ball Co-authored-by: Bennet Bo Fenner <53836821+bennetbo@users.noreply.github.com> Co-authored-by: Marshall Bowers <1486634+maxdeviant@users.noreply.github.com> --- crates/assistant/src/assistant_panel.rs | 301 ++++++++++-------- .../language_model/src/provider/anthropic.rs | 8 +- .../src/provider/copilot_chat.rs | 8 +- crates/language_model/src/provider/google.rs | 8 +- crates/language_model/src/provider/open_ai.rs | 10 +- crates/ui/src/components/button/button.rs | 20 +- .../ui/src/components/button/button_icon.rs | 9 +- 7 files changed, 211 insertions(+), 153 deletions(-) diff --git a/crates/assistant/src/assistant_panel.rs b/crates/assistant/src/assistant_panel.rs index e2fe437b746d3ae6fe1d7c0c90a13942916f5bad..35af2c4ad017f01bfd24317d8ecfbd3290fed986 100644 --- a/crates/assistant/src/assistant_panel.rs +++ b/crates/assistant/src/assistant_panel.rs @@ -2281,6 +2281,22 @@ impl ContextEditor { } }); + let trigger = Button::new("show-error", "Error") + .color(Color::Error) + .selected_label_color(Color::Error) + .selected_icon_color(Color::Error) + .icon(IconName::XCircle) + .icon_color(Color::Error) + .icon_size(IconSize::Small) + .icon_position(IconPosition::Start) + .tooltip(move |cx| { + Tooltip::with_meta( + "Error interacting with language model", + None, + "Click for more details", + cx, + ) + }); h_flex() .id(("message_header", message_id.as_u64())) .pl(cx.gutter_dimensions.full_width()) @@ -2292,13 +2308,14 @@ impl ContextEditor { .children( if let MessageStatus::Error(error) = message.status.clone() { Some( - div() - .id("error") - .tooltip(move |cx| Tooltip::text(error.clone(), cx)) - .child( - Icon::new(IconName::ExclamationTriangle) - .color(Color::Error), - ), + PopoverMenu::new("show-error-popover") + .menu(move |cx| { + Some(cx.new_view(|cx| ErrorPopover { + error: error.clone(), + focus_handle: cx.focus_handle(), + })) + }) + .trigger(trigger), ) } else { None @@ -2483,33 +2500,31 @@ impl ContextEditor { .unwrap_or_else(|| Cow::Borrowed(DEFAULT_TAB_TITLE)) } - fn dismiss_error_message(&mut self, cx: &mut ViewContext) { - self.error_message = None; - cx.notify(); - } - fn render_notice(&self, cx: &mut ViewContext) -> Option { use feature_flags::FeatureFlagAppExt; let nudge = self.assistant_panel.upgrade().map(|assistant_panel| { assistant_panel.read(cx).show_zed_ai_notice && cx.has_flag::() }); - if let Some(error) = self.error_message.clone() { - Some(Self::render_error_popover(error, cx).into_any_element()) - } else if nudge.unwrap_or(false) { + if nudge.map_or(false, |value| value) { Some( - v_flex() - .elevation_3(cx) - .p_2() - .gap_2() + h_flex() + .p_3() + .border_b_1() + .border_color(cx.theme().colors().border_variant) + .bg(cx.theme().colors().editor_background) + .justify_between() .child( - Label::new("Use Zed AI") - .size(LabelSize::Small) - .color(Color::Muted), + h_flex() + .gap_3() + .child(Icon::new(IconName::ZedAssistant).color(Color::Accent)) + .child(Label::new("Zed AI is here! Get started by signing in →")), ) - .child(h_flex().justify_end().child( - Button::new("sign-in", "Sign in to use Zed AI").on_click(cx.listener( - |this, _event, cx| { + .child( + Button::new("sign-in", "Sign in") + .size(ButtonSize::Compact) + .style(ButtonStyle::Filled) + .on_click(cx.listener(|this, _event, cx| { let client = this .workspace .update(cx, |workspace, _| workspace.client().clone()) @@ -2522,34 +2537,43 @@ impl ContextEditor { }) .detach_and_log_err(cx) } - }, - )), - )) + })), + ) .into_any_element(), ) } else if let Some(configuration_error) = configuration_error(cx) { let label = match configuration_error { - ConfigurationError::NoProvider => "No provider configured", - ConfigurationError::ProviderNotAuthenticated => "Provider is not configured", + ConfigurationError::NoProvider => "No LLM provider selected.", + ConfigurationError::ProviderNotAuthenticated => "LLM provider is not configured.", }; Some( - v_flex() - .elevation_3(cx) - .p_2() - .gap_2() - .child(Label::new(label).size(LabelSize::Small).color(Color::Muted)) + h_flex() + .p_3() + .border_b_1() + .border_color(cx.theme().colors().border_variant) + .bg(cx.theme().colors().editor_background) + .justify_between() .child( - h_flex().justify_end().child( - Button::new("open-configuration", "Open configuration") - .icon(IconName::Settings) - .icon_size(IconSize::Small) - .on_click({ - let focus_handle = self.focus_handle(cx).clone(); - move |_event, cx| { - focus_handle.dispatch_action(&ShowConfiguration, cx); - } - }), - ), + h_flex() + .gap_3() + .child( + Icon::new(IconName::ExclamationTriangle) + .size(IconSize::Small) + .color(Color::Warning), + ) + .child(Label::new(label)), + ) + .child( + Button::new("open-configuration", "Open configuration") + .size(ButtonSize::Compact) + .icon_size(IconSize::Small) + .style(ButtonStyle::Filled) + .on_click({ + let focus_handle = self.focus_handle(cx).clone(); + move |_event, cx| { + focus_handle.dispatch_action(&ShowConfiguration, cx); + } + }), ) .into_any_element(), ) @@ -2558,28 +2582,6 @@ impl ContextEditor { } } - fn render_error_popover(error: SharedString, cx: &mut ViewContext) -> Div { - v_flex() - .p_2() - .elevation_2(cx) - .bg(cx.theme().colors().surface_background) - .min_w_24() - .occlude() - .child( - Label::new("Error interacting with language model") - .size(LabelSize::Small) - .weight(FontWeight::BOLD) - .color(Color::Muted), - ) - .child(Label::new(error).size(LabelSize::Small)) - .child( - h_flex().justify_end().child( - Button::new("dismiss", "Dismiss") - .on_click(cx.listener(|this, _, cx| this.dismiss_error_message(cx))), - ), - ) - } - fn render_send_button(&self, cx: &mut ViewContext) -> impl IntoElement { let focus_handle = self.focus_handle(cx).clone(); let button_text = match self.active_workflow_step(cx) { @@ -2664,7 +2666,7 @@ impl EventEmitter for ContextEditor {} impl Render for ContextEditor { fn render(&mut self, cx: &mut ViewContext) -> impl IntoElement { - div() + v_flex() .key_context("ContextEditor") .capture_action(cx.listener(ContextEditor::cancel_last_assist)) .capture_action(cx.listener(ContextEditor::save)) @@ -2675,7 +2677,7 @@ impl Render for ContextEditor { .on_action(cx.listener(ContextEditor::split)) .on_action(cx.listener(ContextEditor::debug_edit_steps)) .size_full() - .v_flex() + .children(self.render_notice(cx)) .child( div() .flex_grow() @@ -2683,30 +2685,50 @@ impl Render for ContextEditor { .child(self.editor.clone()), ) .child( - h_flex() - .flex_none() - .relative() - .when_some(self.render_notice(cx), |this, notice| { - this.child( - div() - .absolute() - .w_3_4() - .min_w_24() - .max_w_128() - .right_4() - .bottom_9() - .child(notice), - ) - }) - .child( - h_flex() - .w_full() - .absolute() - .right_4() - .bottom_2() - .justify_end() - .child(self.render_send_button(cx)), - ), + h_flex().flex_none().relative().child( + h_flex() + .w_full() + .absolute() + .right_4() + .bottom_2() + .justify_end() + .child(self.render_send_button(cx)), + ), + ) + } +} + +struct ErrorPopover { + error: SharedString, + focus_handle: FocusHandle, +} + +impl EventEmitter for ErrorPopover {} + +impl FocusableView for ErrorPopover { + fn focus_handle(&self, _: &AppContext) -> FocusHandle { + self.focus_handle.clone() + } +} + +impl Render for ErrorPopover { + fn render(&mut self, cx: &mut ViewContext) -> impl IntoElement { + v_flex() + .mt_2() + .max_w_96() + .py_2() + .px_3() + .gap_0p5() + .elevation_2(cx) + .bg(cx.theme().colors().surface_background) + .occlude() + .child(Label::new("Error interacting with language model").weight(FontWeight::SEMIBOLD)) + .child(Label::new(self.error.clone())) + .child( + h_flex().justify_end().mt_1().child( + Button::new("dismiss", "Dismiss") + .on_click(cx.listener(|_, _, cx| cx.emit(DismissEvent))), + ), ) } } @@ -3326,13 +3348,38 @@ impl ConfigurationView { let provider_name = provider.name().0.clone(); let configuration_view = self.configuration_views.get(&provider.id()).cloned(); + let open_new_context = cx.listener({ + let provider = provider.clone(); + move |_, _, cx| { + cx.emit(ConfigurationViewEvent::NewProviderContextEditor( + provider.clone(), + )) + } + }); + v_flex() - .gap_4() - .child(Headline::new(provider_name.clone()).size(HeadlineSize::Medium)) + .gap_2() + .child( + h_flex() + .justify_between() + .child(Headline::new(provider_name.clone()).size(HeadlineSize::Small)) + .when(provider.is_authenticated(cx), move |this| { + this.child( + h_flex().justify_end().child( + Button::new("new-context", "Open new context") + .icon_position(IconPosition::Start) + .icon(IconName::Plus) + .style(ButtonStyle::Filled) + .layer(ElevationIndex::ModalSurface) + .on_click(open_new_context), + ), + ) + }), + ) .child( div() .p(Spacing::Large.rems(cx)) - .bg(cx.theme().colors().title_bar_background) + .bg(cx.theme().colors().surface_background) .border_1() .border_color(cx.theme().colors().border_variant) .rounded_md() @@ -3346,28 +3393,6 @@ impl ConfigurationView { this.child(configuration_view) }), ) - .when(provider.is_authenticated(cx), move |this| { - this.child( - h_flex().justify_end().child( - Button::new( - "new-context", - format!("Open new context using {}", provider_name), - ) - .icon_position(IconPosition::Start) - .icon(IconName::Plus) - .style(ButtonStyle::Filled) - .layer(ElevationIndex::ModalSurface) - .on_click(cx.listener({ - let provider = provider.clone(); - move |_, _, cx| { - cx.emit(ConfigurationViewEvent::NewProviderContextEditor( - provider.clone(), - )) - } - })), - ), - ) - }) } } @@ -3382,26 +3407,30 @@ impl Render for ConfigurationView { v_flex() .id("assistant-configuration-view") .track_focus(&self.focus_handle) - .w_full() - .min_h_full() - .p(Spacing::XXLarge.rems(cx)) + .bg(cx.theme().colors().editor_background) + .size_full() .overflow_y_scroll() - .gap_6() .child( v_flex() - .gap_2() - .child(Headline::new("Configure your Assistant").size(HeadlineSize::Medium)), - ) - .child( - v_flex() - .gap_2() + .p(Spacing::XXLarge.rems(cx)) + .border_b_1() + .border_color(cx.theme().colors().border) + .gap_1() + .child(Headline::new("Configure your Assistant").size(HeadlineSize::Medium)) .child( Label::new( - "At least one provider must be configured to use the assistant.", + "At least one LLM provider must be configured to use the Assistant.", ) .color(Color::Muted), - ) - .child(v_flex().mt_2().gap_4().children(provider_views)), + ), + ) + .child( + v_flex() + .p(Spacing::XXLarge.rems(cx)) + .mt_1() + .gap_6() + .size_full() + .children(provider_views), ) } } @@ -3514,8 +3543,12 @@ fn render_docs_slash_command_trailer( children.push( div() .id(("latest-error", row.0)) - .child(Icon::new(IconName::ExclamationTriangle).color(Color::Warning)) - .tooltip(move |cx| Tooltip::text(format!("failed to index: {latest_error}"), cx)) + .child( + Icon::new(IconName::ExclamationTriangle) + .size(IconSize::Small) + .color(Color::Warning), + ) + .tooltip(move |cx| Tooltip::text(format!("Failed to index: {latest_error}"), cx)) .into_any_element(), ) } diff --git a/crates/language_model/src/provider/anthropic.rs b/crates/language_model/src/provider/anthropic.rs index 1a2c06e4c774bd45cb18156bf782c4ce17249f65..2fae6e4b099be292d09e1c5223edd0309e3b9b77 100644 --- a/crates/language_model/src/provider/anthropic.rs +++ b/crates/language_model/src/provider/anthropic.rs @@ -18,7 +18,7 @@ use settings::{Settings, SettingsStore}; use std::{sync::Arc, time::Duration}; use strum::IntoEnumIterator; use theme::ThemeSettings; -use ui::{prelude::*, Indicator}; +use ui::{prelude::*, Icon, IconName}; use util::ResultExt; const PROVIDER_ID: &str = "anthropic"; @@ -535,9 +535,9 @@ impl Render for ConfigurationView { .justify_between() .child( h_flex() - .gap_2() - .child(Indicator::dot().color(Color::Success)) - .child(Label::new("API key configured").size(LabelSize::Small)), + .gap_1() + .child(Icon::new(IconName::Check).color(Color::Success)) + .child(Label::new("API key configured.")), ) .child( Button::new("reset-key", "Reset key") diff --git a/crates/language_model/src/provider/copilot_chat.rs b/crates/language_model/src/provider/copilot_chat.rs index 5457be3c859eb5d40e5bd92ca37e31e9d3be8374..a0bdc3beab64f5f8bc0721944708db0f27dbdd03 100644 --- a/crates/language_model/src/provider/copilot_chat.rs +++ b/crates/language_model/src/provider/copilot_chat.rs @@ -18,8 +18,8 @@ use settings::{Settings, SettingsStore}; use std::time::Duration; use strum::IntoEnumIterator; use ui::{ - div, h_flex, v_flex, Button, ButtonCommon, Clickable, Color, Context, FixedWidth, IconName, - IconPosition, IconSize, Indicator, IntoElement, Label, LabelCommon, ParentElement, Styled, + div, h_flex, v_flex, Button, ButtonCommon, Clickable, Color, Context, FixedWidth, Icon, + IconName, IconPosition, IconSize, IntoElement, Label, LabelCommon, ParentElement, Styled, ViewContext, VisualContext, WindowContext, }; @@ -305,8 +305,8 @@ impl Render for ConfigurationView { if self.state.read(cx).is_authenticated(cx) { const LABEL: &str = "Authorized."; h_flex() - .gap_2() - .child(Indicator::dot().color(Color::Success)) + .gap_1() + .child(Icon::new(IconName::Check).color(Color::Success)) .child(Label::new(LABEL)) } else { let loading_icon = svg() diff --git a/crates/language_model/src/provider/google.rs b/crates/language_model/src/provider/google.rs index 27a1e7cf2f53b30d02c096e9551dc7b235f79e43..1fee3fb3483780a2344af1a5f81b4d044ec8fc99 100644 --- a/crates/language_model/src/provider/google.rs +++ b/crates/language_model/src/provider/google.rs @@ -14,7 +14,7 @@ use settings::{Settings, SettingsStore}; use std::{future, sync::Arc, time::Duration}; use strum::IntoEnumIterator; use theme::ThemeSettings; -use ui::{prelude::*, Indicator}; +use ui::{prelude::*, Icon, IconName}; use util::ResultExt; use crate::{ @@ -454,9 +454,9 @@ impl Render for ConfigurationView { .justify_between() .child( h_flex() - .gap_2() - .child(Indicator::dot().color(Color::Success)) - .child(Label::new("API key configured").size(LabelSize::Small)), + .gap_1() + .child(Icon::new(IconName::Check).color(Color::Success)) + .child(Label::new("API key configured.")), ) .child( Button::new("reset-key", "Reset key") diff --git a/crates/language_model/src/provider/open_ai.rs b/crates/language_model/src/provider/open_ai.rs index aacb386651793201104cdf9a51cebe7b747431ce..1d696d28ca1a9e82af605b5d36a7e0dd54a0e467 100644 --- a/crates/language_model/src/provider/open_ai.rs +++ b/crates/language_model/src/provider/open_ai.rs @@ -16,7 +16,7 @@ use settings::{Settings, SettingsStore}; use std::{sync::Arc, time::Duration}; use strum::IntoEnumIterator; use theme::ThemeSettings; -use ui::{prelude::*, Indicator}; +use ui::{prelude::*, Icon, IconName}; use util::ResultExt; use crate::{ @@ -505,7 +505,7 @@ impl Render for ConfigurationView { .size_full() .on_action(cx.listener(Self::save_api_key)) .children( - INSTRUCTIONS.map(|instruction| Label::new(instruction).size(LabelSize::Small)), + INSTRUCTIONS.map(|instruction| Label::new(instruction)), ) .child( h_flex() @@ -530,9 +530,9 @@ impl Render for ConfigurationView { .justify_between() .child( h_flex() - .gap_2() - .child(Indicator::dot().color(Color::Success)) - .child(Label::new("API key configured").size(LabelSize::Small)), + .gap_1() + .child(Icon::new(IconName::Check).color(Color::Success)) + .child(Label::new("API key configured.")), ) .child( Button::new("reset-key", "Reset key") diff --git a/crates/ui/src/components/button/button.rs b/crates/ui/src/components/button/button.rs index 906d808c0c625a75271657a370e641bd338469ba..babcafbfac85557435d2d32268f0ded35ead9eed 100644 --- a/crates/ui/src/components/button/button.rs +++ b/crates/ui/src/components/button/button.rs @@ -81,11 +81,13 @@ pub struct Button { label_color: Option, label_size: Option, selected_label: Option, + selected_label_color: Option, icon: Option, icon_position: Option, icon_size: Option, icon_color: Option, selected_icon: Option, + selected_icon_color: Option, key_binding: Option, } @@ -103,11 +105,13 @@ impl Button { label_color: None, label_size: None, selected_label: None, + selected_label_color: None, icon: None, icon_position: None, icon_size: None, icon_color: None, selected_icon: None, + selected_icon_color: None, key_binding: None, } } @@ -130,6 +134,12 @@ impl Button { self } + /// Sets the label color used when the button is in a selected state. + pub fn selected_label_color(mut self, color: impl Into>) -> Self { + self.selected_label_color = color.into(); + self + } + /// Assigns an icon to the button. pub fn icon(mut self, icon: impl Into>) -> Self { self.icon = icon.into(); @@ -160,6 +170,12 @@ impl Button { self } + /// Sets the icon color used when the button is in a selected state. + pub fn selected_icon_color(mut self, color: impl Into>) -> Self { + self.selected_icon_color = color.into(); + self + } + /// Binds a key combination to the button for keyboard shortcuts. pub fn key_binding(mut self, key_binding: impl Into>) -> Self { self.key_binding = key_binding.into(); @@ -366,7 +382,7 @@ impl RenderOnce for Button { let label_color = if is_disabled { Color::Disabled } else if is_selected { - Color::Selected + self.selected_label_color.unwrap_or(Color::Selected) } else { self.label_color.unwrap_or_default() }; @@ -380,6 +396,7 @@ impl RenderOnce for Button { .disabled(is_disabled) .selected(is_selected) .selected_icon(self.selected_icon) + .selected_icon_color(self.selected_icon_color) .size(self.icon_size) .color(self.icon_color) })) @@ -402,6 +419,7 @@ impl RenderOnce for Button { .disabled(is_disabled) .selected(is_selected) .selected_icon(self.selected_icon) + .selected_icon_color(self.selected_icon_color) .size(self.icon_size) .color(self.icon_color) })) diff --git a/crates/ui/src/components/button/button_icon.rs b/crates/ui/src/components/button/button_icon.rs index b8f5427d30aaa6dfb21b802bdd49922de9e17433..54ca0f79218eaa6ee1942c081a90566d49ab54e1 100644 --- a/crates/ui/src/components/button/button_icon.rs +++ b/crates/ui/src/components/button/button_icon.rs @@ -12,6 +12,7 @@ pub(super) struct ButtonIcon { disabled: bool, selected: bool, selected_icon: Option, + selected_icon_color: Option, selected_style: Option, } @@ -24,6 +25,7 @@ impl ButtonIcon { disabled: false, selected: false, selected_icon: None, + selected_icon_color: None, selected_style: None, } } @@ -48,6 +50,11 @@ impl ButtonIcon { self.selected_icon = icon.into(); self } + + pub fn selected_icon_color(mut self, color: impl Into>) -> Self { + self.selected_icon_color = color.into(); + self + } } impl Disableable for ButtonIcon { @@ -83,7 +90,7 @@ impl RenderOnce for ButtonIcon { } else if self.selected_style.is_some() && self.selected { self.selected_style.unwrap().into() } else if self.selected { - Color::Selected + self.selected_icon_color.unwrap_or(Color::Selected) } else { self.color };