From 4ee20dda23256517410b16ac2cc307d99a48e75a Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Sun, 30 Mar 2025 18:52:48 -0300 Subject: [PATCH] assistant2: Adjust edit files design (#27762) This PR includes design tweaks to elements involved on the "edit files" flow: the bar that appears above the message editor, buttons on the multibuffer hunks, adding keybindings to the "Review Changes" button, etc. Release Notes: - N/A --- Cargo.lock | 1 + assets/keymaps/default-linux.json | 3 +- assets/keymaps/default-macos.json | 3 +- crates/assistant2/Cargo.toml | 1 + crates/assistant2/src/active_thread.rs | 2 +- crates/assistant2/src/assistant.rs | 1 + crates/assistant2/src/assistant_diff.rs | 91 +++++++--------- crates/assistant2/src/assistant_panel.rs | 23 +++- crates/assistant2/src/message_editor.rs | 128 +++++++++++++++++------ 9 files changed, 165 insertions(+), 88 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 649c553278..731551ee52 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -497,6 +497,7 @@ dependencies = [ "serde", "serde_json", "settings", + "smallvec", "smol", "streaming_diff", "telemetry", diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index c9953541b7..73af3e169d 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -651,7 +651,8 @@ "context": "MessageEditor > Editor", "bindings": { "enter": "assistant2::Chat", - "ctrl-i": "assistant2::ToggleProfileSelector" + "ctrl-i": "assistant2::ToggleProfileSelector", + "shift-ctrl-r": "assistant2::OpenAssistantDiff" } }, { diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 7ff2ae0a25..18e1e0580e 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -305,8 +305,7 @@ "bindings": { "enter": "assistant2::Chat", "cmd-i": "assistant2::ToggleProfileSelector", - "cmd-g d": "git::Diff", - "shift-escape": "git::ExpandCommitEditor" + "shift-ctrl-r": "assistant2::OpenAssistantDiff" } }, { diff --git a/crates/assistant2/Cargo.toml b/crates/assistant2/Cargo.toml index b19e0ab1a8..0c7c36a460 100644 --- a/crates/assistant2/Cargo.toml +++ b/crates/assistant2/Cargo.toml @@ -69,6 +69,7 @@ schemars.workspace = true serde.workspace = true serde_json.workspace = true settings.workspace = true +smallvec.workspace = true smol.workspace = true streaming_diff.workspace = true telemetry.workspace = true diff --git a/crates/assistant2/src/active_thread.rs b/crates/assistant2/src/active_thread.rs index ee17a76d7c..6b63a47acf 100644 --- a/crates/assistant2/src/active_thread.rs +++ b/crates/assistant2/src/active_thread.rs @@ -1268,7 +1268,7 @@ impl ActiveThread { let editor_bg = cx.theme().colors().editor_background; - div().py_2().child( + div().pt_0p5().pb_2().child( v_flex() .rounded_lg() .border_1() diff --git a/crates/assistant2/src/assistant.rs b/crates/assistant2/src/assistant.rs index 04c8dc2f43..ff17faa465 100644 --- a/crates/assistant2/src/assistant.rs +++ b/crates/assistant2/src/assistant.rs @@ -65,6 +65,7 @@ actions!( RemoveFocusedContext, AcceptSuggestedContext, OpenActiveThreadAsMarkdown, + OpenAssistantDiff, ToggleKeep, Reject, RejectAll, diff --git a/crates/assistant2/src/assistant_diff.rs b/crates/assistant2/src/assistant_diff.rs index c2c7561a4a..5d4f4dc515 100644 --- a/crates/assistant2/src/assistant_diff.rs +++ b/crates/assistant2/src/assistant_diff.rs @@ -471,6 +471,7 @@ impl Item for AssistantDiff { impl Render for AssistantDiff { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { let is_empty = self.multibuffer.read(cx).is_empty(); + div() .track_focus(&self.focus_handle) .key_context(if is_empty { @@ -503,16 +504,17 @@ fn render_diff_hunk_controls( cx: &mut App, ) -> AnyElement { let editor = assistant_diff.read(cx).editor.clone(); + h_flex() .h(line_height) - .mr_1() + .mr_0p5() .gap_1() .px_0p5() .pb_1() .border_x_1() .border_b_1() - .border_color(cx.theme().colors().border_variant) - .rounded_b_lg() + .border_color(cx.theme().colors().border) + .rounded_b_md() .bg(cx.theme().colors().editor_background) .gap_1() .occlude() @@ -520,24 +522,16 @@ fn render_diff_hunk_controls( .children(if status.has_secondary_hunk() { vec![ Button::new("reject", "Reject") - .key_binding(KeyBinding::for_action_in( - &crate::Reject, - &editor.read(cx).focus_handle(cx), - window, - cx, - )) - .tooltip({ - let focus_handle = editor.focus_handle(cx); - move |window, cx| { - Tooltip::for_action_in( - "Reject Hunk", - &crate::Reject, - &focus_handle, - window, - cx, - ) - } - }) + .disabled(is_created_file) + .key_binding( + KeyBinding::for_action_in( + &crate::Reject, + &editor.read(cx).focus_handle(cx), + window, + cx, + ) + .map(|kb| kb.size(rems_from_px(12.))), + ) .on_click({ let editor = editor.clone(); move |_event, window, cx| { @@ -547,27 +541,17 @@ fn render_diff_hunk_controls( editor.restore_hunks_in_ranges(vec![point..point], window, cx); }); } - }) - .disabled(is_created_file), + }), Button::new(("keep", row as u64), "Keep") - .key_binding(KeyBinding::for_action_in( - &crate::ToggleKeep, - &editor.read(cx).focus_handle(cx), - window, - cx, - )) - .tooltip({ - let focus_handle = editor.focus_handle(cx); - move |window, cx| { - Tooltip::for_action_in( - "Keep Hunk", - &crate::ToggleKeep, - &focus_handle, - window, - cx, - ) - } - }) + .key_binding( + KeyBinding::for_action_in( + &crate::ToggleKeep, + &editor.read(cx).focus_handle(cx), + window, + cx, + ) + .map(|kb| kb.size(rems_from_px(12.))), + ) .on_click({ let assistant_diff = assistant_diff.clone(); move |_event, _window, cx| { @@ -583,12 +567,12 @@ fn render_diff_hunk_controls( ] } else { vec![Button::new(("review", row as u64), "Review") - .tooltip({ - let focus_handle = editor.focus_handle(cx); - move |window, cx| { - Tooltip::for_action_in("Review", &ToggleKeep, &focus_handle, window, cx) - } - }) + .key_binding(KeyBinding::for_action_in( + &ToggleKeep, + &editor.read(cx).focus_handle(cx), + window, + cx, + )) .on_click({ let assistant_diff = assistant_diff.clone(); move |_event, _window, cx| { @@ -752,16 +736,21 @@ impl ToolbarItemView for AssistantDiffToolbar { impl Render for AssistantDiffToolbar { fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { - if self.assistant_diff(cx).is_none() { + let assistant_diff = match self.assistant_diff(cx) { + Some(ad) => ad, + None => return div(), + }; + + let is_empty = assistant_diff.read(cx).multibuffer.read(cx).is_empty(); + + if is_empty { return div(); } h_group_xl() .my_neg_1() .items_center() - .py_1() - .pl_2() - .pr_1() + .p_1() .flex_wrap() .justify_between() .child( diff --git a/crates/assistant2/src/assistant_panel.rs b/crates/assistant2/src/assistant_panel.rs index 7a3faa6268..40d0cd1f05 100644 --- a/crates/assistant2/src/assistant_panel.rs +++ b/crates/assistant2/src/assistant_panel.rs @@ -39,8 +39,8 @@ use crate::thread::{Thread, ThreadError, ThreadId}; use crate::thread_history::{PastContext, PastThread, ThreadHistory}; use crate::thread_store::ThreadStore; use crate::{ - InlineAssistant, NewPromptEditor, NewThread, OpenActiveThreadAsMarkdown, OpenConfiguration, - OpenHistory, + AssistantDiff, InlineAssistant, NewPromptEditor, NewThread, OpenActiveThreadAsMarkdown, + OpenAssistantDiff, OpenConfiguration, OpenHistory, }; action_with_deprecated_aliases!( @@ -90,6 +90,14 @@ pub fn init(cx: &mut App) { workspace.focus_panel::(window, cx); panel.update(cx, |panel, cx| panel.open_configuration(window, cx)); } + }) + .register_action(|workspace, _: &OpenAssistantDiff, window, cx| { + if let Some(panel) = workspace.panel::(cx) { + workspace.focus_panel::(window, cx); + panel.update(cx, |panel, cx| { + panel.open_assistant_diff(&OpenAssistantDiff, window, cx); + }); + } }); }, ) @@ -432,6 +440,16 @@ impl AssistantPanel { }) } + pub fn open_assistant_diff( + &mut self, + _: &OpenAssistantDiff, + window: &mut Window, + cx: &mut Context, + ) { + let thread = self.thread.read(cx).thread().clone(); + AssistantDiff::deploy(thread, self.workspace.clone(), window, cx).log_err(); + } + pub(crate) fn open_configuration(&mut self, window: &mut Window, cx: &mut Context) { let context_server_manager = self.thread_store.read(cx).context_server_manager(); let tools = self.thread_store.read(cx).tools(); @@ -1105,6 +1123,7 @@ impl Render for AssistantPanel { })) .on_action(cx.listener(Self::open_active_thread_as_markdown)) .on_action(cx.listener(Self::deploy_prompt_library)) + .on_action(cx.listener(Self::open_assistant_diff)) .child(self.render_toolbar(window, cx)) .map(|parent| match self.active_view { ActiveView::Thread => parent diff --git a/crates/assistant2/src/message_editor.rs b/crates/assistant2/src/message_editor.rs index 87ab684a02..6c5d64a5d0 100644 --- a/crates/assistant2/src/message_editor.rs +++ b/crates/assistant2/src/message_editor.rs @@ -6,8 +6,8 @@ use editor::{ContextMenuOptions, ContextMenuPlacement, Editor, EditorElement, Ed use file_icons::FileIcons; use fs::Fs; use gpui::{ - Animation, AnimationExt, App, DismissEvent, Entity, Focusable, Subscription, TextStyle, - WeakEntity, + linear_color_stop, linear_gradient, point, Animation, AnimationExt, App, DismissEvent, Entity, + Focusable, Subscription, TextStyle, WeakEntity, }; use language_model::LanguageModelRegistry; use language_model_selector::ToggleModelSelector; @@ -31,8 +31,8 @@ use crate::profile_selector::ProfileSelector; use crate::thread::{RequestKind, Thread}; use crate::thread_store::ThreadStore; use crate::{ - AssistantDiff, Chat, ChatMode, RemoveAllContext, ThreadEvent, ToggleContextPicker, - ToggleProfileSelector, + AssistantDiff, Chat, ChatMode, OpenAssistantDiff, RemoveAllContext, ThreadEvent, + ToggleContextPicker, ToggleProfileSelector, }; pub struct MessageEditor { @@ -331,7 +331,11 @@ impl Render for MessageEditor { let action_log = self.thread.read(cx).action_log(); let changed_buffers = action_log.read(cx).changed_buffers(cx); let changed_buffers_count = changed_buffers.len(); + let editor_bg_color = cx.theme().colors().editor_background; + let border_color = cx.theme().colors().border; + let active_color = cx.theme().colors().element_selected; + let bg_edit_files_disclosure = editor_bg_color.blend(active_color.opacity(0.3)); v_flex() .size_full() @@ -397,18 +401,27 @@ impl Render for MessageEditor { parent.child( v_flex() .mx_2() - .bg(cx.theme().colors().element_background) + .bg(bg_edit_files_disclosure) .border_1() .border_b_0() - .border_color(cx.theme().colors().border) + .border_color(border_color) .rounded_t_md() + .shadow(smallvec::smallvec![gpui::BoxShadow { + color: gpui::black().opacity(0.15), + offset: point(px(1.), px(-1.)), + blur_radius: px(3.), + spread_radius: px(0.), + }]) .child( h_flex() - .p_2() + .p_1p5() .justify_between() + .when(self.edits_expanded, |this| { + this.border_b_1().border_color(border_color) + }) .child( h_flex() - .gap_2() + .gap_1() .child( Disclosure::new( "edits-disclosure", @@ -423,7 +436,7 @@ impl Render for MessageEditor { ) .child( Label::new("Edits") - .size(LabelSize::XSmall) + .size(LabelSize::Small) .color(Color::Muted), ) .child( @@ -441,13 +454,22 @@ impl Render for MessageEditor { "files" } )) - .size(LabelSize::XSmall) + .size(LabelSize::Small) .color(Color::Muted), ), ) .child( - Button::new("review", "Review") - .label_size(LabelSize::XSmall) + Button::new("review", "Review Changes") + .label_size(LabelSize::Small) + .key_binding( + KeyBinding::for_action_in( + &OpenAssistantDiff, + &focus_handle, + window, + cx, + ) + .map(|kb| kb.size(rems_from_px(12.))), + ) .on_click(cx.listener(|this, _, window, cx| { this.handle_review_click(window, cx) })), @@ -474,50 +496,94 @@ impl Render for MessageEditor { std::path::MAIN_SEPARATOR_STR )) .color(Color::Muted) - .size(LabelSize::Small), + .size(LabelSize::XSmall) + .buffer_font(cx), ) } }); let name_label = path.file_name().map(|name| { Label::new(name.to_string_lossy().to_string()) - .size(LabelSize::Small) + .size(LabelSize::XSmall) + .buffer_font(cx) }); let file_icon = FileIcons::get_icon(&path, cx) .map(Icon::from_path) - .unwrap_or_else(|| Icon::new(IconName::File)); + .map(|icon| { + icon.color(Color::Muted).size(IconSize::Small) + }) + .unwrap_or_else(|| { + Icon::new(IconName::File) + .color(Color::Muted) + .size(IconSize::Small) + }); let element = div() - .p_2() + .relative() + .py_1() + .px_2() .when(index + 1 < changed_buffers_count, |parent| { - parent - .border_color(cx.theme().colors().border) - .border_b_1() + parent.border_color(border_color).border_b_1() }) .child( h_flex() .gap_2() - .child(file_icon) + .justify_between() .child( - // TODO: handle overflow h_flex() - .children(parent_label) - .children(name_label), - ) - // TODO: show lines changed - .child( - Label::new("+").color(Color::Created), - ) - .child( - Label::new("-").color(Color::Deleted), + .id("file-container") + .pr_8() + .gap_1p5() + .max_w_full() + .overflow_x_scroll() + .child(file_icon) + .child( + h_flex() + .children(parent_label) + .children(name_label), + ) // TODO: show lines changed + .child( + Label::new("+") + .color(Color::Created), + ) + .child( + Label::new("-") + .color(Color::Deleted), + ), ) .when(!changed.needs_review, |parent| { parent.child( Icon::new(IconName::Check) .color(Color::Success), ) - }), + }) + .child( + div() + .h_full() + .absolute() + .w_8() + .bottom_0() + .map(|this| { + if !changed.needs_review { + this.right_4() + } else { + this.right_0() + } + }) + .bg(linear_gradient( + 90., + linear_color_stop( + editor_bg_color, + 1., + ), + linear_color_stop( + editor_bg_color + .opacity(0.2), + 0., + ), + )), + ), ); Some(element)