assistant2: Adjust edit files design (#27762)

Danilo Leal created

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.

<img
src="https://github.com/user-attachments/assets/4bff883a-c5c4-443e-8bf5-d98f535c83ce"
width="750" />

Release Notes:

- N/A

Change summary

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(-)

Detailed changes

Cargo.lock 🔗

@@ -497,6 +497,7 @@ dependencies = [
  "serde",
  "serde_json",
  "settings",
+ "smallvec",
  "smol",
  "streaming_diff",
  "telemetry",

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"
     }
   },
   {

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"
     }
   },
   {

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

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()

crates/assistant2/src/assistant.rs 🔗

@@ -65,6 +65,7 @@ actions!(
         RemoveFocusedContext,
         AcceptSuggestedContext,
         OpenActiveThreadAsMarkdown,
+        OpenAssistantDiff,
         ToggleKeep,
         Reject,
         RejectAll,

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<Self>) -> 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<Self>) -> 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(

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::<AssistantPanel>(window, cx);
                         panel.update(cx, |panel, cx| panel.open_configuration(window, cx));
                     }
+                })
+                .register_action(|workspace, _: &OpenAssistantDiff, window, cx| {
+                    if let Some(panel) = workspace.panel::<AssistantPanel>(cx) {
+                        workspace.focus_panel::<AssistantPanel>(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<Self>,
+    ) {
+        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<Self>) {
         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

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)