Fix focus handle leak (#26090)

This fixes a major performance issue in the current git beta.
This PR also removes the PopoverButton component, which was easy to
misuse.

Release Notes:

- Git Beta: Fix frame drops caused by opening the git panel

---------

Co-authored-by: Mikayla Maki <mikayla.c.maki@gmail.com>

crates/assistant/src/inline_assistant.rs

@@ -35,7 +35,7 @@ use language_model::{
     report_assistant_event, LanguageModel, LanguageModelRegistry, LanguageModelRequest,
     LanguageModelRequestMessage, LanguageModelTextStream, Role,
 };
-use language_model_selector::{InlineLanguageModelSelector, LanguageModelSelector};
+use language_model_selector::inline_language_model_selector;
 use multi_buffer::MultiBufferRow;
 use parking_lot::Mutex;
 use project::{CodeAction, ProjectTransaction};
@@ -1425,7 +1425,6 @@ enum PromptEditorEvent {
 struct PromptEditor {
     id: InlineAssistId,
     editor: Entity<Editor>,
-    language_model_selector: Entity<LanguageModelSelector>,
     edited_since_done: bool,
     gutter_dimensions: Arc<Mutex<GutterDimensions>>,
     prompt_history: VecDeque<String>,
@@ -1439,6 +1438,7 @@ struct PromptEditor {
     _token_count_subscriptions: Vec<Subscription>,
     workspace: Option<WeakEntity<Workspace>>,
     show_rate_limit_notice: bool,
+    fs: Arc<dyn Fs>,
 }
 
 #[derive(Copy, Clone)]
@@ -1567,6 +1567,7 @@ impl Render for PromptEditor {
                 ]
             }
         });
+        let fs_clone = self.fs.clone();
 
         h_flex()
             .key_context("PromptEditor")
