oak-team/oak-gpui
3
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Adjust model selector popover design (#15056)

Browse Source 
This PR mostly refines the model selector popover design by formatting
the models names' and adjusting spacing/alignment in the list-related
items. The list component changes could've been made in a separate PR
but it was also very practical to do it here as I was already
in-context. Either way, I'm happy to separate if that's better!

One thing I couldn't necessarily figure out, though, is why the order
changed (e.g., Anthropic at last ). I wonder if that was because of the
separator logic somehow? I'd love guidance here—new to Rust!

| Before | After |
|--------|--------|
| <img width="228" alt="Screenshot 2024-07-23 at 21 02 33"
src="https://github.com/user-attachments/assets/3372c6c9-08dc-4d71-9265-26f015e2dbc2">
| <img width="228" alt="Screenshot 2024-07-23 at 21 01 45"
src="https://github.com/user-attachments/assets/624cc7db-a3d9-48e3-99d7-c29829501130">
|

---

Release Notes:

- N/A

---------

Co-authored-by: Marshall Bowers <elliott.codes@gmail.com>
Co-authored-by: Bennet Bo Fenner <bennet@zed.dev>
Co-authored-by: Antonio <antonio@zed.dev>
Co-authored-by: Antonio Scandurra <me@as-cii.com>
This commit is contained in:
Danilo Leal
2024-07-24 12:24:54 +02:00
committed by GitHub
co-authored by Marshall Bowers Bennet Bo Fenner Antonio Antonio Scandurra
parent 87d93033d1
commit 912b396e58
13 changed files with 220 additions and 298 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/assistant/src/assistant_panel.rs
+37 -4
View File
@@ -2630,10 +2630,43 @@ impl Render for ContextEditorToolbarItem {
});
let right_side = h_flex()
.gap_2()
.child(ModelSelector::new(
self.model_selector_menu_handle.clone(),
self.fs.clone(),
))
.child(
ModelSelector::new(
self.fs.clone(),
ButtonLike::new("active-model")
.style(ButtonStyle::Subtle)
.child(
h_flex()
.w_full()
.gap_0p5()
.child(
div()
.overflow_x_hidden()
.flex_grow()
.whitespace_nowrap()
.child(
Label::new(
LanguageModelCompletionProvider::read_global(cx)
.active_model()
.map(|model| model.name().0)
.unwrap_or_else(|| "No model selected".into()),
)
.size(LabelSize::Small)
.color(Color::Muted),
),
)
.child(
Icon::new(IconName::ChevronDown)
.color(Color::Muted)
.size(IconSize::XSmall),
),
)
.tooltip(move |cx| {
Tooltip::for_action("Change Model", &ToggleModelSelector, cx)
}),
)
.with_handle(self.model_selector_menu_handle.clone()),
)
.children(self.render_remaining_tokens(cx))
.child(self.render_inject_context_menu(cx));
crates/assistant/src/inline_assistant.rs
+27 -77
View File
@@ -1,6 +1,6 @@
use crate::{
assistant_settings::AssistantSettings, humanize_token_count, prompts::generate_content_prompt,
AssistantPanel, AssistantPanelEvent, Hunk, LanguageModelCompletionProvider, StreamingDiff,
humanize_token_count, prompts::generate_content_prompt, AssistantPanel, AssistantPanelEvent,
Hunk, LanguageModelCompletionProvider, ModelSelector, StreamingDiff,
};
use anyhow::{anyhow, Context as _, Result};
use client::telemetry::Telemetry;
@@ -27,13 +27,11 @@ use gpui::{
WindowContext,
};
use language::{Buffer, Point, Selection, TransactionId};
use language_model::{
LanguageModelRegistry, LanguageModelRequest, LanguageModelRequestMessage, Role,
};
use language_model::{LanguageModelRequest, LanguageModelRequestMessage, Role};
use multi_buffer::MultiBufferRow;
use parking_lot::Mutex;
use rope::Rope;
use settings::{update_settings_file, Settings};
use settings::Settings;
use similar::TextDiff;
use smol::future::FutureExt;
use std::{
@@ -47,7 +45,7 @@ use std::{
time::{Duration, Instant},
};
use theme::ThemeSettings;
use ui::{prelude::*, ContextMenu, IconButtonShape, PopoverMenu, Tooltip};
use ui::{prelude::*, IconButtonShape, Tooltip};
use util::RangeExt;
use workspace::{notifications::NotificationId, Toast, Workspace};
@@ -1325,8 +1323,6 @@ impl EventEmitter<PromptEditorEvent> for PromptEditor {}
impl Render for PromptEditor {
fn render(&mut self, cx: &mut ViewContext<Self>) -> impl IntoElement {
let gutter_dimensions = *self.gutter_dimensions.lock();
let fs = self.fs.clone();
let buttons = match &self.codegen.read(cx).status {
CodegenStatus::Idle => {
vec![
@@ -1427,74 +1423,27 @@ impl Render for PromptEditor {
.w(gutter_dimensions.full_width() + (gutter_dimensions.margin / 2.0))
.justify_center()
.gap_2()
.child(
PopoverMenu::new("model-switcher")
.menu(move |cx| {
ContextMenu::build(cx, |mut menu, cx| {
for available_model in
LanguageModelRegistry::read_global(cx).available_models(cx)
{
menu = menu.custom_entry(
{
let model_name = available_model.name().0.clone();
let provider =
available_model.provider_id().0.clone();
move |_| {
h_flex()
.w_full()
.justify_between()
.child(Label::new(model_name.clone()))
.child(
div().ml_4().child(
Label::new(provider.clone())
.color(Color::Muted),
),
)
.into_any()
}
},
{
let fs = fs.clone();
let model = available_model.clone();
move |cx| {
let model = model.clone();
update_settings_file::<AssistantSettings>(
fs.clone(),
cx,
move |settings, _| {
settings.set_model(model)
},
);
}
},
);
}
menu
})
.into()
})
.trigger(
IconButton::new("context", IconName::Settings)
.shape(IconButtonShape::Square)
.icon_size(IconSize::Small)
.icon_color(Color::Muted)
.tooltip(move |cx| {
Tooltip::with_meta(
format!(
"Using {}",
LanguageModelCompletionProvider::read_global(cx)
.active_model()
.map(|model| model.name().0)
.unwrap_or_else(|| "No model selected".into()),
),
None,
"Change Model",
cx,
)
}),
)
.anchor(gpui::AnchorCorner::BottomRight),
)
.child(ModelSelector::new(
self.fs.clone(),
IconButton::new("context", IconName::Settings)
.shape(IconButtonShape::Square)
.icon_size(IconSize::Small)
.icon_color(Color::Muted)
.tooltip(move |cx| {
Tooltip::with_meta(
format!(
"Using {}",
LanguageModelCompletionProvider::read_global(cx)
.active_model()
.map(|model| model.name().0)
.unwrap_or_else(|| "No model selected".into()),
),
None,
"Change Model",
cx,
)
}),
))
.children(
if let CodegenStatus::Error(error) = &self.codegen.read(cx).status {
let error_message = SharedString::from(error.to_string());
@@ -2625,6 +2574,7 @@ mod tests {
language_settings, tree_sitter_rust, Buffer, Language, LanguageConfig, LanguageMatcher,
Point,
};
use language_model::LanguageModelRegistry;
use rand::prelude::*;
use serde::Serialize;
use settings::SettingsStore;
crates/assistant/src/model_selector.rs
+110 -109
View File
@@ -1,127 +1,128 @@
use std::sync::Arc;
use crate::{
assistant_settings::AssistantSettings, LanguageModelCompletionProvider, ToggleModelSelector,
};
use crate::{assistant_settings::AssistantSettings, LanguageModelCompletionProvider};
use fs::Fs;
use language_model::LanguageModelRegistry;
use settings::update_settings_file;
use ui::{prelude::*, ButtonLike, ContextMenu, PopoverMenu, PopoverMenuHandle, Tooltip};
use ui::{prelude::*, ContextMenu, PopoverMenu, PopoverMenuHandle, PopoverTrigger};
#[derive(IntoElement)]
pub struct ModelSelector {
handle: PopoverMenuHandle<ContextMenu>,
pub struct ModelSelector<T: PopoverTrigger> {
handle: Option<PopoverMenuHandle<ContextMenu>>,
fs: Arc<dyn Fs>,
trigger: T,
}
impl ModelSelector {
pub fn new(handle: PopoverMenuHandle<ContextMenu>, fs: Arc<dyn Fs>) -> Self {
ModelSelector { handle, fs }
impl<T: PopoverTrigger> ModelSelector<T> {
pub fn new(fs: Arc<dyn Fs>, trigger: T) -> Self {
ModelSelector {
handle: None,
fs,
trigger,
}
}
pub fn with_handle(mut self, handle: PopoverMenuHandle<ContextMenu>) -> Self {
self.handle = Some(handle);
self
}
}
impl RenderOnce for ModelSelector {
fn render(self, cx: &mut WindowContext) -> impl IntoElement {
PopoverMenu::new("model-switcher")
.with_handle(self.handle)
.menu(move |cx| {
ContextMenu::build(cx, |mut menu, cx| {
for (provider, available_models) in LanguageModelRegistry::global(cx)
.read(cx)
.available_models_grouped_by_provider(cx)
{
menu = menu.header(provider.0.clone());
impl<T: PopoverTrigger> RenderOnce for ModelSelector<T> {
fn render(self, _: &mut WindowContext) -> impl IntoElement {
let mut menu = PopoverMenu::new("model-switcher");
if let Some(handle) = self.handle {
menu = menu.with_handle(handle);
}
if available_models.is_empty() {
menu = menu.custom_entry(
{
move |_| {
h_flex()
.w_full()
.gap_1()
.child(Icon::new(IconName::Settings))
.child(Label::new("Configure"))
.into_any()
}
},
{
let provider = provider.clone();
move |cx| {
LanguageModelCompletionProvider::global(cx).update(
cx,
|completion_provider, cx| {
completion_provider
.set_active_provider(provider.clone(), cx)
},
);
}
},
);
}
for available_model in available_models {
menu = menu.custom_entry(
{
let model_name = available_model.name().0.clone();
move |_| {
h_flex()
.w_full()
.child(Label::new(model_name.clone()))
.into_any()
}
},
{
let fs = self.fs.clone();
let model = available_model.clone();
move |cx| {
let model = model.clone();
update_settings_file::<AssistantSettings>(
fs.clone(),
cx,
move |settings, _| settings.set_model(model),
);
}
},
);
}
menu.menu(move |cx| {
ContextMenu::build(cx, |mut menu, cx| {
for (index, provider) in LanguageModelRegistry::global(cx)
.read(cx)
.providers()
.enumerate()
{
if index > 0 {
menu = menu.separator();
}
menu
})
.into()
})
.trigger(
ButtonLike::new("active-model")
.style(ButtonStyle::Subtle)
.child(
h_flex()
.w_full()
.gap_0p5()
.child(
div()
.overflow_x_hidden()
.flex_grow()
.whitespace_nowrap()
.child(
Label::new(
LanguageModelCompletionProvider::read_global(cx)
.active_model()
.map(|model| model.name().0)
.unwrap_or_else(|| "No model selected".into()),
menu = menu.header(provider.name().0);
let available_models = provider.provided_models(cx);
if available_models.is_empty() {
menu = menu.custom_entry(
{
move |_| {
h_flex()
.w_full()
.gap_1()
.child(Icon::new(IconName::Settings))
.child(Label::new("Configure"))
.into_any()
}
},
{
let provider = provider.id();
move |cx| {
LanguageModelCompletionProvider::global(cx).update(
cx,
|completion_provider, cx| {
completion_provider
.set_active_provider(provider.clone(), cx)
},
);
}
},
);
}
let selected_model = LanguageModelCompletionProvider::read_global(cx)
.active_model()
.map(|m| m.id());
let selected_provider = LanguageModelCompletionProvider::read_global(cx)
.active_provider()
.map(|m| m.id());
for available_model in available_models {
menu = menu.custom_entry(
{
let id = available_model.id();
let provider_id = available_model.provider_id();
let model_name = available_model.name().0.clone();
let selected_model = selected_model.clone();
let selected_provider = selected_provider.clone();
move |_| {
h_flex()
.w_full()
.justify_between()
.child(Label::new(model_name.clone()))
.when(
selected_model.as_ref() == Some(&id)
&& selected_provider.as_ref() == Some(&provider_id),
|this| this.child(Icon::new(IconName::Check)),
)
.size(LabelSize::Small)
.color(Color::Muted),
),
)
.child(
Icon::new(IconName::ChevronDown)
.color(Color::Muted)
.size(IconSize::XSmall),
),
)
.tooltip(move |cx| {
Tooltip::for_action("Change Model", &ToggleModelSelector, cx)
}),
)
.attach(gpui::AnchorCorner::BottomLeft)
.into_any()
}
},
{
let fs = self.fs.clone();
let model = available_model.clone();
move |cx| {
let model = model.clone();
update_settings_file::<AssistantSettings>(
fs.clone(),
cx,
move |settings, _| settings.set_model(model),
);
}
},
);
}
}
menu
})
.into()
})
.trigger(self.trigger)
.attach(gpui::AnchorCorner::BottomLeft)
}
}
crates/assistant/src/terminal_inline_assistant.rs
+26 -78
View File
@@ -1,7 +1,6 @@
use crate::{
assistant_settings::AssistantSettings, humanize_token_count,
prompts::generate_terminal_assistant_prompt, AssistantPanel, AssistantPanelEvent,
LanguageModelCompletionProvider,
humanize_token_count, prompts::generate_terminal_assistant_prompt, AssistantPanel,
AssistantPanelEvent, LanguageModelCompletionProvider, ModelSelector,
};
use anyhow::{Context as _, Result};
use client::telemetry::Telemetry;
@@ -17,10 +16,8 @@ use gpui::{
Subscription, Task, TextStyle, UpdateGlobal, View, WeakView,
};
use language::Buffer;
use language_model::{
LanguageModelRegistry, LanguageModelRequest, LanguageModelRequestMessage, Role,
};
use settings::{update_settings_file, Settings};
use language_model::{LanguageModelRequest, LanguageModelRequestMessage, Role};
use settings::Settings;
use std::{
cmp,
sync::Arc,
@@ -29,7 +26,7 @@ use std::{
use terminal::Terminal;
use terminal_view::TerminalView;
use theme::ThemeSettings;
use ui::{prelude::*, ContextMenu, IconButtonShape, PopoverMenu, Tooltip};
use ui::{prelude::*, IconButtonShape, Tooltip};
use util::ResultExt;
use workspace::{notifications::NotificationId, Toast, Workspace};
@@ -450,8 +447,6 @@ impl EventEmitter<PromptEditorEvent> for PromptEditor {}
impl Render for PromptEditor {
fn render(&mut self, cx: &mut ViewContext<Self>) -> impl IntoElement {
let fs = self.fs.clone();
let buttons = match &self.codegen.read(cx).status {
CodegenStatus::Idle => {
vec![
@@ -554,74 +549,27 @@ impl Render for PromptEditor {
.w_12()
.justify_center()
.gap_2()
.child(
PopoverMenu::new("model-switcher")
.menu(move |cx| {
ContextMenu::build(cx, |mut menu, cx| {
for available_model in
LanguageModelRegistry::read_global(cx).available_models(cx)
{
menu = menu.custom_entry(
{
let model_name = available_model.name().0.clone();
let provider =
available_model.provider_id().0.clone();
move |_| {
h_flex()
.w_full()
.justify_between()
.child(Label::new(model_name.clone()))
.child(
div().ml_4().child(
Label::new(provider.clone())
.color(Color::Muted),
),
)
.into_any()
}
},
{
let fs = fs.clone();
let model = available_model.clone();
move |cx| {
let model = model.clone();
update_settings_file::<AssistantSettings>(
fs.clone(),
cx,
move |settings, _| {
settings.set_model(model)
},
);
}
},
);
}
menu
})
.into()
})
.trigger(
IconButton::new("context", IconName::Settings)
.shape(IconButtonShape::Square)
.icon_size(IconSize::Small)
.icon_color(Color::Muted)
.tooltip(move |cx| {
Tooltip::with_meta(
format!(
"Using {}",
LanguageModelCompletionProvider::read_global(cx)
.active_model()
.map(|model| model.name().0)
.unwrap_or_else(|| "No model selected".into())
),
None,
"Change Model",
cx,
)
}),
)
.anchor(gpui::AnchorCorner::BottomRight),
)
.child(ModelSelector::new(
self.fs.clone(),
IconButton::new("context", IconName::Settings)
.shape(IconButtonShape::Square)
.icon_size(IconSize::Small)
.icon_color(Color::Muted)
.tooltip(move |cx| {
Tooltip::with_meta(
format!(
"Using {}",
LanguageModelCompletionProvider::read_global(cx)
.active_model()
.map(|model| model.name().0)
.unwrap_or_else(|| "No model selected".into()),
),
None,
"Change Model",
cx,
)
}),
))
.children(
if let CodegenStatus::Error(error) = &self.codegen.read(cx).status {
let error_message = SharedString::from(error.to_string());
crates/completion/src/completion.rs
+2 -2
View File
@@ -89,10 +89,10 @@ impl LanguageModelCompletionProvider {
pub fn set_active_provider(
&mut self,
provider_name: LanguageModelProviderId,
provider_id: LanguageModelProviderId,
cx: &mut ModelContext<Self>,
) {
self.active_provider = LanguageModelRegistry::read_global(cx).provider(&provider_name);
self.active_provider = LanguageModelRegistry::read_global(cx).provider(&provider_id);
self.active_model = None;
cx.notify();
}
crates/language_model/src/language_model.rs
+4 -4
View File
@@ -59,16 +59,16 @@ pub trait LanguageModelProviderState: 'static {
fn subscribe<T: 'static>(&self, cx: &mut gpui::ModelContext<T>) -> Option<gpui::Subscription>;
}
#[derive(Clone, Eq, PartialEq, Hash, Debug)]
#[derive(Clone, Eq, PartialEq, Hash, Debug, Ord, PartialOrd)]
pub struct LanguageModelId(pub SharedString);
#[derive(Clone, Eq, PartialEq, Hash, Debug)]
#[derive(Clone, Eq, PartialEq, Hash, Debug, Ord, PartialOrd)]
pub struct LanguageModelName(pub SharedString);
#[derive(Clone, Eq, PartialEq, Hash, Debug)]
#[derive(Clone, Eq, PartialEq, Hash, Debug, Ord, PartialOrd)]
pub struct LanguageModelProviderId(pub SharedString);
#[derive(Clone, Eq, PartialEq, Hash, Debug)]
#[derive(Clone, Eq, PartialEq, Hash, Debug, Ord, PartialOrd)]
pub struct LanguageModelProviderName(pub SharedString);
impl From<String> for LanguageModelId {
crates/language_model/src/provider/anthropic.rs
+2 -1
View File
@@ -1,5 +1,6 @@
use anthropic::{stream_completion, Request, RequestMessage};
use anyhow::{anyhow, Result};
use collections::BTreeMap;
use editor::{Editor, EditorElement, EditorStyle};
use futures::{future::BoxFuture, stream::BoxStream, FutureExt, StreamExt};
use gpui::{
@@ -8,7 +9,7 @@ use gpui::{
};
use http_client::HttpClient;
use settings::{Settings, SettingsStore};
use std::{collections::BTreeMap, sync::Arc, time::Duration};
use std::{sync::Arc, time::Duration};
use strum::IntoEnumIterator;
use theme::ThemeSettings;
use ui::prelude::*;
crates/language_model/src/provider/cloud.rs
+2 -1
View File
@@ -6,10 +6,11 @@ use crate::{
};
use anyhow::Result;
use client::Client;
use collections::BTreeMap;
use futures::{future::BoxFuture, stream::BoxStream, FutureExt, StreamExt, TryFutureExt};
use gpui::{AnyView, AppContext, AsyncAppContext, Subscription, Task};
use settings::{Settings, SettingsStore};
use std::{collections::BTreeMap, sync::Arc};
use std::sync::Arc;
use strum::IntoEnumIterator;
use ui::prelude::*;
crates/language_model/src/provider/ollama.rs
+2 -2
View File
@@ -75,12 +75,12 @@ impl OllamaLanguageModelProvider {
http_client,
available_models: Default::default(),
_subscription: cx.observe_global::<SettingsStore>(|this: &mut State, cx| {
this.fetch_models(cx).detach_and_log_err(cx);
this.fetch_models(cx).detach();
cx.notify();
}),
}),
};
this.fetch_models(cx).detach_and_log_err(cx);
this.fetch_models(cx).detach();
this
}
crates/language_model/src/registry.rs
+5 -17
View File
@@ -1,5 +1,5 @@
use client::Client;
use collections::HashMap;
use collections::BTreeMap;
use gpui::{AppContext, Global, Model, ModelContext};
use std::sync::Arc;
use ui::Context;
@@ -65,7 +65,7 @@ impl Global for GlobalLanguageModelRegistry {}
#[derive(Default)]
pub struct LanguageModelRegistry {
providers: HashMap<LanguageModelProviderId, Arc<dyn LanguageModelProvider>>,
providers: BTreeMap<LanguageModelProviderId, Arc<dyn LanguageModelProvider>>,
}
impl LanguageModelRegistry {
@@ -114,10 +114,8 @@ impl LanguageModelRegistry {
}
}
pub fn providers(
&self,
) -> impl Iterator<Item = (&LanguageModelProviderId, &Arc<dyn LanguageModelProvider>)> {
self.providers.iter()
pub fn providers(&self) -> impl Iterator<Item = &Arc<dyn LanguageModelProvider>> {
self.providers.values()
}
pub fn available_models(&self, cx: &AppContext) -> Vec<Arc<dyn LanguageModel>> {
@@ -127,16 +125,6 @@ impl LanguageModelRegistry {
.collect()
}
pub fn available_models_grouped_by_provider(
&self,
cx: &AppContext,
) -> HashMap<LanguageModelProviderId, Vec<Arc<dyn LanguageModel>>> {
self.providers
.iter()
.map(|(name, provider)| (name.clone(), provider.provided_models(cx)))
.collect()
}
pub fn provider(
&self,
name: &LanguageModelProviderId,
@@ -160,7 +148,7 @@ mod tests {
let providers = registry.read(cx).providers().collect::<Vec<_>>();
assert_eq!(providers.len(), 1);
assert_eq!(providers[0].0, &crate::provider::fake::provider_id());
assert_eq!(providers[0].id(), crate::provider::fake::provider_id());
registry.update(cx, |registry, cx| {
registry.unregister_provider(&crate::provider::fake::provider_id(), cx);
crates/ui/src/components/list/list_item.rs
+1 -1
View File
@@ -185,7 +185,7 @@ impl RenderOnce for ListItem {
.w_full()
.relative()
.gap_1()
.px_2()
.px_1p5()
.map(|this| match self.spacing {
ListItemSpacing::Dense => this,
ListItemSpacing::Sparse => this.py_1(),
crates/ui/src/components/list/list_separator.rs
+1 -1
View File
@@ -8,7 +8,7 @@ impl RenderOnce for ListSeparator {
div()
.h_px()
.w_full()
.my_1()
.my_1p5()
.bg(cx.theme().colors().border_variant)
}
}
crates/ui/src/components/list/list_sub_header.rs
+1 -1
View File
@@ -39,7 +39,7 @@ impl Selectable for ListSubHeader {
impl RenderOnce for ListSubHeader {
fn render(self, cx: &mut WindowContext) -> impl IntoElement {
h_flex().flex_1().w_full().relative().py_1().child(
h_flex().flex_1().w_full().relative().pb_1().px_0p5().child(
div()
.h_6()
.when(self.inset, |this| this.px_2())