assistant panel: Intermediate fix for focus problems (#15674)

Thorsten Ball created

This is an intermediate fix for the focus problems in the assistant
panel. Intermediate because I'm going to shred the whole
ConfigurationView now and replace the tabs inside with a list.


Release Notes:

- N/A

Change summary

crates/assistant/src/assistant_panel.rs         | 58 ++++++++++-----
crates/language_model/src/provider/anthropic.rs | 71 ++++++++++++------
crates/language_model/src/provider/google.rs    | 63 ++++++++++-----
crates/language_model/src/provider/open_ai.rs   | 63 ++++++++++-----
4 files changed, 165 insertions(+), 90 deletions(-)

Detailed changes

crates/assistant/src/assistant_panel.rs 🔗

@@ -943,7 +943,7 @@ impl AssistantPanel {
             });
         } else {
             let configuration = cx.new_view(|cx| {
-                let mut view = ConfigurationView::new(self.focus_handle(cx), cx);
+                let mut view = ConfigurationView::new(cx);
                 if let Some(provider) = provider {
                     view.set_active_tab(provider, cx);
                 }
@@ -3044,11 +3044,6 @@ impl Item for ContextHistory {
     }
 }
 
-pub struct ConfigurationView {
-    fallback_handle: FocusHandle,
-    active_tab: Option<ActiveTab>,
-}
-
 struct ActiveTab {
     provider: Arc<dyn LanguageModelProvider>,
     configuration_prompt: AnyView,
@@ -3067,12 +3062,37 @@ impl ActiveTab {
     }
 }
 
+pub struct ConfigurationView {
+    focus_handle: FocusHandle,
+    active_tab: Option<ActiveTab>,
+}
+
 impl ConfigurationView {
-    fn new(fallback_handle: FocusHandle, _cx: &mut ViewContext<Self>) -> Self {
-        Self {
-            fallback_handle,
+    fn new(cx: &mut ViewContext<Self>) -> Self {
+        let focus_handle = cx.focus_handle();
+
+        cx.on_focus(&focus_handle, |this, cx| {
+            if let Some(focus_handle) = this
+                .active_tab
+                .as_ref()
+                .and_then(|tab| tab.focus_handle.as_ref())
+            {
+                focus_handle.focus(cx);
+            }
+        })
+        .detach();
+
+        let mut this = Self {
+            focus_handle,
             active_tab: None,
+        };
+
+        let providers = LanguageModelRegistry::read_global(cx).providers();
+        if !providers.is_empty() {
+            this.set_active_tab(providers[0].clone(), cx);
         }
+
+        this
     }
 
     fn set_active_tab(
@@ -3085,7 +3105,7 @@ impl ConfigurationView {
         if let Some(focus_handle) = &focus_handle {
             focus_handle.focus(cx);
         } else {
-            self.fallback_handle.focus(cx);
+            self.focus_handle.focus(cx);
         }
 
         let load_credentials = provider.authenticate(cx);
@@ -3224,11 +3244,6 @@ impl ConfigurationView {
 impl Render for ConfigurationView {
     fn render(&mut self, cx: &mut ViewContext<Self>) -> impl IntoElement {
         let providers = LanguageModelRegistry::read_global(cx).providers();
-
-        if self.active_tab.is_none() && !providers.is_empty() {
-            self.set_active_tab(providers[0].clone(), cx);
-        }
-
         let tabs = h_flex().mx_neg_1().gap_3().children(
             providers
                 .iter()
@@ -3237,6 +3252,7 @@ impl Render for ConfigurationView {
 
         v_flex()
             .id("assistant-configuration-view")
+            .track_focus(&self.focus_handle)
             .w_full()
             .min_h_full()
             .p(Spacing::XXLarge.rems(cx))
@@ -3258,7 +3274,12 @@ impl Render for ConfigurationView {
                         .color(Color::Muted),
                     )
                     .child(tabs)
-                    .children(self.render_active_tab_view(cx)),
+                    .when(self.active_tab.is_some(), |this| {
+                        this.children(self.render_active_tab_view(cx))
+                    })
+                    .when(self.active_tab.is_none(), |this| {
+                        this.child(Label::new("No providers configured").color(Color::Warning))
+                    }),
             )
     }
 }
@@ -3271,10 +3292,7 @@ impl EventEmitter<ConfigurationViewEvent> for ConfigurationView {}
 
 impl FocusableView for ConfigurationView {
     fn focus_handle(&self, _: &AppContext) -> FocusHandle {
-        self.active_tab
-            .as_ref()
-            .and_then(|tab| tab.focus_handle.clone())
-            .unwrap_or(self.fallback_handle.clone())
+        self.focus_handle.clone()
     }
 }
 

crates/language_model/src/provider/anthropic.rs 🔗

@@ -383,18 +383,22 @@ impl LanguageModel for AnthropicModel {
 }
 
 struct ConfigurationView {
+    focus_handle: FocusHandle,
     api_key_editor: View<Editor>,
     state: gpui::Model<State>,
 }
 
-impl FocusableView for ConfigurationView {
-    fn focus_handle(&self, cx: &AppContext) -> FocusHandle {
-        self.api_key_editor.read(cx).focus_handle(cx)
-    }
-}
-
 impl ConfigurationView {
-    fn new(state: gpui::Model<State>, cx: &mut WindowContext) -> Self {
+    fn new(state: gpui::Model<State>, cx: &mut ViewContext<Self>) -> Self {
+        let focus_handle = cx.focus_handle();
+
+        cx.on_focus(&focus_handle, |this, cx| {
+            if this.should_render_editor(cx) {
+                this.api_key_editor.read(cx).focus_handle(cx).focus(cx)
+            }
+        })
+        .detach();
+
         Self {
             api_key_editor: cx.new_view(|cx| {
                 let mut editor = Editor::single_line(cx);
@@ -404,6 +408,7 @@ impl ConfigurationView {
                 );
                 editor
             }),
+            focus_handle,
             state,
         }
     }
@@ -453,6 +458,16 @@ impl ConfigurationView {
             },
         )
     }
+
+    fn should_render_editor(&self, cx: &mut ViewContext<Self>) -> bool {
+        !self.state.read(cx).is_authenticated()
+    }
+}
+
+impl FocusableView for ConfigurationView {
+    fn focus_handle(&self, _cx: &AppContext) -> FocusHandle {
+        self.focus_handle.clone()
+    }
 }
 
 impl Render for ConfigurationView {
@@ -464,26 +479,10 @@ impl Render for ConfigurationView {
             "Paste your Anthropic API key below and hit enter to use the assistant:",
         ];
 
-        if self.state.read(cx).is_authenticated() {
-            h_flex()
-                .size_full()
-                .justify_between()
-                .child(
-                    h_flex()
-                        .gap_2()
-                        .child(Indicator::dot().color(Color::Success))
-                        .child(Label::new("API Key configured").size(LabelSize::Small)),
-                )
-                .child(
-                    Button::new("reset-key", "Reset key")
-                        .icon(Some(IconName::Trash))
-                        .icon_size(IconSize::Small)
-                        .icon_position(IconPosition::Start)
-                        .on_click(cx.listener(|this, _, cx| this.reset_api_key(cx))),
-                )
-                .into_any()
-        } else {
+        if self.should_render_editor(cx) {
             v_flex()
+                .id("anthropic-configuration-view")
+                .track_focus(&self.focus_handle)
                 .size_full()
                 .on_action(cx.listener(Self::save_api_key))
                 .children(
@@ -506,6 +505,26 @@ impl Render for ConfigurationView {
                     .size(LabelSize::Small),
                 )
                 .into_any()
+        } else {
+            h_flex()
+                .id("anthropic-configuration-view")
+                .track_focus(&self.focus_handle)
+                .size_full()
+                .justify_between()
+                .child(
+                    h_flex()
+                        .gap_2()
+                        .child(Indicator::dot().color(Color::Success))
+                        .child(Label::new("API Key configured").size(LabelSize::Small)),
+                )
+                .child(
+                    Button::new("reset-key", "Reset key")
+                        .icon(Some(IconName::Trash))
+                        .icon_size(IconSize::Small)
+                        .icon_position(IconPosition::Start)
+                        .on_click(cx.listener(|this, _, cx| this.reset_api_key(cx))),
+                )
+                .into_any()
         }
     }
 }

crates/language_model/src/provider/google.rs 🔗

@@ -292,12 +292,22 @@ impl LanguageModel for GoogleLanguageModel {
 }
 
 struct ConfigurationView {
+    focus_handle: FocusHandle,
     api_key_editor: View<Editor>,
     state: gpui::Model<State>,
 }
 
 impl ConfigurationView {
-    fn new(state: gpui::Model<State>, cx: &mut WindowContext) -> Self {
+    fn new(state: gpui::Model<State>, cx: &mut ViewContext<Self>) -> Self {
+        let focus_handle = cx.focus_handle();
+
+        cx.on_focus(&focus_handle, |this, cx| {
+            if this.should_render_editor(cx) {
+                this.api_key_editor.read(cx).focus_handle(cx).focus(cx)
+            }
+        })
+        .detach();
+
         Self {
             api_key_editor: cx.new_view(|cx| {
                 let mut editor = Editor::single_line(cx);
@@ -305,6 +315,7 @@ impl ConfigurationView {
                 editor
             }),
             state,
+            focus_handle,
         }
     }
 
@@ -362,11 +373,15 @@ impl ConfigurationView {
             },
         )
     }
+
+    fn should_render_editor(&self, cx: &mut ViewContext<Self>) -> bool {
+        !self.state.read(cx).is_authenticated()
+    }
 }
 
 impl FocusableView for ConfigurationView {
-    fn focus_handle(&self, cx: &AppContext) -> FocusHandle {
-        self.api_key_editor.read(cx).focus_handle(cx)
+    fn focus_handle(&self, _cx: &AppContext) -> FocusHandle {
+        self.focus_handle.clone()
     }
 }
 
@@ -379,26 +394,10 @@ impl Render for ConfigurationView {
             "Paste your Google AI API key below and hit enter to use the assistant:",
         ];
 
-        if self.state.read(cx).is_authenticated() {
-            h_flex()
-                .size_full()
-                .justify_between()
-                .child(
-                    h_flex()
-                        .gap_2()
-                        .child(Indicator::dot().color(Color::Success))
-                        .child(Label::new("API Key configured").size(LabelSize::Small)),
-                )
-                .child(
-                    Button::new("reset-key", "Reset key")
-                        .icon(Some(IconName::Trash))
-                        .icon_size(IconSize::Small)
-                        .icon_position(IconPosition::Start)
-                        .on_click(cx.listener(|this, _, cx| this.reset_api_key(cx))),
-                )
-                .into_any()
-        } else {
+        if self.should_render_editor(cx) {
             v_flex()
+                .id("google-ai-configuration-view")
+                .track_focus(&self.focus_handle)
                 .size_full()
                 .on_action(cx.listener(Self::save_api_key))
                 .children(
@@ -421,6 +420,26 @@ impl Render for ConfigurationView {
                     .size(LabelSize::Small),
                 )
                 .into_any()
+        } else {
+            h_flex()
+                .id("google-ai-configuration-view")
+                .track_focus(&self.focus_handle)
+                .size_full()
+                .justify_between()
+                .child(
+                    h_flex()
+                        .gap_2()
+                        .child(Indicator::dot().color(Color::Success))
+                        .child(Label::new("API Key configured").size(LabelSize::Small)),
+                )
+                .child(
+                    Button::new("reset-key", "Reset key")
+                        .icon(Some(IconName::Trash))
+                        .icon_size(IconSize::Small)
+                        .icon_position(IconPosition::Start)
+                        .on_click(cx.listener(|this, _, cx| this.reset_api_key(cx))),
+                )
+                .into_any()
         }
     }
 }

crates/language_model/src/provider/open_ai.rs 🔗

@@ -302,12 +302,22 @@ pub fn count_open_ai_tokens(
 }
 
 struct ConfigurationView {
+    focus_handle: FocusHandle,
     api_key_editor: View<Editor>,
     state: gpui::Model<State>,
 }
 
 impl ConfigurationView {
-    fn new(state: gpui::Model<State>, cx: &mut WindowContext) -> Self {
+    fn new(state: gpui::Model<State>, cx: &mut ViewContext<Self>) -> Self {
+        let focus_handle = cx.focus_handle();
+
+        cx.on_focus(&focus_handle, |this, cx| {
+            if this.should_render_editor(cx) {
+                this.api_key_editor.read(cx).focus_handle(cx).focus(cx)
+            }
+        })
+        .detach();
+
         Self {
             api_key_editor: cx.new_view(|cx| {
                 let mut editor = Editor::single_line(cx);
@@ -318,6 +328,7 @@ impl ConfigurationView {
                 editor
             }),
             state,
+            focus_handle,
         }
     }
 
@@ -375,11 +386,15 @@ impl ConfigurationView {
             },
         )
     }
+
+    fn should_render_editor(&self, cx: &mut ViewContext<Self>) -> bool {
+        !self.state.read(cx).is_authenticated()
+    }
 }
 
 impl FocusableView for ConfigurationView {
-    fn focus_handle(&self, cx: &AppContext) -> FocusHandle {
-        self.api_key_editor.read(cx).focus_handle(cx)
+    fn focus_handle(&self, _cx: &AppContext) -> FocusHandle {
+        self.focus_handle.clone()
     }
 }
 
@@ -394,26 +409,10 @@ impl Render for ConfigurationView {
             "Paste your OpenAI API key below and hit enter to use the assistant:",
         ];
 
-        if self.state.read(cx).is_authenticated() {
-            h_flex()
-                .size_full()
-                .justify_between()
-                .child(
-                    h_flex()
-                        .gap_2()
-                        .child(Indicator::dot().color(Color::Success))
-                        .child(Label::new("API Key configured").size(LabelSize::Small)),
-                )
-                .child(
-                    Button::new("reset-key", "Reset key")
-                        .icon(Some(IconName::Trash))
-                        .icon_size(IconSize::Small)
-                        .icon_position(IconPosition::Start)
-                        .on_click(cx.listener(|this, _, cx| this.reset_api_key(cx))),
-                )
-                .into_any()
-        } else {
+        if self.should_render_editor(cx) {
             v_flex()
+                .id("openai-configuration-view")
+                .track_focus(&self.focus_handle)
                 .size_full()
                 .on_action(cx.listener(Self::save_api_key))
                 .children(
@@ -436,6 +435,26 @@ impl Render for ConfigurationView {
                     .size(LabelSize::Small),
                 )
                 .into_any()
+        } else {
+            h_flex()
+                .id("openai-configuration-view")
+                .track_focus(&self.focus_handle)
+                .size_full()
+                .justify_between()
+                .child(
+                    h_flex()
+                        .gap_2()
+                        .child(Indicator::dot().color(Color::Success))
+                        .child(Label::new("API Key configured").size(LabelSize::Small)),
+                )
+                .child(
+                    Button::new("reset-key", "Reset key")
+                        .icon(Some(IconName::Trash))
+                        .icon_size(IconSize::Small)
+                        .icon_position(IconPosition::Start)
+                        .on_click(cx.listener(|this, _, cx| this.reset_api_key(cx))),
+                )
+                .into_any()
         }
     }
 }