From 4f116da026b12e75feb60d66f6ce19ff4d8cdf8f Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 28 Jun 2023 12:02:18 +0200 Subject: [PATCH] Polish assistant (#2653) This is a mix of styling and behavioral improvements to the assistant that we wanted to land before shipping today. Release Notes: - N/A --- 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(-) diff --git a/assets/keymaps/default.json b/assets/keymaps/default.json index e6727e7a0df69b2397457d6f694de25566dfdb6f..9af246721c74264e7a53711b951869b7190208e3 100644 --- a/assets/keymaps/default.json +++ b/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" } diff --git a/crates/ai/src/assistant.rs b/crates/ai/src/assistant.rs index 01693425fbdf2cbb708c67b1690cf3da4b750371..7631381273a56c3920e2b2179359a4f10ab9611f 100644 --- a/crates/ai/src/assistant.rs +++ b/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::(cx); cx.add_action( - |workspace: &mut Workspace, _: &NewContext, cx: &mut ViewContext| { - if let Some(this) = workspace.panel::(cx) { - this.update(cx, |this, cx| { - this.new_conversation(cx); - }) - } - - workspace.focus_panel::(cx); + |this: &mut AssistantPanel, + _: &workspace::NewFile, + cx: &mut ViewContext| { + this.new_conversation(cx); }, ); cx.add_action(ConversationEditor::assist); @@ -344,9 +340,10 @@ impl AssistantPanel { } fn render_hamburger_button(cx: &mut ViewContext) -> impl Element { - enum ListConversations {} + enum History {} let theme = theme::current(cx); - MouseEventHandler::::new(0, cx, |state, _| { + let tooltip_style = theme::current(cx).tooltip.clone(); + MouseEventHandler::::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::(1, "History".into(), None, tooltip_style, cx) } fn render_editor_tools(&self, cx: &mut ViewContext) -> Vec> { @@ -443,7 +441,7 @@ impl AssistantPanel { }) .with_tooltip::( 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) -> impl Element { - enum AddConversation {} let theme = theme::current(cx); - MouseEventHandler::::new(0, cx, |state, _| { + let tooltip_style = theme::current(cx).tooltip.clone(); + MouseEventHandler::::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::( + 1, + "New Conversation".into(), + Some(Box::new(NewConversation)), + tooltip_style, + cx, + ) } fn render_zoom_button(&self, cx: &mut ViewContext) -> impl Element { 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::( + 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); } } diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 41684f32267b5273e2795435950039422ea9b0ac..20043a9093638c8f10c125dd39c9b3dea8855bed 100644 --- a/crates/gpui/src/app.rs +++ b/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 || { diff --git a/crates/gpui/src/platform/mac/platform.rs b/crates/gpui/src/platform/mac/platform.rs index ea415cc6a66e987052800c3c78e64826e1445355..e1d80fe25ceead41828aa18d90f5938eb518a17f 100644 --- a/crates/gpui/src/platform/mac/platform.rs +++ b/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] } } diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 54b18ea8b33510f61d6e11b13a05de1eedc1d4a2..9776fede2c3fb76c1639ba083ff9d3b0b6d06dca 100644 --- a/crates/workspace/src/pane.rs +++ b/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() }), } diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 23ecbf978ba3b4a9d66d19022743cb9da46f2143..1cb7bc8edcc0c9efec817c9038540a07315d1f66 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -70,8 +70,6 @@ fn main() { init_panic_hook(&app); - app.background(); - load_embedded_fonts(&app); let fs = Arc::new(RealFs); diff --git a/styles/src/styleTree/assistant.ts b/styles/src/styleTree/assistant.ts index 91b52f20ad1902bac074ad1575dd83dc6a3230fc..b6e97303f24f7f3006d97a0db372e58103527567 100644 --- a/styles/src/styleTree/assistant.ts +++ b/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" }),