@@ -1589,10 +1590,13 @@ impl Render for PromptEditor {
                     .w(gutter_dimensions.full_width() + (gutter_dimensions.margin / 2.0))
                     .justify_center()
                     .gap_2()
-                    .child(
-                        InlineLanguageModelSelector::new(self.language_model_selector.clone())
-                            .render(window, cx),
-                    )
+                    .child(inline_language_model_selector(move |model, cx| {
+                        update_settings_file::<AssistantSettings>(
+                            fs_clone.clone(),
+                            cx,
+                            move |settings, _| settings.set_model(model.clone()),
+                        );
+                    }))
                     .map(|el| {
                         let CodegenStatus::Error(error) = self.codegen.read(cx).status(cx) else {
                             return el;
@@ -1705,21 +1709,8 @@ impl PromptEditor {
 
         let mut this = Self {
             id,
+            fs,
             editor: prompt_editor,
-            language_model_selector: cx.new(|cx| {
-                let fs = fs.clone();
-                LanguageModelSelector::new(
-                    move |model, cx| {
-                        update_settings_file::<AssistantSettings>(
-                            fs.clone(),
-                            cx,
-                            move |settings, _| settings.set_model(model.clone()),
-                        );
-                    },
-                    window,
-                    cx,
-                )
-            }),
             edited_since_done: false,
             gutter_dimensions,
             prompt_history,

crates/assistant/src/terminal_inline_assistant.rs

@@ -19,7 +19,7 @@ use language_model::{
     report_assistant_event, LanguageModelRegistry, LanguageModelRequest,
     LanguageModelRequestMessage, Role,
 };
-use language_model_selector::{InlineLanguageModelSelector, LanguageModelSelector};
+use language_model_selector::inline_language_model_selector;
 use prompt_store::PromptBuilder;
 use settings::{update_settings_file, Settings};
 use std::{
@@ -487,9 +487,9 @@ enum PromptEditorEvent {
 
 struct PromptEditor {
     id: TerminalInlineAssistId,
+    fs: Arc<dyn Fs>,
     height_in_lines: u8,
     editor: Entity<Editor>,
-    language_model_selector: Entity<LanguageModelSelector>,
     edited_since_done: bool,
     prompt_history: VecDeque<String>,
     prompt_history_ix: Option<usize>,
@@ -506,7 +506,7 @@ struct PromptEditor {
 impl EventEmitter<PromptEditorEvent> for PromptEditor {}
 
 impl Render for PromptEditor {
-    fn render(&mut self, window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
+    fn render(&mut self, _window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
         let status = &self.codegen.read(cx).status;
         let buttons = match status {
             CodegenStatus::Idle => {
@@ -624,6 +624,8 @@ impl Render for PromptEditor {
             }
         };
 
+        let fs_clone = self.fs.clone();
+
         h_flex()
             .bg(cx.theme().colors().editor_background)
             .border_y_1()
@@ -641,10 +643,13 @@ impl Render for PromptEditor {
                     .w_12()
                     .justify_center()
                     .gap_2()
-                    .child(
-                        InlineLanguageModelSelector::new(self.language_model_selector.clone())
-                            .render(window, cx),
-                    )
+                    .child(inline_language_model_selector(move |model, cx| {
+                        update_settings_file::<AssistantSettings>(
+                            fs_clone.clone(),
+                            cx,
+                            move |settings, _| settings.set_model(model.clone()),
+                        );
+                    }))
                     .children(
                         if let CodegenStatus::Error(error) = &self.codegen.read(cx).status {
                             let error_message = SharedString::from(error.to_string());
@@ -722,22 +727,9 @@ impl PromptEditor {
 
         let mut this = Self {
             id,
+            fs,
             height_in_lines: 1,
             editor: prompt_editor,
-            language_model_selector: cx.new(|cx| {
-                let fs = fs.clone();
-                LanguageModelSelector::new(
-                    move |model, cx| {
-                        update_settings_file::<AssistantSettings>(
-                            fs.clone(),
-                            cx,
-                            move |settings, _| settings.set_model(model.clone()),
-                        );
-                    },
-                    window,
-                    cx,
-                )
-            }),
             edited_since_done: false,
             prompt_history,
             prompt_history_ix: None,

crates/assistant2/src/assistant_model_selector.rs

@@ -1,46 +1,50 @@
 use assistant_settings::AssistantSettings;
 use fs::Fs;
-use gpui::{Entity, FocusHandle};
-use language_model_selector::{AssistantLanguageModelSelector, LanguageModelSelector};
+use gpui::FocusHandle;
+use language_model_selector::{assistant_language_model_selector, LanguageModelSelector};
 use settings::update_settings_file;
 use std::sync::Arc;
-use ui::prelude::*;
+use ui::{prelude::*, PopoverMenuHandle};
 
 pub struct AssistantModelSelector {
-    pub selector: Entity<LanguageModelSelector>,
+    menu_handle: PopoverMenuHandle<LanguageModelSelector>,
     focus_handle: FocusHandle,
+    fs: Arc<dyn Fs>,
 }
 
 impl AssistantModelSelector {
     pub(crate) fn new(
         fs: Arc<dyn Fs>,
         focus_handle: FocusHandle,
-        window: &mut Window,
-        cx: &mut App,
+        _window: &mut Window,
+        _cx: &mut App,
     ) -> Self {
         Self {
-            selector: cx.new(|cx| {
-                let fs = fs.clone();
-                LanguageModelSelector::new(
-                    move |model, cx| {
-                        update_settings_file::<AssistantSettings>(
-                            fs.clone(),
-                            cx,
-                            move |settings, _cx| settings.set_model(model.clone()),
-                        );
-                    },
-                    window,
-                    cx,
-                )
-            }),
+            fs,
             focus_handle,
+            menu_handle: PopoverMenuHandle::default(),
         }
     }
+
+    pub fn toggle(&self, window: &mut Window, cx: &mut Context<Self>) {
+        self.menu_handle.toggle(window, cx);
+    }
 }
 
 impl Render for AssistantModelSelector {
-    fn render(&mut self, window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
-        AssistantLanguageModelSelector::new(self.focus_handle.clone(), self.selector.clone())
-            .render(window, cx)
+    fn render(&mut self, _: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
+        let fs_clone = self.fs.clone();
+        assistant_language_model_selector(
+            self.focus_handle.clone(),
+            Some(self.menu_handle.clone()),
+            cx,
+            move |model, cx| {
+                update_settings_file::<AssistantSettings>(
+                    fs_clone.clone(),
+                    cx,
+                    move |settings, _| settings.set_model(model.clone()),
+                );
+            },
+        )
     }
 }

crates/assistant2/src/inline_prompt_editor.rs

@@ -20,6 +20,7 @@ use gpui::{
     EventEmitter, FocusHandle, Focusable, FontWeight, Subscription, TextStyle, WeakEntity, Window,
 };
 use language_model::{LanguageModel, LanguageModelRegistry};
+use language_model_selector::ToggleModelSelector;
 use parking_lot::Mutex;
 use settings::Settings;
 use std::cmp;
@@ -102,11 +103,9 @@ impl<T: 'static> Render for PromptEditor<T> {
                     .items_start()
                     .cursor(CursorStyle::Arrow)
                     .on_action(cx.listener(Self::toggle_context_picker))
-                    .on_action(cx.listener(|this, action, window, cx| {
-                        let selector = this.model_selector.read(cx).selector.clone();
-                        selector.update(cx, |selector, cx| {
-                            selector.toggle_model_selector(action, window, cx);
-                        })
+                    .on_action(cx.listener(|this, _: &ToggleModelSelector, window, cx| {
+                        this.model_selector
+                            .update(cx, |model_selector, cx| model_selector.toggle(window, cx));
                     }))
                     .on_action(cx.listener(Self::confirm))
                     .on_action(cx.listener(Self::cancel))

crates/assistant2/src/message_editor.rs

@@ -8,6 +8,7 @@ use gpui::{
     TextStyle, WeakEntity,
 };
 use language_model::LanguageModelRegistry;
+use language_model_selector::ToggleModelSelector;
 use rope::Point;
 use settings::Settings;
 use std::time::Duration;
@@ -297,11 +298,9 @@ impl Render for MessageEditor {
         v_flex()
             .key_context("MessageEditor")
             .on_action(cx.listener(Self::chat))
-            .on_action(cx.listener(|this, action, window, cx| {
-                let selector = this.model_selector.read(cx).selector.clone();
-                selector.update(cx, |this, cx| {
-                    this.toggle_model_selector(action, window, cx);
-                })
+            .on_action(cx.listener(|this, _: &ToggleModelSelector, window, cx| {
+                this.model_selector
+                    .update(cx, |model_selector, cx| model_selector.toggle(window, cx));
             }))
             .on_action(cx.listener(Self::toggle_context_picker))
             .on_action(cx.listener(Self::remove_all_context))

crates/assistant_context_editor/src/context_editor.rs

@@ -37,7 +37,9 @@ use language_model::{
     LanguageModelImage, LanguageModelProvider, LanguageModelProviderTosView, LanguageModelRegistry,
     Role,
 };
-use language_model_selector::{AssistantLanguageModelSelector, LanguageModelSelector};
+use language_model_selector::{
+    assistant_language_model_selector, LanguageModelSelector, ToggleModelSelector,
+};
 use multi_buffer::MultiBufferRow;
 use picker::Picker;
 use project::lsp_store::LocalLspAdapterDelegate;
@@ -195,7 +197,7 @@ pub struct ContextEditor {
     // the file is opened. In order to keep the worktree alive for the duration of the
     // context editor, we keep a reference here.
     dragged_file_worktrees: Vec<Entity<Worktree>>,
-    language_model_selector: Entity<LanguageModelSelector>,
+    language_model_selector: PopoverMenuHandle<LanguageModelSelector>,
 }
 
 pub const DEFAULT_TAB_TITLE: &str = "New Chat";
@@ -249,21 +251,6 @@ impl ContextEditor {
             cx.observe_global_in::<SettingsStore>(window, Self::settings_changed),
         ];
 
-        let fs_clone = fs.clone();
-        let language_model_selector = cx.new(|cx| {
-            LanguageModelSelector::new(
-                move |model, cx| {
-                    update_settings_file::<AssistantSettings>(
-                        fs_clone.clone(),
-                        cx,
-                        move |settings, _| settings.set_model(model.clone()),
-                    );
-                },
-                window,
-                cx,
-            )
-        });
-
         let sections = context.read(cx).slash_command_output_sections().to_vec();
         let patch_ranges = context.read(cx).patch_ranges().collect::<Vec<_>>();
         let slash_commands = context.read(cx).slash_commands().clone();
@@ -288,7 +275,7 @@ impl ContextEditor {
             show_accept_terms: false,
             slash_menu_handle: Default::default(),
             dragged_file_worktrees: Vec::new(),
-            language_model_selector,
+            language_model_selector: PopoverMenuHandle::default(),
         };
         this.update_message_headers(cx);
         this.update_image_blocks(cx);
@@ -2831,6 +2818,7 @@ impl Render for ContextEditor {
         } else {
             None
         };
+        let fs_clone = self.fs.clone();
 
         let language_model_selector = self.language_model_selector.clone();
         v_flex()
@@ -2845,10 +2833,8 @@ impl Render for ContextEditor {
             .on_action(cx.listener(ContextEditor::edit))
             .on_action(cx.listener(ContextEditor::assist))
             .on_action(cx.listener(ContextEditor::split))
-            .on_action(move |action, window, cx| {
-                language_model_selector.update(cx, |this, cx| {
-                    this.toggle_model_selector(action, window, cx);
-                })
+            .on_action(move |_: &ToggleModelSelector, window, cx| {
+                language_model_selector.toggle(window, cx);
             })
             .size_full()
             .children(self.render_notice(cx))
@@ -2887,14 +2873,18 @@ impl Render for ContextEditor {
                                 .gap_1()
                                 .child(self.render_inject_context_menu(cx))
                                 .child(ui::Divider::vertical())
-                                .child(div().pl_0p5().child({
-                                    let focus_handle = self.editor().focus_handle(cx).clone();
-                                    AssistantLanguageModelSelector::new(
-                                        focus_handle,
-                                        self.language_model_selector.clone(),
-                                    )
-                                    .render(window, cx)
-                                })),
+                                .child(div().pl_0p5().child(assistant_language_model_selector(
+                                    self.editor().focus_handle(cx),
+                                    Some(self.language_model_selector.clone()),
+                                    cx,
+                                    move |model, cx| {
+                                        update_settings_file::<AssistantSettings>(
+                                            fs_clone.clone(),
+                                            cx,
+                                            move |settings, _| settings.set_model(model.clone()),
+                                        );
+                                    },
+                                ))),
                         )
                         .child(
                             h_flex()

crates/git_ui/src/branch_picker.rs

@@ -1,18 +1,16 @@
-use anyhow::{Context as _, Result};
+use anyhow::Context as _;
 use fuzzy::{StringMatch, StringMatchCandidate};
 
 use git::repository::Branch;
 use gpui::{
-    rems, App, AsyncApp, Context, DismissEvent, Entity, EventEmitter, FocusHandle, Focusable,
+    rems, App, Context, DismissEvent, Entity, EventEmitter, FocusHandle, Focusable,
     InteractiveElement, IntoElement, ParentElement, Render, SharedString, Styled, Subscription,
     Task, Window,
 };
 use picker::{Picker, PickerDelegate};
 use project::{Project, ProjectPath};
 use std::sync::Arc;
-use ui::{
-    prelude::*, HighlightedLabel, ListItem, ListItemSpacing, PopoverMenuHandle, TriggerablePopover,
-};
+use ui::{prelude::*, HighlightedLabel, ListItem, ListItemSpacing, PopoverMenuHandle};
 use util::ResultExt;
 use workspace::notifications::DetachAndPromptErr;
 use workspace::{ModalView, Workspace};
@@ -31,35 +29,16 @@ pub fn open(
     cx: &mut Context<Workspace>,
 ) {
     let project = workspace.project().clone();
-    let this = cx.entity();
     let style = BranchListStyle::Modal;
-    cx.spawn_in(window, |_, mut cx| async move {
-        // Modal branch picker has a longer trailoff than a popover one.
-        let delegate = BranchListDelegate::new(project.clone(), style, 70, &cx).await?;
-
-        this.update_in(&mut cx, move |workspace, window, cx| {
     workspace.toggle_modal(window, cx, |window, cx| {
-                let picker = cx.new(|cx| Picker::uniform_list(delegate, window, cx));
-                let _subscription = cx.subscribe(&picker, |_, _, _, cx| {
-                    cx.emit(DismissEvent);
-                });
-
-                let mut list = BranchList::new(project, style, 34., cx);
-                list._subscription = Some(_subscription);
-                list.picker = Some(picker);
-                list
+        BranchList::new(project, style, 34., window, cx)
     })
-        })?;
-
-        Ok(())
-    })
-    .detach_and_prompt_err("Failed to read branches", window, cx, |_, _, _| None)
 }
 
 pub fn popover(project: Entity<Project>, window: &mut Window, cx: &mut App) -> Entity<BranchList> {
     cx.new(|cx| {
-        let mut list = BranchList::new(project, BranchListStyle::Popover, 15., cx);
-        list.reload_branches(window, cx);
+        let list = BranchList::new(project, BranchListStyle::Popover, 15., window, cx);
+        list.focus_handle(cx).focus(window);
         list
     })
 }
@@ -72,59 +51,54 @@ enum BranchListStyle {
 
 pub struct BranchList {
     rem_width: f32,
-    popover_handle: PopoverMenuHandle<Self>,
-    default_focus_handle: FocusHandle,
-    project: Entity<Project>,
-    style: BranchListStyle,
-    pub picker: Option<Entity<Picker<BranchListDelegate>>>,
-    _subscription: Option<Subscription>,
-}
-
-impl TriggerablePopover for BranchList {
-    fn menu_handle(
-        &mut self,
-        _window: &mut Window,
-        _cx: &mut gpui::Context<Self>,
-    ) -> PopoverMenuHandle<Self> {
-        self.popover_handle.clone()
-    }
+    pub popover_handle: PopoverMenuHandle<Self>,
+    pub picker: Entity<Picker<BranchListDelegate>>,
+    _subscription: Subscription,
 }
 
 impl BranchList {
-    fn new(project: Entity<Project>, style: BranchListStyle, rem_width: f32, cx: &mut App) -> Self {
+    fn new(
+        project_handle: Entity<Project>,
+        style: BranchListStyle,
+        rem_width: f32,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+    ) -> Self {
         let popover_handle = PopoverMenuHandle::default();
-        Self {
-            project,
-            picker: None,
-            rem_width,
-            popover_handle,
-            default_focus_handle: cx.focus_handle(),
-            style,
-            _subscription: None,
-        }
-    }
+        let project = project_handle.read(cx);
+        let all_branches_request = project
+            .visible_worktrees(cx)
+            .next()
+            .map(|worktree| project.branches(ProjectPath::root_path(worktree.read(cx).id()), cx))
+            .context("No worktrees found");
 
-    fn reload_branches(&mut self, window: &mut Window, cx: &mut Context<Self>) {
-        let project = self.project.clone();
-        let style = self.style;
         cx.spawn_in(window, |this, mut cx| async move {
-            let delegate = BranchListDelegate::new(project, style, 20, &cx).await?;
-            let picker =
-                cx.new_window_entity(|window, cx| Picker::uniform_list(delegate, window, cx))?;
+            let all_branches = all_branches_request?.await?;
 
-            this.update(&mut cx, |branch_list, cx| {
-                let subscription =
-                    cx.subscribe(&picker, |_, _, _: &DismissEvent, cx| cx.emit(DismissEvent));
-
-                branch_list.picker = Some(picker);
-                branch_list._subscription = Some(subscription);
-
-                cx.notify();
+            this.update_in(&mut cx, |this, window, cx| {
+                this.picker.update(cx, |picker, cx| {
+                    picker.delegate.all_branches = Some(all_branches);
+                    picker.refresh(window, cx);
+                })
             })?;
 
             anyhow::Ok(())
         })
         .detach_and_log_err(cx);
+
+        let delegate = BranchListDelegate::new(project_handle.clone(), style, 20);
+        let picker = cx.new(|cx| Picker::uniform_list(delegate, window, cx));
+
+        let _subscription = cx.subscribe(&picker, |_, _, _, cx| {
+            cx.emit(DismissEvent);
+        });
+
+        Self {
+            picker,
+            rem_width,
+            popover_handle,
+            _subscription,
+        }
     }
 }
 impl ModalView for BranchList {}
@@ -132,10 +106,7 @@ impl EventEmitter<DismissEvent> for BranchList {}
 
 impl Focusable for BranchList {
     fn focus_handle(&self, cx: &App) -> FocusHandle {
-        self.picker
-            .as_ref()
-            .map(|picker| picker.focus_handle(cx))
-            .unwrap_or_else(|| self.default_focus_handle.clone())
+        self.picker.focus_handle(cx)
     }
 }
 
@@ -143,24 +114,13 @@ impl Render for BranchList {
     fn render(&mut self, _: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
         v_flex()
             .w(rems(self.rem_width))
-            .map(|parent| match self.picker.as_ref() {
-                Some(picker) => parent.child(picker.clone()).on_mouse_down_out({
-                    let picker = picker.clone();
-                    cx.listener(move |_, _, window, cx| {
-                        picker.update(cx, |this, cx| {
+            .child(self.picker.clone())
+            .on_mouse_down_out({
+                cx.listener(move |this, _, window, cx| {
+                    this.picker.update(cx, |this, cx| {
                         this.cancel(&Default::default(), window, cx);
                     })
                 })
-                }),
-                None => parent.child(
-                    h_flex()
-                        .id("branch-picker-error")
-                        .on_click(
-                            cx.listener(|this, _, window, cx| this.reload_branches(window, cx)),
-                        )
-                        .child("Could not load branches.")
-                        .child("Click to retry"),
-                ),
             })
     }
 }
@@ -184,7 +144,7 @@ impl BranchEntry {
 
 pub struct BranchListDelegate {
     matches: Vec<BranchEntry>,
-    all_branches: Vec<Branch>,
+    all_branches: Option<Vec<Branch>>,
     project: Entity<Project>,
     style: BranchListStyle,
     selected_index: usize,
@@ -194,33 +154,20 @@ pub struct BranchListDelegate {
 }
 
 impl BranchListDelegate {
-    async fn new(
+    fn new(
         project: Entity<Project>,
         style: BranchListStyle,
         branch_name_trailoff_after: usize,
-        cx: &AsyncApp,
-    ) -> Result<Self> {
-        let all_branches_request = cx.update(|cx| {
-            let project = project.read(cx);
-            let first_worktree = project
-                .visible_worktrees(cx)
-                .next()
-                .context("No worktrees found")?;
-            let project_path = ProjectPath::root_path(first_worktree.read(cx).id());
-            anyhow::Ok(project.branches(project_path, cx))
-        })??;
-
-        let all_branches = all_branches_request.await?;
-
-        Ok(Self {
+    ) -> Self {
+        Self {
             matches: vec![],
             project,
             style,
-            all_branches,
+            all_branches: None,
             selected_index: 0,
             last_query: Default::default(),
             branch_name_trailoff_after,
-        })
+        }
     }
 
     pub fn branch_count(&self) -> usize {
@@ -261,32 +208,31 @@ impl PickerDelegate for BranchListDelegate {
         window: &mut Window,
         cx: &mut Context<Picker<Self>>,
     ) -> Task<()> {
+        let Some(mut all_branches) = self.all_branches.clone() else {
+            return Task::ready(());
+        };
+
         cx.spawn_in(window, move |picker, mut cx| async move {
-            let candidates = picker.update(&mut cx, |picker, _| {
             const RECENT_BRANCHES_COUNT: usize = 10;
-                let mut branches = picker.delegate.all_branches.clone();
             if query.is_empty() {
-                    if branches.len() > RECENT_BRANCHES_COUNT {
+                if all_branches.len() > RECENT_BRANCHES_COUNT {
                     // Truncate list of recent branches
                     // Do a partial sort to show recent-ish branches first.
-                        branches.select_nth_unstable_by(RECENT_BRANCHES_COUNT - 1, |lhs, rhs| {
+                    all_branches.select_nth_unstable_by(RECENT_BRANCHES_COUNT - 1, |lhs, rhs| {
                         rhs.priority_key().cmp(&lhs.priority_key())
                     });
-                        branches.truncate(RECENT_BRANCHES_COUNT);
+                    all_branches.truncate(RECENT_BRANCHES_COUNT);
                 }
-                    branches.sort_unstable_by(|lhs, rhs| {
+                all_branches.sort_unstable_by(|lhs, rhs| {
                     rhs.is_head.cmp(&lhs.is_head).then(lhs.name.cmp(&rhs.name))
                 });
             }
-                branches
+
+            let candidates = all_branches
                 .into_iter()
                 .enumerate()
                 .map(|(ix, command)| StringMatchCandidate::new(ix, &command.name))
-                    .collect::<Vec<StringMatchCandidate>>()
-            });
-            let Some(candidates) = candidates.log_err() else {
-                return;
-            };
+                .collect::<Vec<StringMatchCandidate>>();
             let matches: Vec<BranchEntry> = if query.is_empty() {
                 candidates
                     .into_iter()

crates/git_ui/src/commit_modal.rs

@@ -5,7 +5,7 @@ use crate::git_panel::{commit_message_editor, GitPanel};
 use git::{Commit, ShowCommitEditor};
 use panel::{panel_button, panel_editor_style, panel_filled_button};
 use project::Project;
-use ui::{prelude::*, KeybindingHint, PopoverButton, Tooltip, TriggerablePopover};
+use ui::{prelude::*, KeybindingHint, PopoverMenu, Tooltip};
 
 use editor::{Editor, EditorElement};
 use gpui::*;
@@ -265,12 +265,20 @@ impl CommitModal {
             }))
             .style(ButtonStyle::Transparent);
 
-        let branch_picker = PopoverButton::new(
-            self.branch_list.clone(),
-            Corner::BottomLeft,
+        let branch_picker = PopoverMenu::new("popover-button")
+            .menu({
+                let branch_list = self.branch_list.clone();
+                move |_window, _cx| Some(branch_list.clone())
+            })
+            .trigger_with_tooltip(
                 branch_picker_button,
                 Tooltip::for_action_title("Switch Branch", &zed_actions::git::Branch),
-        );
+            )
+            .anchor(Corner::BottomLeft)
+            .offset(gpui::Point {
+                x: px(0.0),
+                y: px(-2.0),
+            });
 
         let close_kb_hint =
             if let Some(close_kb) = ui::KeyBinding::for_action(&menu::Cancel, window, cx) {
@@ -305,12 +313,7 @@ impl CommitModal {
             .w_full()
             .h(px(self.properties.footer_height))
             .gap_1()
-            .child(
-                h_flex()
-                    .gap_1()
-                    .child(branch_picker.render(window, cx))
-                    .children(co_authors),
-            )
+            .child(h_flex().gap_1().child(branch_picker).children(co_authors))
             .child(div().flex_1())
             .child(
                 h_flex()
@@ -351,7 +354,7 @@ impl Render for CommitModal {
             .on_action(
                 cx.listener(|this, _: &zed_actions::git::Branch, window, cx| {
                     this.branch_list.update(cx, |branch_list, cx| {
-                        branch_list.menu_handle(window, cx).toggle(window, cx);
+                        branch_list.popover_handle.toggle(window, cx);
                     })
                 }),
             )

crates/git_ui/src/git_panel.rs

@@ -1,7 +1,6 @@
-use crate::branch_picker::{self, BranchList};
+use crate::branch_picker::{self};
 use crate::git_panel_settings::StatusStyle;
 use crate::remote_output_toast::{RemoteAction, RemoteOutputToast};
-use crate::repository_selector::RepositorySelectorPopoverMenu;
 use crate::{
     git_panel_settings::GitPanelSettings, git_status_icon, repository_selector::RepositorySelector,
 };
@@ -41,8 +40,8 @@ use std::{collections::HashSet, sync::Arc, time::Duration, usize};
 use strum::{IntoEnumIterator, VariantNames};
 use time::OffsetDateTime;
 use ui::{
-    prelude::*, ButtonLike, Checkbox, ContextMenu, ElevationIndex, PopoverButton, PopoverMenu,
-    Scrollbar, ScrollbarState, Tooltip,
+    prelude::*, ButtonLike, Checkbox, ContextMenu, ElevationIndex, PopoverMenu, Scrollbar,
+    ScrollbarState, Tooltip,
 };
 use util::{maybe, post_inc, ResultExt, TryFutureExt};
 
@@ -206,7 +205,6 @@ pub struct GitPanel {
     pending_commit: Option<Task<()>>,
     pending_serialization: Task<Option<()>>,
     pub(crate) project: Entity<Project>,
-    repository_selector: Entity<RepositorySelector>,
     scroll_handle: UniformListScrollHandle,
     scrollbar_state: ScrollbarState,
     selected_entry: Option<usize>,
@@ -311,9 +309,6 @@ impl GitPanel {
             let scrollbar_state =
                 ScrollbarState::new(scroll_handle.clone()).parent_entity(&cx.entity());
 
-            let repository_selector =
-                cx.new(|cx| RepositorySelector::new(project.clone(), window, cx));
-
             let mut git_panel = Self {
                 pending_remote_operations: Default::default(),
                 remote_operation_id: 0,
@@ -333,7 +328,6 @@ impl GitPanel {
                 pending_commit: None,
                 pending_serialization: Task::ready(None),
                 project,
-                repository_selector,
                 scroll_handle,
                 scrollbar_state,
                 selected_entry: None,
@@ -2039,14 +2033,13 @@ impl GitPanel {
                 .display_name(project, cx)
                 .trim_end_matches("/"),
         ));
-        let branches = branch_picker::popover(self.project.clone(), window, cx);
+
         let footer = v_flex()
             .child(PanelRepoFooter::new(
                 "footer-button",
                 display_name,
                 branch,
                 Some(git_panel),
-                Some(branches),
             ))
             .child(
                 panel_editor_container(window, cx)
@@ -3072,7 +3065,6 @@ pub struct PanelRepoFooter {
     //
     // For now just take an option here, and we won't bind handlers to buttons in previews.
     git_panel: Option<Entity<GitPanel>>,
-    branches: Option<Entity<BranchList>>,
 }
 
 impl PanelRepoFooter {
@@ -3081,14 +3073,12 @@ impl PanelRepoFooter {
         active_repository: SharedString,
         branch: Option<Branch>,
         git_panel: Option<Entity<GitPanel>>,
-        branches: Option<Entity<BranchList>>,
     ) -> Self {
         Self {
             id: id.into(),
             active_repository,
             branch,
             git_panel,
-            branches,
         }
     }
 
@@ -3102,7 +3092,6 @@ impl PanelRepoFooter {
             active_repository,
             branch,
             git_panel: None,
-            branches: None,
         }
     }
 
@@ -3324,7 +3313,7 @@ impl PanelRepoFooter {
 }
 
 impl RenderOnce for PanelRepoFooter {
-    fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement {
+    fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement {
         let active_repo = self.active_repository.clone();
         let overflow_menu_id: SharedString = format!("overflow-menu-{}", active_repo).into();
         let repo_selector_trigger = Button::new("repo-selector", active_repo)
@@ -3333,29 +3322,36 @@ impl RenderOnce for PanelRepoFooter {
             .label_size(LabelSize::Small)
             .color(Color::Muted);
 
-        let repo_selector = if let Some(panel) = self.git_panel.clone() {
-            let repo_selector = panel.read(cx).repository_selector.clone();
-            let repo_count = repo_selector.read(cx).repositories_len(cx);
-            let single_repo = repo_count == 1;
+        let project = self
+            .git_panel
+            .as_ref()
+            .map(|panel| panel.read(cx).project.clone());
 
-            RepositorySelectorPopoverMenu::new(
-                panel.read(cx).repository_selector.clone(),
+        let single_repo = project
+            .as_ref()
+            .map(|project| project.read(cx).all_repositories(cx).len() == 1)
+            .unwrap_or(true);
+
+        let repo_selector = PopoverMenu::new("repository-switcher")
+            .menu({
+                let project = project.clone();
+                move |window, cx| {
+                    let project = project.clone()?;
+                    Some(cx.new(|cx| RepositorySelector::new(project, window, cx)))
+                }
+            })
+            .trigger_with_tooltip(
                 repo_selector_trigger.disabled(single_repo).truncate(true),
                 Tooltip::text("Switch active repository"),
             )
-            .into_any_element()
-        } else {
-            // for rendering preview, we don't have git_panel there
-            repo_selector_trigger.into_any_element()
-        };
+            .attach(gpui::Corner::BottomLeft)
+            .into_any_element();
 
         let branch = self.branch.clone();
         let branch_name = branch
             .as_ref()
             .map_or(" (no branch)".into(), |branch| branch.name.clone());
 
-        let branches = self.branches.clone();
-
         let branch_selector_button = Button::new("branch-selector", branch_name)
             .style(ButtonStyle::Transparent)
             .size(ButtonSize::None)
@@ -3369,18 +3365,17 @@ impl RenderOnce for PanelRepoFooter {
                 window.dispatch_action(zed_actions::git::Branch.boxed_clone(), cx);
             });
 
-        let branch_selector = if let Some(branches) = branches {
-            PopoverButton::new(
-                branches,
-                Corner::BottomLeft,
+        let branch_selector = PopoverMenu::new("popover-button")
+            .menu(move |window, cx| Some(branch_picker::popover(project.clone()?, window, cx)))
+            .trigger_with_tooltip(
                 branch_selector_button,
                 Tooltip::for_action_title("Switch Branch", &zed_actions::git::Branch),
             )
-            .render(window, cx)
-            .into_any_element()
-        } else {
-            branch_selector_button.into_any_element()
-        };
+            .anchor(Corner::TopLeft)
+            .offset(gpui::Point {
+                x: px(0.0),
+                y: px(-2.0),
+            });
 
         let spinner = self
             .git_panel

crates/git_ui/src/repository_selector.rs

@@ -1,6 +1,6 @@
 use gpui::{
-    AnyElement, AnyView, App, DismissEvent, Entity, EventEmitter, FocusHandle, Focusable,
-    Subscription, Task, WeakEntity,
+    AnyElement, App, DismissEvent, Entity, EventEmitter, FocusHandle, Focusable, Subscription,
+    Task, WeakEntity,
 };
 use picker::{Picker, PickerDelegate};
 use project::{
@@ -8,7 +8,7 @@ use project::{
     Project,
 };
 use std::sync::Arc;
-use ui::{prelude::*, ListItem, ListItemSpacing, PopoverMenu, PopoverMenuHandle, PopoverTrigger};
+use ui::{prelude::*, ListItem, ListItemSpacing};
 
 pub struct RepositorySelector {
     picker: Entity<Picker<RepositorySelectorDelegate>>,
@@ -47,10 +47,6 @@ impl RepositorySelector {
         }
     }
 
-    pub(crate) fn repositories_len(&self, cx: &App) -> usize {
-        self.picker.read(cx).delegate.repository_entries.len()
-    }
-
     fn handle_project_git_event(
         &mut self,
         git_store: &Entity<GitStore>,
@@ -82,54 +78,6 @@ impl Render for RepositorySelector {
     }
 }
 
-#[derive(IntoElement)]
-pub struct RepositorySelectorPopoverMenu<T, TT>
-where
-    T: PopoverTrigger + ButtonCommon,
-    TT: Fn(&mut Window, &mut App) -> AnyView + 'static,
-{
-    repository_selector: Entity<RepositorySelector>,
-    trigger: T,
-    tooltip: TT,
-    handle: Option<PopoverMenuHandle<RepositorySelector>>,
-}
-
-impl<T, TT> RepositorySelectorPopoverMenu<T, TT>
-where
-    T: PopoverTrigger + ButtonCommon,
-    TT: Fn(&mut Window, &mut App) -> AnyView + 'static,
-{
-    pub fn new(repository_selector: Entity<RepositorySelector>, trigger: T, tooltip: TT) -> Self {
-        Self {
-            repository_selector,
-            trigger,
-            tooltip,
-            handle: None,
-        }
-    }
-
-    pub fn with_handle(mut self, handle: PopoverMenuHandle<RepositorySelector>) -> Self {
-        self.handle = Some(handle);
-        self
-    }
-}
-
-impl<T, TT> RenderOnce for RepositorySelectorPopoverMenu<T, TT>
-where
-    T: PopoverTrigger + ButtonCommon,
-    TT: Fn(&mut Window, &mut App) -> AnyView + 'static,
-{
-    fn render(self, _window: &mut Window, _cx: &mut App) -> impl IntoElement {
-        let repository_selector = self.repository_selector.clone();
-
-        PopoverMenu::new("repository-switcher")
-            .menu(move |_window, _cx| Some(repository_selector.clone()))
-            .trigger_with_tooltip(self.trigger, self.tooltip)
-            .attach(gpui::Corner::BottomLeft)
-            .when_some(self.handle.clone(), |menu, handle| menu.with_handle(handle))
-    }
-}
-
 pub struct RepositorySelectorDelegate {
     project: WeakEntity<Project>,
     repository_selector: WeakEntity<RepositorySelector>,

crates/language_model_selector/src/language_model_selector.rs

@@ -1,4 +1,4 @@
-use std::sync::Arc;
+use std::{rc::Rc, sync::Arc};
 
 use feature_flags::ZedPro;
 use gpui::{
@@ -11,8 +11,8 @@ use language_model::{
 use picker::{Picker, PickerDelegate};
 use proto::Plan;
 use ui::{
-    prelude::*, ButtonLike, IconButtonShape, ListItem, ListItemSpacing, PopoverButton,
-    PopoverMenuHandle, Tooltip, TriggerablePopover,
+    prelude::*, ButtonLike, IconButtonShape, ListItem, ListItemSpacing, PopoverMenu,
+    PopoverMenuHandle, Tooltip,
 };
 use workspace::ShowConfiguration;
 
@@ -201,16 +201,6 @@ impl Render for LanguageModelSelector {
     }
 }
 
-impl TriggerablePopover for LanguageModelSelector {
-    fn menu_handle(
-        &mut self,
-        _window: &mut Window,
-        _cx: &mut gpui::Context<Self>,
-    ) -> PopoverMenuHandle<Self> {
-        self.popover_menu_handle.clone()
-    }
-}
-
 #[derive(Clone)]
 struct ModelInfo {
     model: Arc<dyn LanguageModel>,
@@ -493,21 +483,26 @@ impl PickerDelegate for LanguageModelPickerDelegate {
     }
 }
 
-pub struct InlineLanguageModelSelector {
-    selector: Entity<LanguageModelSelector>,
-}
-
-impl InlineLanguageModelSelector {
-    pub fn new(selector: Entity<LanguageModelSelector>) -> Self {
-        Self { selector }
+pub fn inline_language_model_selector(
+    f: impl Fn(Arc<dyn LanguageModel>, &App) + 'static,
+) -> AnyElement {
+    let f = Rc::new(f);
+    PopoverMenu::new("popover-button")
+        .menu(move |window, cx| {
+            Some(cx.new(|cx| {
+                LanguageModelSelector::new(
+                    {
+                        let f = f.clone();
+                        move |model, cx| {
+                            f(model, cx);
                         }
-}
-
-impl RenderOnce for InlineLanguageModelSelector {
-    fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement {
-        PopoverButton::new(
-            self.selector,
-            gpui::Corner::TopRight,
+                    },
+                    window,
+                    cx,
+                )
+            }))
+        })
+        .trigger_with_tooltip(
             IconButton::new("context", IconName::SettingsAlt)
                 .shape(IconButtonShape::Square)
                 .icon_size(IconSize::Small)
@@ -528,36 +523,45 @@ impl RenderOnce for InlineLanguageModelSelector {
                 )
             },
         )
-        .render(window, cx)
-    }
+        .anchor(gpui::Corner::TopRight)
+        // .when_some(menu_handle, |el, handle| el.with_handle(handle))
+        .offset(gpui::Point {
+            x: px(0.0),
+            y: px(-2.0),
+        })
+        .into_any_element()
 }
 
-pub struct AssistantLanguageModelSelector {
-    focus_handle: FocusHandle,
-    selector: Entity<LanguageModelSelector>,
-}
-
-impl AssistantLanguageModelSelector {
-    pub fn new(focus_handle: FocusHandle, selector: Entity<LanguageModelSelector>) -> Self {
-        Self {
-            focus_handle,
-            selector,
-        }
-    }
-}
-
-impl RenderOnce for AssistantLanguageModelSelector {
-    fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement {
+pub fn assistant_language_model_selector(
+    keybinding_target: FocusHandle,
+    menu_handle: Option<PopoverMenuHandle<LanguageModelSelector>>,
+    cx: &App,
+    f: impl Fn(Arc<dyn LanguageModel>, &App) + 'static,
+) -> AnyElement {
     let active_model = LanguageModelRegistry::read_global(cx).active_model();
-        let focus_handle = self.focus_handle.clone();
     let model_name = match active_model {
         Some(model) => model.name().0,
         _ => SharedString::from("No model selected"),
     };
 
-        PopoverButton::new(
-            self.selector.clone(),
-            Corner::BottomRight,
+    let f = Rc::new(f);
+
+    PopoverMenu::new("popover-button")
+        .menu(move |window, cx| {
+            Some(cx.new(|cx| {
+                LanguageModelSelector::new(
+                    {
+                        let f = f.clone();
+                        move |model, cx| {
+                            f(model, cx);
+                        }
+                    },
+                    window,
+                    cx,
+                )
+            }))
+        })
+        .trigger_with_tooltip(
             ButtonLike::new("active-model")
                 .style(ButtonStyle::Subtle)
                 .child(
@@ -578,12 +582,17 @@ impl RenderOnce for AssistantLanguageModelSelector {
                 Tooltip::for_action_in(
                     "Change Model",
                     &ToggleModelSelector,
-                    &focus_handle,
+                    &keybinding_target,
                     window,
                     cx,
                 )
             },
         )
-        .render(window, cx)
-    }
+        .anchor(Corner::BottomRight)
+        .when_some(menu_handle, |el, handle| el.with_handle(handle))
+        .offset(gpui::Point {
+            x: px(0.0),
+            y: px(-2.0),
+        })
+        .into_any_element()
 }

crates/ui/src/components.rs

@@ -19,7 +19,6 @@ mod modal;
 mod navigable;
 mod numeric_stepper;
 mod popover;
-mod popover_button;
 mod popover_menu;
 mod radio;
 mod right_click_menu;
@@ -57,7 +56,6 @@ pub use modal::*;
 pub use navigable::*;
 pub use numeric_stepper::*;
 pub use popover::*;
-pub use popover_button::*;
 pub use popover_menu::*;
 pub use radio::*;
 pub use right_click_menu::*;

crates/ui/src/components/popover_button.rs

@@ -1,57 +0,0 @@
-use gpui::{AnyView, Corner, Entity, ManagedView};
-
-use crate::{prelude::*, PopoverMenu, PopoverMenuHandle, PopoverTrigger};
-
-pub trait TriggerablePopover: ManagedView {
-    fn menu_handle(
-        &mut self,
-        window: &mut Window,
-        cx: &mut gpui::Context<Self>,
-    ) -> PopoverMenuHandle<Self>;
-}
-
-pub struct PopoverButton<T, B, F> {
-    selector: Entity<T>,
-    button: B,
-    tooltip: F,
-    corner: Corner,
-}
-
-impl<T, B, F> PopoverButton<T, B, F> {
-    pub fn new(selector: Entity<T>, corner: Corner, button: B, tooltip: F) -> Self
-    where
-        F: Fn(&mut Window, &mut App) -> AnyView + 'static,
-    {
-        Self {
-            selector,
-            button,
-            tooltip,
-            corner,
-        }
-    }
-}
-
-impl<T: TriggerablePopover, B: PopoverTrigger + ButtonCommon, F> RenderOnce
-    for PopoverButton<T, B, F>
-where
-    F: Fn(&mut Window, &mut App) -> AnyView + 'static,
-{
-    fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement {
-        let menu_handle = self
-            .selector
-            .update(cx, |selector, cx| selector.menu_handle(window, cx));
-
-        PopoverMenu::new("popover-button")
-            .menu({
-                let selector = self.selector.clone();
-                move |_window, _cx| Some(selector.clone())
-            })
-            .trigger_with_tooltip(self.button, self.tooltip)
-            .anchor(self.corner)
-            .with_handle(menu_handle)
-            .offset(gpui::Point {
-                x: px(0.0),
-                y: px(-2.0),
-            })
-    }
-}