Layout gutter hunk diff close button (X) better (#15178)

Closes https://github.com/zed-industries/zed/issues/15164

* Deleted hunk
Before:

![before_top](https://github.com/user-attachments/assets/27c72ee5-719f-4787-b222-4f597840936a)

After:

![after_top](https://github.com/user-attachments/assets/58095b21-698d-4778-8412-b680d5253e2c)

* Modified hunk
Before:

![before_down](https://github.com/user-attachments/assets/3fffb73b-7101-493c-b63b-15c3ccaf5362)

After:

![after_down](https://github.com/user-attachments/assets/f07a05ed-1260-4e2a-9388-c9cb93020d78)

* Added hunk
Before:

![before_mid](https://github.com/user-attachments/assets/0dd3f0f9-51e8-449a-bdd7-4c3ba5cd7791)

After:

![after_mid](https://github.com/user-attachments/assets/92f749bd-2d95-4650-8334-1d5c38c6aeb6)

Release Notes:

- N/A

crates/editor/src/editor.rs

@@ -594,7 +594,7 @@ pub struct EditorSnapshot {
 
 const GIT_BLAME_GUTTER_WIDTH_CHARS: f32 = 53.;
 
-#[derive(Debug, Clone, Copy)]
+#[derive(Default, Debug, Clone, Copy)]
 pub struct GutterDimensions {
     pub left_padding: Pixels,
     pub right_padding: Pixels,
@@ -617,18 +617,6 @@ impl GutterDimensions {
     }
 }
 
-impl Default for GutterDimensions {
-    fn default() -> Self {
-        Self {
-            left_padding: Pixels::ZERO,
-            right_padding: Pixels::ZERO,
-            width: Pixels::ZERO,
-            margin: Pixels::ZERO,
-            git_blame_entries_width: None,
-        }
-    }
-}
-
 #[derive(Debug)]
 pub struct RemoteSelection {
     pub replica_id: ReplicaId,
@@ -5167,7 +5155,7 @@ impl Editor {
             }))
     }
 
-    fn render_close_hunk_diff_button(
+    fn close_hunk_diff_button(
         &self,
         hunk: HoveredHunk,
         row: DisplayRow,
@@ -11854,6 +11842,11 @@ impl Editor {
         let source_y = line_height * (source.row() - first_visible_line.row()).0 as f32;
         Some(gpui::Point::new(source_x, source_y))
     }
+
+    fn gutter_bounds(&self) -> Option<Bounds<Pixels>> {
+        let bounds = self.last_bounds?;
+        Some(element::gutter_bounds(bounds, self.gutter_dimensions))
+    }
 }
 
 fn hunks_for_selections(
@@ -12240,7 +12233,7 @@ impl EditorSnapshot {
         self.scroll_anchor.scroll_position(&self.display_snapshot)
     }
 
-    pub fn gutter_dimensions(
+    fn gutter_dimensions(
         &self,
         font_id: FontId,
         font_size: Pixels,

crates/editor/src/element.rs

@@ -1248,7 +1248,7 @@ impl EditorElement {
 
     // Folds contained in a hunk are ignored apart from shrinking visual size
     // If a fold contains any hunks then that fold line is marked as modified
-    fn layout_git_gutters(
+    fn layout_gutter_git_hunks(
         &self,
         line_height: Pixels,
         gutter_hitbox: &Hitbox,
@@ -1553,12 +1553,14 @@ impl EditorElement {
         (offset_y, length)
     }
 
+    #[allow(clippy::too_many_arguments)]
     fn layout_run_indicators(
         &self,
         line_height: Pixels,
         scroll_pixel_position: gpui::Point<Pixels>,
         gutter_dimensions: &GutterDimensions,
         gutter_hitbox: &Hitbox,
+        rows_with_hunk_bounds: &HashMap<DisplayRow, Bounds<Pixels>>,
         snapshot: &EditorSnapshot,
         cx: &mut WindowContext,
     ) -> Vec<AnyElement> {
@@ -1602,6 +1604,7 @@ impl EditorElement {
                         gutter_dimensions,
                         scroll_pixel_position,
                         gutter_hitbox,
+                        rows_with_hunk_bounds,
                         cx,
                     );
                     Some(button)
@@ -1610,6 +1613,7 @@ impl EditorElement {
         })
     }
 
+    #[allow(clippy::too_many_arguments)]
     fn layout_code_actions_indicator(
         &self,
         line_height: Pixels,
@@ -1617,6 +1621,7 @@ impl EditorElement {
         scroll_pixel_position: gpui::Point<Pixels>,
         gutter_dimensions: &GutterDimensions,
         gutter_hitbox: &Hitbox,
+        rows_with_hunk_bounds: &HashMap<DisplayRow, Bounds<Pixels>>,
         cx: &mut WindowContext,
     ) -> Option<AnyElement> {
         let mut active = false;
@@ -1640,6 +1645,7 @@ impl EditorElement {
             gutter_dimensions,
             scroll_pixel_position,
             gutter_hitbox,
+            rows_with_hunk_bounds,
             cx,
         );
 
@@ -3170,7 +3176,7 @@ impl EditorElement {
         });
     }
 
-    fn diff_hunk_bounds(
+    pub(super) fn diff_hunk_bounds(
         snapshot: &EditorSnapshot,
         line_height: Pixels,
         gutter_bounds: Bounds<Pixels>,
@@ -4040,20 +4046,22 @@ impl EditorElement {
         self.column_pixels(digit_count, cx)
     }
 
+    #[allow(clippy::too_many_arguments)]
     fn layout_hunk_diff_close_indicators(
         &self,
-        expanded_hunks_by_rows: HashMap<DisplayRow, ExpandedHunk>,
         line_height: Pixels,
         scroll_pixel_position: gpui::Point<Pixels>,
         gutter_dimensions: &GutterDimensions,
         gutter_hitbox: &Hitbox,
+        rows_with_hunk_bounds: &HashMap<DisplayRow, Bounds<Pixels>>,
+        expanded_hunks_by_rows: HashMap<DisplayRow, ExpandedHunk>,
         cx: &mut WindowContext,
     ) -> Vec<AnyElement> {
         self.editor.update(cx, |editor, cx| {
             expanded_hunks_by_rows
                 .into_iter()
                 .map(|(display_row, hunk)| {
-                    let button = editor.render_close_hunk_diff_button(
+                    let button = editor.close_hunk_diff_button(
                         HoveredHunk {
                             multi_buffer_range: hunk.hunk_range,
                             status: hunk.status,
@@ -4070,6 +4078,7 @@ impl EditorElement {
                         gutter_dimensions,
                         scroll_pixel_position,
                         gutter_hitbox,
+                        rows_with_hunk_bounds,
                         cx,
                     )
                 })
@@ -4078,6 +4087,7 @@ impl EditorElement {
     }
 }
 
+#[allow(clippy::too_many_arguments)]
 fn prepaint_gutter_button(
     button: IconButton,
     row: DisplayRow,
@@ -4085,6 +4095,7 @@ fn prepaint_gutter_button(
     gutter_dimensions: &GutterDimensions,
     scroll_pixel_position: gpui::Point<Pixels>,
     gutter_hitbox: &Hitbox,
+    rows_with_hunk_bounds: &HashMap<DisplayRow, Bounds<Pixels>>,
     cx: &mut WindowContext<'_>,
 ) -> AnyElement {
     let mut button = button.into_any_element();
@@ -4094,14 +4105,16 @@ fn prepaint_gutter_button(
     );
     let indicator_size = button.layout_as_root(available_space, cx);
 
-    let blame_width = gutter_dimensions
+    let blame_offset = gutter_dimensions.git_blame_entries_width;
-        .git_blame_entries_width
+    let gutter_offset = rows_with_hunk_bounds
-        .unwrap_or(Pixels::ZERO);
+        .get(&row)
+        .map(|bounds| bounds.origin.x + bounds.size.width);
+    let left_offset = blame_offset.max(gutter_offset).unwrap_or(Pixels::ZERO);
 
-    let mut x = blame_width;
+    let mut x = left_offset;
     let available_width = gutter_dimensions.margin + gutter_dimensions.left_padding
         - indicator_size.width
-        - blame_width;
+        - left_offset;
     x += available_width / 2.;
 
     let mut y = row.as_f32() * line_height - scroll_pixel_position.y;
@@ -4949,13 +4962,8 @@ impl Element for EditorElement {
                         .collect::<SmallVec<[_; 2]>>();
 
                     let hitbox = cx.insert_hitbox(bounds, false);
-                    let gutter_hitbox = cx.insert_hitbox(
+                    let gutter_hitbox =
-                        Bounds {
+                        cx.insert_hitbox(gutter_bounds(bounds, gutter_dimensions), false);
-                            origin: bounds.origin,
-                            size: size(gutter_dimensions.width, bounds.size.height),
-                        },
-                        false,
-                    );
                     let text_hitbox = cx.insert_hitbox(
                         Bounds {
                             origin: gutter_hitbox.upper_right(),
@@ -5078,7 +5086,7 @@ impl Element for EditorElement {
                         self.layout_crease_trailers(buffer_rows.iter().copied(), &snapshot, cx)
                     });
 
-                    let display_hunks = self.layout_git_gutters(
+                    let display_hunks = self.layout_gutter_git_hunks(
                         line_height,
                         &gutter_hitbox,
                         start_row..end_row,
@@ -5305,6 +5313,27 @@ impl Element for EditorElement {
                             .collect::<HashMap<_, _>>()
                     });
 
+                    let rows_with_hunk_bounds = display_hunks
+                        .iter()
+                        .filter_map(|(hunk, hitbox)| Some((hunk, hitbox.as_ref()?.bounds)))
+                        .fold(
+                            HashMap::default(),
+                            |mut rows_with_hunk_bounds, (hunk, bounds)| {
+                                match hunk {
+                                    DisplayDiffHunk::Folded { display_row } => {
+                                        rows_with_hunk_bounds.insert(*display_row, bounds);
+                                    }
+                                    DisplayDiffHunk::Unfolded {
+                                        display_row_range, ..
+                                    } => {
+                                        for display_row in display_row_range.iter_rows() {
+                                            rows_with_hunk_bounds.insert(display_row, bounds);
+                                        }
+                                    }
+                                }
+                                rows_with_hunk_bounds
+                            },
+                        );
                     let mut _context_menu_visible = false;
                     let mut code_actions_indicator = None;
                     if let Some(newest_selection_head) = newest_selection_head {
@@ -5353,6 +5382,7 @@ impl Element for EditorElement {
                                                     scroll_pixel_position,
                                                     &gutter_dimensions,
                                                     &gutter_hitbox,
+                                                    &rows_with_hunk_bounds,
                                                     cx,
                                                 );
                                         }
@@ -5368,6 +5398,7 @@ impl Element for EditorElement {
                             scroll_pixel_position,
                             &gutter_dimensions,
                             &gutter_hitbox,
+                            &rows_with_hunk_bounds,
                             &snapshot,
                             cx,
                         )
@@ -5376,11 +5407,12 @@ impl Element for EditorElement {
                     };
 
                     let close_indicators = self.layout_hunk_diff_close_indicators(
-                        expanded_add_hunks_by_rows,
                         line_height,
                         scroll_pixel_position,
                         &gutter_dimensions,
                         &gutter_hitbox,
+                        &rows_with_hunk_bounds,
+                        expanded_add_hunks_by_rows,
                         cx,
                     );
 
@@ -5591,6 +5623,16 @@ impl Element for EditorElement {
     }
 }
 
+pub(super) fn gutter_bounds(
+    editor_bounds: Bounds<Pixels>,
+    gutter_dimensions: GutterDimensions,
+) -> Bounds<Pixels> {
+    Bounds {
+        origin: editor_bounds.origin,
+        size: size(gutter_dimensions.width, editor_bounds.size.height),
+    }
+}
+
 impl IntoElement for EditorElement {
     type Element = Self;
 

crates/editor/src/hunk_diff.rs

@@ -5,7 +5,7 @@ use std::{
 
 use collections::{hash_map, HashMap, HashSet};
 use git::diff::{DiffHunk, DiffHunkStatus};
-use gpui::{Action, AppContext, Hsla, Model, MouseButton, Subscription, Task, View};
+use gpui::{Action, AppContext, CursorStyle, Hsla, Model, MouseButton, Subscription, Task, View};
 use language::Buffer;
 use multi_buffer::{
     Anchor, AnchorRangeExt, ExcerptRange, MultiBuffer, MultiBufferRow, MultiBufferSnapshot, ToPoint,
@@ -13,7 +13,7 @@ use multi_buffer::{
 use settings::SettingsStore;
 use text::{BufferId, Point};
 use ui::{
-    h_flex, v_flex, ActiveTheme, Context as _, ContextMenu, InteractiveElement, IntoElement,
+    div, h_flex, v_flex, ActiveTheme, Context as _, ContextMenu, InteractiveElement, IntoElement,
     ParentElement, Pixels, Styled, ViewContext, VisualContext,
 };
 use util::{debug_panic, RangeExt};
@@ -24,8 +24,8 @@ use crate::{
     hunk_status, hunks_for_selections,
     mouse_context_menu::MouseContextMenu,
     BlockDisposition, BlockProperties, BlockStyle, CustomBlockId, DiffRowHighlight, Editor,
-    EditorSnapshot, ExpandAllHunkDiffs, RangeToAnchorExt, RevertSelectedHunks, ToDisplayPoint,
+    EditorElement, EditorSnapshot, ExpandAllHunkDiffs, RangeToAnchorExt, RevertSelectedHunks,
-    ToggleHunkDiff,
+    ToDisplayPoint, ToggleHunkDiff,
 };
 
 #[derive(Debug, Clone)]
@@ -430,7 +430,6 @@ impl Editor {
         let (editor_height, editor_with_deleted_text) =
             editor_with_deleted_text(diff_base_buffer, deleted_hunk_color, hunk, cx);
         let editor = cx.view().clone();
-        let editor_model = cx.model().clone();
         let hunk = hunk.clone();
         let mut new_block_ids = self.insert_blocks(
             Some(BlockProperties {
@@ -439,37 +438,84 @@ impl Editor {
                 style: BlockStyle::Flex,
                 disposition: BlockDisposition::Above,
                 render: Box::new(move |cx| {
-                    let close_button = editor.update(cx.context, |editor, cx| {
+                    let Some(gutter_bounds) = editor.read(cx).gutter_bounds() else {
-                        let editor_snapshot = editor.snapshot(cx);
+                        return div().into_any_element();
-                        let hunk_start_row = hunk
+                    };
-                            .multi_buffer_range
+                    let (gutter_dimensions, hunk_bounds, close_button) =
-                            .start
+                        editor.update(cx.context, |editor, cx| {
-                            .to_display_point(&editor_snapshot)
+                            let editor_snapshot = editor.snapshot(cx);
-                            .row();
+                            let hunk_display_range = hunk
-                        editor.render_close_hunk_diff_button(hunk.clone(), hunk_start_row, cx)
+                                .multi_buffer_range
-                    });
+                                .clone()
-                    let gutter_dimensions = editor_model.read(cx).gutter_dimensions;
+                                .to_display_points(&editor_snapshot);
+                            let gutter_dimensions = editor.gutter_dimensions;
+                            let hunk_bounds = EditorElement::diff_hunk_bounds(
+                                &editor_snapshot,
+                                cx.line_height(),
+                                gutter_bounds,
+                                &DisplayDiffHunk::Unfolded {
+                                    diff_base_byte_range: hunk.diff_base_byte_range.clone(),
+                                    multi_buffer_range: hunk.multi_buffer_range.clone(),
+                                    display_row_range: hunk_display_range.start.row()
+                                        ..hunk_display_range.end.row(),
+                                    status: hunk.status,
+                                },
+                            );
+
+                            let close_button = editor.close_hunk_diff_button(
+                                hunk.clone(),
+                                hunk_display_range.start.row(),
+                                cx,
+                            );
+                            (gutter_dimensions, hunk_bounds, close_button)
+                        });
                     let click_editor = editor.clone();
+                    let clicked_hunk = hunk.clone();
                     h_flex()
+                        .id("gutter with editor")
                         .bg(deleted_hunk_color)
                         .size_full()
                         .child(
-                            v_flex()
+                            h_flex()
+                                .id("gutter")
                                 .max_w(gutter_dimensions.full_width())
                                 .min_w(gutter_dimensions.full_width())
                                 .size_full()
-                                .on_mouse_down(MouseButton::Left, {
+                                .child(
-                                    let click_hunk = hunk.clone();
+                                    h_flex()
-                                    move |e, cx| {
+                                        .id("gutter hunk")
-                                        let modifiers = e.modifiers;
+                                        .pl(hunk_bounds.origin.x)
-                                        if modifiers.control || modifiers.platform {
+                                        .max_w(hunk_bounds.size.width)
-                                            click_editor.update(cx, |editor, cx| {
+                                        .min_w(hunk_bounds.size.width)
-                                                editor.toggle_hovered_hunk(&click_hunk, cx);
+                                        .size_full()
-                                            });
+                                        .cursor(CursorStyle::PointingHand)
-                                        }
+                                        .on_mouse_down(MouseButton::Left, {
-                                    }
+                                            let click_hunk = hunk.clone();
-                                })
+                                            move |e, cx| {
-                                .child(close_button),
+                                                let modifiers = e.modifiers;
+                                                if modifiers.control || modifiers.platform {
+                                                    click_editor.update(cx, |editor, cx| {
+                                                        editor.toggle_hovered_hunk(&click_hunk, cx);
+                                                    });
+                                                } else {
+                                                    click_editor.update(cx, |editor, cx| {
+                                                        editor.open_hunk_context_menu(
+                                                            clicked_hunk.clone(),
+                                                            e.position,
+                                                            cx,
+                                                        );
+                                                    });
+                                                }
+                                            }
+                                        }),
+                                )
+                                .child(
+                                    v_flex()
+                                        .size_full()
+                                        .pt(ui::rems(0.25))
+                                        .justify_start()
+                                        .child(close_button),
+                                ),
                         )
                         .child(editor_with_deleted_text.clone())
                         .into_any_element()