From 22ad207bafa9f11b764858203095ce028cc806d2 Mon Sep 17 00:00:00 2001 From: Finn Evers Date: Wed, 7 May 2025 19:17:59 +0200 Subject: [PATCH] agent: Fix profile menu hover flicker after settings update (#30109) Closes #30091 Follow-up to #29958 This PR fixes the profile menu flickering due to the documentation aside after updating the agent dock position over the settings file. The problem arose because the `documentation_side` could get out of sync with the actual agent panel dock position. The `documentation_side` was only updated whenever the user changed the agent panel position using the UI, but not when updating the position in the settings file. You can reproduce this easily by changing the `agent.dock` position to the opposite site in your settings, which will make the profile menu flicker again in some scenarios due to the de-sync. This PR fixes this behavior by computing the position during render, thus the actual set panel position and the documentation position can never get out of sync Release Notes: - Fixed the agent profile menu flickering after updating the assistant panel dock position in the settings. --- crates/agent/src/assistant_panel.rs | 7 ------ crates/agent/src/message_editor.rs | 30 +++---------------------- crates/agent/src/profile_selector.rs | 33 +++++++++++++++------------- 3 files changed, 21 insertions(+), 49 deletions(-) diff --git a/crates/agent/src/assistant_panel.rs b/crates/agent/src/assistant_panel.rs index 53a1e9edd1fca33079729fc30e8bf92902d765fe..9752d4385e872b017f0ef6ab73657577165f1101 100644 --- a/crates/agent/src/assistant_panel.rs +++ b/crates/agent/src/assistant_panel.rs @@ -491,7 +491,6 @@ impl AssistantPanel { thread_store.downgrade(), context_store.downgrade(), thread.clone(), - agent_panel_dock_position(cx), window, cx, ) @@ -822,7 +821,6 @@ impl AssistantPanel { self.thread_store.downgrade(), self.context_store.downgrade(), thread, - agent_panel_dock_position(cx), window, cx, ) @@ -1031,7 +1029,6 @@ impl AssistantPanel { self.thread_store.downgrade(), self.context_store.downgrade(), thread, - agent_panel_dock_position(cx), window, cx, ) @@ -1360,10 +1357,6 @@ impl Panel for AssistantPanel { } fn set_position(&mut self, position: DockPosition, _: &mut Window, cx: &mut Context) { - self.message_editor.update(cx, |message_editor, cx| { - message_editor.set_dock_position(position, cx); - }); - settings::update_settings_file::( self.fs.clone(), cx, diff --git a/crates/agent/src/message_editor.rs b/crates/agent/src/message_editor.rs index 061ad6335cc717642278cf8d8714ecda9eb84072..f15ca504bacecaf695fe0a3976d6e6d22aa0c8c3 100644 --- a/crates/agent/src/message_editor.rs +++ b/crates/agent/src/message_editor.rs @@ -38,9 +38,8 @@ use proto::Plan; use settings::Settings; use std::time::Duration; use theme::ThemeSettings; -use ui::{Disclosure, DocumentationSide, KeyBinding, PopoverMenuHandle, Tooltip, prelude::*}; +use ui::{Disclosure, KeyBinding, PopoverMenuHandle, Tooltip, prelude::*}; use util::{ResultExt as _, maybe}; -use workspace::dock::DockPosition; use workspace::{CollaboratorId, Workspace}; use crate::context_picker::{ContextPicker, ContextPickerCompletionProvider, crease_for_mention}; @@ -133,14 +132,6 @@ pub(crate) fn create_editor( editor } -fn documentation_side(position: DockPosition) -> DocumentationSide { - match position { - DockPosition::Left => DocumentationSide::Right, - DockPosition::Bottom => DocumentationSide::Left, - DockPosition::Right => DocumentationSide::Left, - } -} - impl MessageEditor { pub fn new( fs: Arc, @@ -151,7 +142,6 @@ impl MessageEditor { thread_store: WeakEntity, text_thread_store: WeakEntity, thread: Entity, - dock_position: DockPosition, window: &mut Window, cx: &mut Context, ) -> Self { @@ -225,15 +215,8 @@ impl MessageEditor { model_selector, edits_expanded: false, editor_is_expanded: false, - profile_selector: cx.new(|cx| { - ProfileSelector::new( - fs, - thread_store, - editor.focus_handle(cx), - documentation_side(dock_position), - cx, - ) - }), + profile_selector: cx + .new(|cx| ProfileSelector::new(fs, thread_store, editor.focus_handle(cx), cx)), last_estimated_token_count: None, update_token_count_task: None, _subscriptions: subscriptions, @@ -1283,12 +1266,6 @@ impl MessageEditor { .ok(); })); } - - pub fn set_dock_position(&mut self, position: DockPosition, cx: &mut Context) { - self.profile_selector.update(cx, |profile_selector, cx| { - profile_selector.set_documentation_side(documentation_side(position), cx) - }); - } } pub fn extract_message_creases( @@ -1462,7 +1439,6 @@ impl AgentPreview for MessageEditor { thread_store.downgrade(), text_thread_store.downgrade(), thread, - DockPosition::Left, window, cx, ) diff --git a/crates/agent/src/profile_selector.rs b/crates/agent/src/profile_selector.rs index 38b878aea06dc2ded2bc08a70881646c8ed0078b..51eb20934488a3463ce3e113c747757bc4f7dfa6 100644 --- a/crates/agent/src/profile_selector.rs +++ b/crates/agent/src/profile_selector.rs @@ -1,7 +1,8 @@ use std::sync::Arc; use assistant_settings::{ - AgentProfile, AgentProfileId, AssistantSettings, GroupedAgentProfiles, builtin_profiles, + AgentProfile, AgentProfileId, AssistantDockPosition, AssistantSettings, GroupedAgentProfiles, + builtin_profiles, }; use fs::Fs; use gpui::{Action, Entity, FocusHandle, Subscription, WeakEntity, prelude::*}; @@ -22,7 +23,6 @@ pub struct ProfileSelector { menu_handle: PopoverMenuHandle, focus_handle: FocusHandle, _subscriptions: Vec, - documentation_side: DocumentationSide, } impl ProfileSelector { @@ -30,7 +30,6 @@ impl ProfileSelector { fs: Arc, thread_store: WeakEntity, focus_handle: FocusHandle, - documentation_side: DocumentationSide, cx: &mut Context, ) -> Self { let settings_subscription = cx.observe_global::(move |this, cx| { @@ -44,15 +43,9 @@ impl ProfileSelector { menu_handle: PopoverMenuHandle::default(), focus_handle, _subscriptions: vec![settings_subscription], - documentation_side, } } - pub fn set_documentation_side(&mut self, side: DocumentationSide, cx: &mut Context) { - self.documentation_side = side; - cx.notify(); - } - pub fn menu_handle(&self) -> PopoverMenuHandle { self.menu_handle.clone() } @@ -112,7 +105,7 @@ impl ProfileSelector { .toggleable(IconPosition::End, profile_id == settings.default_profile); let entry = if let Some(doc_text) = documentation { - entry.documentation_aside(self.documentation_side, move |_| { + entry.documentation_aside(documentation_side(settings.dock), move |_| { Label::new(doc_text).into_any_element() }) } else { @@ -180,11 +173,13 @@ impl Render for ProfileSelector { ) } }) - .anchor(if self.documentation_side == DocumentationSide::Left { - gpui::Corner::BottomRight - } else { - gpui::Corner::BottomLeft - }) + .anchor( + if documentation_side(settings.dock) == DocumentationSide::Left { + gpui::Corner::BottomRight + } else { + gpui::Corner::BottomLeft + }, + ) .with_handle(self.menu_handle.clone()) .menu(move |window, cx| { Some(this.update(cx, |this, cx| this.build_context_menu(window, cx))) @@ -200,3 +195,11 @@ impl Render for ProfileSelector { } } } + +fn documentation_side(position: AssistantDockPosition) -> DocumentationSide { + match position { + AssistantDockPosition::Left => DocumentationSide::Right, + AssistantDockPosition::Bottom => DocumentationSide::Left, + AssistantDockPosition::Right => DocumentationSide::Left, + } +}