agent: Fix profile menu hover flicker after settings update (#30109)

Finn Evers created 7 months ago

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.

Change summary

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(-)

Detailed changes

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>) {
-        self.message_editor.update(cx, |message_editor, cx| {
-            message_editor.set_dock_position(position, cx);
-        });
-
         settings::update_settings_file::<AssistantSettings>(
             self.fs.clone(),
             cx,

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<dyn Fs>,
@@ -151,7 +142,6 @@ impl MessageEditor {
         thread_store: WeakEntity<ThreadStore>,
         text_thread_store: WeakEntity<TextThreadStore>,
         thread: Entity<Thread>,
-        dock_position: DockPosition,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> 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>) {
-        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,
                 )

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<ContextMenu>,
     focus_handle: FocusHandle,
     _subscriptions: Vec<Subscription>,
-    documentation_side: DocumentationSide,
 }
 
 impl ProfileSelector {
@@ -30,7 +30,6 @@ impl ProfileSelector {
         fs: Arc<dyn Fs>,
         thread_store: WeakEntity<ThreadStore>,
         focus_handle: FocusHandle,
-        documentation_side: DocumentationSide,
         cx: &mut Context<Self>,
     ) -> Self {
         let settings_subscription = cx.observe_global::<SettingsStore>(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>) {
-        self.documentation_side = side;
-        cx.notify();
-    }
-
     pub fn menu_handle(&self) -> PopoverMenuHandle<ContextMenu> {
         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,
+    }
+}