Polish assistant (#2653)

Antonio Scandurra created

This is a mix of styling and behavioral improvements to the assistant
that we wanted to land before shipping today.

Release Notes:

- N/A

Change summary

assets/keymaps/default.json              |  5 +
crates/ai/src/assistant.rs               | 72 +++++++++++++++++--------
crates/gpui/src/app.rs                   | 23 ++++++++
crates/gpui/src/platform/mac/platform.rs |  1 
crates/workspace/src/pane.rs             | 30 ++++++---
crates/zed/src/main.rs                   |  2 
styles/src/styleTree/assistant.ts        | 20 +++---
7 files changed, 102 insertions(+), 51 deletions(-)

Detailed changes

assets/keymaps/default.json 🔗

@@ -200,8 +200,8 @@
   {
     "context": "AssistantPanel",
     "bindings": {
-        "cmd-g": "search::SelectNextMatch",
-        "cmd-shift-g": "search::SelectPrevMatch"
+      "cmd-g": "search::SelectNextMatch",
+      "cmd-shift-g": "search::SelectPrevMatch"
     }
   },
   {
@@ -408,6 +408,7 @@
       "cmd-shift-p": "command_palette::Toggle",
       "cmd-shift-m": "diagnostics::Deploy",
       "cmd-shift-e": "project_panel::ToggleFocus",
+      "cmd-?": "assistant::ToggleFocus",
       "cmd-alt-s": "workspace::SaveAll",
       "cmd-k m": "language_selector::Toggle"
     }

crates/ai/src/assistant.rs 🔗

@@ -51,7 +51,7 @@ const OPENAI_API_URL: &'static str = "https://api.openai.com/v1";
 actions!(
     assistant,
     [
-        NewContext,
+        NewConversation,
         Assist,
         Split,
         CycleMessageRole,
@@ -70,14 +70,10 @@ pub fn init(cx: &mut AppContext) {
 
     settings::register::<AssistantSettings>(cx);
     cx.add_action(
-        |workspace: &mut Workspace, _: &NewContext, cx: &mut ViewContext<Workspace>| {
-            if let Some(this) = workspace.panel::<AssistantPanel>(cx) {
-                this.update(cx, |this, cx| {
-                    this.new_conversation(cx);
-                })
-            }
-
-            workspace.focus_panel::<AssistantPanel>(cx);
+        |this: &mut AssistantPanel,
+         _: &workspace::NewFile,
+         cx: &mut ViewContext<AssistantPanel>| {
+            this.new_conversation(cx);
         },
     );
     cx.add_action(ConversationEditor::assist);
@@ -344,9 +340,10 @@ impl AssistantPanel {
     }
 
     fn render_hamburger_button(cx: &mut ViewContext<Self>) -> impl Element<Self> {
-        enum ListConversations {}
+        enum History {}
         let theme = theme::current(cx);
-        MouseEventHandler::<ListConversations, _>::new(0, cx, |state, _| {
+        let tooltip_style = theme::current(cx).tooltip.clone();
+        MouseEventHandler::<History, _>::new(0, cx, |state, _| {
             let style = theme.assistant.hamburger_button.style_for(state);
             Svg::for_style(style.icon.clone())
                 .contained()
@@ -360,6 +357,7 @@ impl AssistantPanel {
                 this.set_active_editor_index(this.prev_active_editor_index, cx);
             }
         })
+        .with_tooltip::<History>(1, "History".into(), None, tooltip_style, cx)
     }
 
     fn render_editor_tools(&self, cx: &mut ViewContext<Self>) -> Vec<AnyElement<Self>> {
@@ -443,7 +441,7 @@ impl AssistantPanel {
         })
         .with_tooltip::<QuoteSelection>(
             1,
-            "Assist".into(),
+            "Quote Selection".into(),
             Some(Box::new(QuoteSelection)),
             tooltip_style,
             cx,
@@ -451,9 +449,9 @@ impl AssistantPanel {
     }
 
     fn render_plus_button(cx: &mut ViewContext<Self>) -> impl Element<Self> {
-        enum AddConversation {}
         let theme = theme::current(cx);
-        MouseEventHandler::<AddConversation, _>::new(0, cx, |state, _| {
+        let tooltip_style = theme::current(cx).tooltip.clone();
+        MouseEventHandler::<NewConversation, _>::new(0, cx, |state, _| {
             let style = theme.assistant.plus_button.style_for(state);
             Svg::for_style(style.icon.clone())
                 .contained()
@@ -463,12 +461,20 @@ impl AssistantPanel {
         .on_click(MouseButton::Left, |_, this: &mut Self, cx| {
             this.new_conversation(cx);
         })
+        .with_tooltip::<NewConversation>(
+            1,
+            "New Conversation".into(),
+            Some(Box::new(NewConversation)),
+            tooltip_style,
+            cx,
+        )
     }
 
     fn render_zoom_button(&self, cx: &mut ViewContext<Self>) -> impl Element<Self> {
         enum ToggleZoomButton {}
 
         let theme = theme::current(cx);
+        let tooltip_style = theme::current(cx).tooltip.clone();
         let style = if self.zoomed {
             &theme.assistant.zoom_out_button
         } else {
@@ -485,6 +491,17 @@ impl AssistantPanel {
         .on_click(MouseButton::Left, |_, this, cx| {
             this.toggle_zoom(&ToggleZoom, cx);
         })
+        .with_tooltip::<ToggleZoom>(
+            0,
+            if self.zoomed {
+                "Zoom Out".into()
+            } else {
+                "Zoom In".into()
+            },
+            Some(Box::new(ToggleZoom)),
+            tooltip_style,
+            cx,
+        )
     }
 
     fn render_saved_conversation(
@@ -610,19 +627,22 @@ impl View for AssistantPanel {
                     .left()
                     .flex(1., false)
             });
+            let mut header = Flex::row()
+                .with_child(Self::render_hamburger_button(cx).aligned())
+                .with_children(title);
+            if self.has_focus {
+                header.add_children(
+                    self.render_editor_tools(cx)
+                        .into_iter()
+                        .map(|tool| tool.aligned().flex_float()),
+                );
+                header.add_child(Self::render_plus_button(cx).aligned().flex_float());
+                header.add_child(self.render_zoom_button(cx).aligned());
+            }
 
             Flex::column()
                 .with_child(
-                    Flex::row()
-                        .with_child(Self::render_hamburger_button(cx).aligned())
-                        .with_children(title)
-                        .with_children(
-                            self.render_editor_tools(cx)
-                                .into_iter()
-                                .map(|tool| tool.aligned().flex_float()),
-                        )
-                        .with_child(Self::render_plus_button(cx).aligned().flex_float())
-                        .with_child(self.render_zoom_button(cx).aligned())
+                    header
                         .contained()
                         .with_style(theme.workspace.tab_bar.container)
                         .expanded()
@@ -658,6 +678,7 @@ impl View for AssistantPanel {
         self.has_focus = true;
         self.toolbar
             .update(cx, |toolbar, cx| toolbar.focus_changed(true, cx));
+        cx.notify();
         if cx.is_self_focused() {
             if let Some(editor) = self.active_editor() {
                 cx.focus(editor);
@@ -671,6 +692,7 @@ impl View for AssistantPanel {
         self.has_focus = false;
         self.toolbar
             .update(cx, |toolbar, cx| toolbar.focus_changed(false, cx));
+        cx.notify();
     }
 }
 
@@ -1659,6 +1681,8 @@ impl ConversationEditor {
                     |selections| selections.select_ranges(new_selections),
                 );
             });
+            // Avoid scrolling to the new cursor position so the assistant's output is stable.
+            cx.defer(|this, _| this.scroll_position = None);
         }
     }
 

crates/gpui/src/app.rs 🔗

@@ -152,6 +152,29 @@ impl App {
             asset_source,
         ))));
 
+        foreground_platform.on_event(Box::new({
+            let cx = app.0.clone();
+            move |event| {
+                if let Event::KeyDown(KeyDownEvent { keystroke, .. }) = &event {
+                    // Allow system menu "cmd-?" shortcut to be overridden
+                    if keystroke.cmd
+                        && !keystroke.shift
+                        && !keystroke.alt
+                        && !keystroke.function
+                        && keystroke.key == "?"
+                    {
+                        if cx
+                            .borrow_mut()
+                            .update_active_window(|cx| cx.dispatch_keystroke(keystroke))
+                            .unwrap_or(false)
+                        {
+                            return true;
+                        }
+                    }
+                }
+                false
+            }
+        }));
         foreground_platform.on_quit(Box::new({
             let cx = app.0.clone();
             move || {

crates/gpui/src/platform/mac/platform.rs 🔗

@@ -939,7 +939,6 @@ extern "C" fn send_event(this: &mut Object, _sel: Sel, native_event: id) {
                 }
             }
         }
-
         msg_send![super(this, class!(NSApplication)), sendEvent: native_event]
     }
 }

crates/workspace/src/pane.rs 🔗

@@ -286,19 +286,27 @@ impl Pane {
                         pane.tab_bar_context_menu
                             .handle_if_kind(TabBarContextMenuKind::Split),
                     ))
-                    .with_child(Pane::render_tab_bar_button(
-                        2,
+                    .with_child({
+                        let icon_path;
+                        let tooltip_label;
                         if pane.is_zoomed() {
-                            "icons/minimize_8.svg"
+                            icon_path = "icons/minimize_8.svg";
+                            tooltip_label = "Zoom In".into();
                         } else {
-                            "icons/maximize_8.svg"
-                        },
-                        pane.is_zoomed(),
-                        Some(("Toggle Zoom".into(), Some(Box::new(ToggleZoom)))),
-                        cx,
-                        move |pane, cx| pane.toggle_zoom(&Default::default(), cx),
-                        None,
-                    ))
+                            icon_path = "icons/maximize_8.svg";
+                            tooltip_label = "Zoom In".into();
+                        }
+
+                        Pane::render_tab_bar_button(
+                            2,
+                            icon_path,
+                            pane.is_zoomed(),
+                            Some((tooltip_label, Some(Box::new(ToggleZoom)))),
+                            cx,
+                            move |pane, cx| pane.toggle_zoom(&Default::default(), cx),
+                            None,
+                        )
+                    })
                     .into_any()
             }),
         }

crates/zed/src/main.rs 🔗

@@ -72,8 +72,6 @@ fn main() {
     let installation_id = app.background().block(installation_id()).ok();
     init_panic_hook(&app, installation_id.clone());
 
-    app.background();
-
     load_embedded_fonts(&app);
 
     let fs = Arc::new(RealFs);

styles/src/styleTree/assistant.ts 🔗

@@ -11,7 +11,6 @@ export default function assistant(colorScheme: ColorScheme) {
             padding: { left: 12 },
         },
         messageHeader: {
-            border: border(layer, "default", { bottom: true, top: true }),
             margin: { bottom: 6, top: 6 },
             background: editor(colorScheme).background,
         },
@@ -26,7 +25,7 @@ export default function assistant(colorScheme: ColorScheme) {
                     },
                 },
                 container: {
-                    margin: { left: 12 },
+                    padding: { left: 12, right: 8.5 },
                 }
             },
             state: {
@@ -48,7 +47,7 @@ export default function assistant(colorScheme: ColorScheme) {
                     },
                 },
                 container: {
-                    margin: { left: 12 },
+                    padding: { left: 8.5, right: 8.5 },
                 }
             },
             state: {
@@ -70,7 +69,7 @@ export default function assistant(colorScheme: ColorScheme) {
                     },
                 },
                 container: {
-                    margin: { left: 12 },
+                    padding: { left: 8.5, right: 8.5 },
                 }
             },
             state: {
@@ -92,7 +91,7 @@ export default function assistant(colorScheme: ColorScheme) {
                     },
                 },
                 container: {
-                    margin: { left: 12, right: 24 },
+                    padding: { left: 8.5, right: 8.5 },
                 }
             },
             state: {
@@ -114,7 +113,7 @@ export default function assistant(colorScheme: ColorScheme) {
                     },
                 },
                 container: {
-                    margin: { right: 12 },
+                    padding: { left: 10, right: 10 },
                 }
             },
             state: {
@@ -136,7 +135,7 @@ export default function assistant(colorScheme: ColorScheme) {
                     },
                 },
                 container: {
-                    margin: { right: 12 },
+                    padding: { left: 10, right: 10 },
                 }
             },
             state: {
@@ -158,7 +157,7 @@ export default function assistant(colorScheme: ColorScheme) {
                     },
                 },
                 container: {
-                    margin: { right: 12 },
+                    padding: { left: 10, right: 10 },
                 }
             },
             state: {
@@ -170,7 +169,6 @@ export default function assistant(colorScheme: ColorScheme) {
             }
         }),
         title: {
-            margin: { left: 12 },
             ...text(layer, "sans", "default", { size: "sm" })
         },
         savedConversation: {
@@ -239,14 +237,14 @@ export default function assistant(colorScheme: ColorScheme) {
         }),
         remainingTokens: {
             background: background(layer, "on"),
-            margin: { top: 12, right: 12 },
+            margin: { top: 12, right: 24 },
             padding: 4,
             cornerRadius: 4,
             ...text(layer, "sans", "positive", { size: "xs" }),
         },
         noRemainingTokens: {
             background: background(layer, "on"),
-            margin: { top: 12, right: 12 },
+            margin: { top: 12, right: 24 },
             padding: 4,
             cornerRadius: 4,
             ...text(layer, "sans", "negative", { size: "xs" }),