From 2627a5fdbe2a0373f0585db38bbce71e9e773673 Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Tue, 18 Feb 2025 21:01:09 -0300 Subject: [PATCH] assistant: Improve the language model selector (#25125) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR includes change such as: - Ensures the popover width is fixed/not dancing around - Ensures the popover is not obscuring the trigger in the buffer and terminal inline assistant scenarios - Removes ellipsis from the trigger button label - Ensures the scrollbar doesn't hide the check icon | Terminal | Prompt Editor | Buffer | |--------|--------|--------| | ![Screenshot 2025-02-18 at 8 43 46 PM](https://github.com/user-attachments/assets/9cdfbaf1-f27e-4f48-877e-9cf61767ecee) | ![Screenshot 2025-02-18 at 8 43 49 PM](https://github.com/user-attachments/assets/7abf9be2-bd2a-43d7-9a5d-d665e7e9fda3) | ![Screenshot 2025-02-18 at 8 43 52 PM](https://github.com/user-attachments/assets/017bbdb3-185a-4bf6-9005-018ecafef9dd) | Release Notes: - N/A --- crates/assistant/src/inline_assistant.rs | 1 + .../src/terminal_inline_assistant.rs | 1 + .../src/assistant_model_selector.rs | 11 +++----- .../src/context_editor.rs | 11 +++----- .../src/language_model_selector.rs | 26 ++++++++++++++----- 5 files changed, 30 insertions(+), 20 deletions(-) diff --git a/crates/assistant/src/inline_assistant.rs b/crates/assistant/src/inline_assistant.rs index 01238d51ef..4d018b0910 100644 --- a/crates/assistant/src/inline_assistant.rs +++ b/crates/assistant/src/inline_assistant.rs @@ -1610,6 +1610,7 @@ impl Render for PromptEditor { cx, ) }, + gpui::Corner::TopRight, )) .map(|el| { let CodegenStatus::Error(error) = self.codegen.read(cx).status(cx) else { diff --git a/crates/assistant/src/terminal_inline_assistant.rs b/crates/assistant/src/terminal_inline_assistant.rs index f2a07e074f..7091b50cad 100644 --- a/crates/assistant/src/terminal_inline_assistant.rs +++ b/crates/assistant/src/terminal_inline_assistant.rs @@ -662,6 +662,7 @@ impl Render for PromptEditor { cx, ) }, + gpui::Corner::TopRight, )) .children( if let CodegenStatus::Error(error) = &self.codegen.read(cx).status { diff --git a/crates/assistant2/src/assistant_model_selector.rs b/crates/assistant2/src/assistant_model_selector.rs index 0308757e59..2769a9e5ca 100644 --- a/crates/assistant2/src/assistant_model_selector.rs +++ b/crates/assistant2/src/assistant_model_selector.rs @@ -61,13 +61,9 @@ impl Render for AssistantModelSelector { h_flex() .gap_0p5() .child( - div().max_w_32().child( - Label::new(model_name) - .size(LabelSize::Small) - .color(Color::Muted) - .text_ellipsis() - .into_any_element(), - ), + Label::new(model_name) + .size(LabelSize::Small) + .color(Color::Muted), ) .child( Icon::new(IconName::ChevronDown) @@ -84,6 +80,7 @@ impl Render for AssistantModelSelector { cx, ) }, + gpui::Corner::BottomRight, ) .with_handle(self.menu_handle.clone()) } diff --git a/crates/assistant_context_editor/src/context_editor.rs b/crates/assistant_context_editor/src/context_editor.rs index 31202f236b..f42be0fe04 100644 --- a/crates/assistant_context_editor/src/context_editor.rs +++ b/crates/assistant_context_editor/src/context_editor.rs @@ -2404,13 +2404,9 @@ impl ContextEditor { h_flex() .gap_0p5() .child( - div().max_w_32().child( - Label::new(model_name) - .size(LabelSize::Small) - .color(Color::Muted) - .text_ellipsis() - .into_any_element(), - ), + Label::new(model_name) + .size(LabelSize::Small) + .color(Color::Muted), ) .child( Icon::new(IconName::ChevronDown) @@ -2427,6 +2423,7 @@ impl ContextEditor { cx, ) }, + gpui::Corner::BottomLeft, ) .with_handle(self.language_model_selector_menu_handle.clone()) } diff --git a/crates/language_model_selector/src/language_model_selector.rs b/crates/language_model_selector/src/language_model_selector.rs index b14459c7bc..5a7641db45 100644 --- a/crates/language_model_selector/src/language_model_selector.rs +++ b/crates/language_model_selector/src/language_model_selector.rs @@ -2,8 +2,8 @@ use std::sync::Arc; use feature_flags::ZedPro; use gpui::{ - Action, AnyElement, AnyView, App, DismissEvent, Entity, EventEmitter, FocusHandle, Focusable, - Subscription, Task, WeakEntity, + Action, AnyElement, AnyView, App, Corner, DismissEvent, Entity, EventEmitter, FocusHandle, + Focusable, Subscription, Task, WeakEntity, }; use language_model::{LanguageModel, LanguageModelAvailability, LanguageModelRegistry}; use picker::{Picker, PickerDelegate}; @@ -37,12 +37,13 @@ impl LanguageModelSelector { on_model_changed: on_model_changed.clone(), all_models: all_models.clone(), filtered_models: all_models, - selected_index: 0, + selected_index: Self::get_active_model_index(cx), }; let picker = cx.new(|cx| { Picker::uniform_list(delegate, window, cx) .show_scrollbar(true) + .width(rems(20.)) .max_height(Some(rems(20.).into())) }); @@ -100,6 +101,16 @@ impl LanguageModelSelector { }) .collect::>() } + + fn get_active_model_index(cx: &App) -> usize { + let active_model = LanguageModelRegistry::read_global(cx).active_model(); + Self::all_models(cx) + .iter() + .position(|model_info| { + Some(model_info.model.id()) == active_model.as_ref().map(|model| model.id()) + }) + .unwrap_or(0) + } } impl EventEmitter for LanguageModelSelector {} @@ -126,6 +137,7 @@ where trigger: T, tooltip: TT, handle: Option>, + anchor: Corner, } impl LanguageModelSelectorPopoverMenu @@ -137,12 +149,14 @@ where language_model_selector: Entity, trigger: T, tooltip: TT, + anchor: Corner, ) -> Self { Self { language_model_selector, trigger, tooltip, handle: None, + anchor, } } @@ -163,7 +177,7 @@ where PopoverMenu::new("model-switcher") .menu(move |_window, _cx| Some(language_model_selector.clone())) .trigger_with_tooltip(self.trigger, self.tooltip) - .anchor(gpui::Corner::BottomRight) + .anchor(self.anchor) .when_some(self.handle.clone(), |menu, handle| menu.with_handle(handle)) .offset(gpui::Point { x: px(0.0), @@ -360,7 +374,7 @@ impl PickerDelegate for LanguageModelPickerDelegate { .items_center() .gap_1p5() .pl_0p5() - .min_w(px(240.)) + .w(px(240.)) .child( div().max_w_40().child( Label::new(model_info.model.name().0.clone()).text_ellipsis(), @@ -387,7 +401,7 @@ impl PickerDelegate for LanguageModelPickerDelegate { }), ), ) - .end_slot(div().when(is_selected, |this| { + .end_slot(div().pr_3().when(is_selected, |this| { this.child( Icon::new(IconName::Check) .color(Color::Accent)