Improve design for the diff review button (#46723)

Danilo Leal created

Follow up to https://github.com/zed-industries/zed/pull/46326.

Release Notes:

- N/A

Change summary

crates/editor/src/element.rs | 108 +++++++++++++++++++++----------------
1 file changed, 61 insertions(+), 47 deletions(-)

Detailed changes

crates/editor/src/element.rs 🔗

@@ -3031,7 +3031,6 @@ impl EditorElement {
         scroll_position: gpui::Point<ScrollOffset>,
         gutter_dimensions: &GutterDimensions,
         gutter_hitbox: &Hitbox,
-        display_hunks: &[(DisplayDiffHunk, Option<Hitbox>)],
         snapshot: &EditorSnapshot,
         breakpoints: HashMap<DisplayRow, (Anchor, Breakpoint, Option<BreakpointSessionState>)>,
         row_infos: &[RowInfo],
@@ -3073,7 +3072,6 @@ impl EditorElement {
                         gutter_dimensions,
                         scroll_position,
                         gutter_hitbox,
-                        display_hunks,
                         window,
                         cx,
                     );
@@ -3088,7 +3086,7 @@ impl EditorElement {
         range: Range<DisplayRow>,
         row_infos: &[RowInfo],
         cx: &App,
-    ) -> Option<DisplayRow> {
+    ) -> Option<(DisplayRow, Option<u32>)> {
         if !cx.has_flag::<DiffReviewFeatureFlag>() {
             return None;
         }
@@ -3104,25 +3102,34 @@ impl EditorElement {
         }
 
         let display_row = indicator.display_row;
+        let row_index = (display_row.0.saturating_sub(range.start.0)) as usize;
 
         // Don't show on rows with expand excerpt buttons
-        if row_infos
-            .get((display_row.0.saturating_sub(range.start.0)) as usize)
-            .is_some_and(|row_info| row_info.expand_info.is_some())
-        {
+        let row_info = row_infos.get(row_index);
+        if row_info.is_some_and(|row_info| row_info.expand_info.is_some()) {
             return None;
         }
 
-        Some(display_row)
+        let buffer_row = row_info.and_then(|info| info.buffer_row);
+        Some((display_row, buffer_row))
     }
 
-    fn diff_review_button() -> AnyElement {
-        IconButton::new("diff_review_button", ui::IconName::Plus)
-            .icon_size(ui::IconSize::XSmall)
-            .size(ui::ButtonSize::None)
-            .icon_color(ui::Color::Default)
-            .style(ui::ButtonStyle::Filled)
-            .layer(ui::ElevationIndex::Surface)
+    fn diff_review_button(width: Pixels, cx: &mut App) -> AnyElement {
+        let text_c = cx.theme().colors().text;
+        let icon_c = cx.theme().colors().icon_accent;
+
+        h_flex()
+            .id("diff_review_button")
+            .cursor_pointer()
+            .w(width - px(1.))
+            .h(relative(0.9))
+            .justify_center()
+            .rounded_sm()
+            .border_1()
+            .border_color(text_c.opacity(0.1))
+            .bg(text_c.opacity(0.15))
+            .hover(|s| s.bg(icon_c.opacity(0.4)).border_color(icon_c.opacity(0.5)))
+            .child(Icon::new(IconName::Plus).size(IconSize::Small))
             .tooltip(Tooltip::text("Add Review"))
             .into_any_element()
     }
@@ -3136,7 +3143,6 @@ impl EditorElement {
         scroll_position: gpui::Point<ScrollOffset>,
         gutter_dimensions: &GutterDimensions,
         gutter_hitbox: &Hitbox,
-        display_hunks: &[(DisplayDiffHunk, Option<Hitbox>)],
         snapshot: &EditorSnapshot,
         breakpoints: &mut HashMap<DisplayRow, (Anchor, Breakpoint, Option<BreakpointSessionState>)>,
         window: &mut Window,
@@ -3229,7 +3235,6 @@ impl EditorElement {
                         gutter_dimensions,
                         scroll_position,
                         gutter_hitbox,
-                        display_hunks,
                         window,
                         cx,
                     );
@@ -3298,14 +3303,13 @@ impl EditorElement {
                     || gutter_dimensions.right_padding == px(0.);
 
                 let width = if is_wide {
-                    available_width - px(2.)
+                    available_width - px(5.)
                 } else {
-                    available_width + em_width - px(2.)
+                    available_width + em_width - px(5.)
                 };
 
                 let toggle = IconButton::new(("expand", ix), icon_name)
                     .icon_color(Color::Custom(cx.theme().colors().editor_line_number))
-                    .selected_icon_color(Color::Custom(cx.theme().colors().editor_foreground))
                     .icon_size(IconSize::Custom(rems(editor_font_size / window.rem_size())))
                     .width(width)
                     .on_click(move |_, window, cx| {
@@ -8354,7 +8358,6 @@ fn prepaint_gutter_button(
     gutter_dimensions: &GutterDimensions,
     scroll_position: gpui::Point<ScrollOffset>,
     gutter_hitbox: &Hitbox,
-    display_hunks: &[(DisplayDiffHunk, Option<Hitbox>)],
     window: &mut Window,
     cx: &mut App,
 ) -> AnyElement {
@@ -8363,28 +8366,12 @@ fn prepaint_gutter_button(
         AvailableSpace::Definite(line_height),
     );
     let indicator_size = button.layout_as_root(available_space, window, cx);
+    let git_gutter_width = EditorElement::gutter_strip_width(line_height)
+        + gutter_dimensions
+            .git_blame_entries_width
+            .unwrap_or_default();
 
-    let blame_width = gutter_dimensions.git_blame_entries_width;
-    let gutter_width = display_hunks
-        .binary_search_by(|(hunk, _)| match hunk {
-            DisplayDiffHunk::Folded { display_row } => display_row.cmp(&row),
-            DisplayDiffHunk::Unfolded {
-                display_row_range, ..
-            } => {
-                if display_row_range.end <= row {
-                    Ordering::Less
-                } else if display_row_range.start > row {
-                    Ordering::Greater
-                } else {
-                    Ordering::Equal
-                }
-            }
-        })
-        .ok()
-        .and_then(|ix| Some(display_hunks[ix].1.as_ref()?.size.width));
-    let left_offset = blame_width.max(gutter_width).unwrap_or_default();
-
-    let x = left_offset;
+    let x = git_gutter_width + px(2.);
 
     let mut y =
         Pixels::from((row.as_f64() - scroll_position.y) * ScrollPixelOffset::from(line_height));
@@ -10409,7 +10396,6 @@ impl Element for EditorElement {
                             scroll_position,
                             &gutter_dimensions,
                             &gutter_hitbox,
-                            &display_hunks,
                             &snapshot,
                             &mut breakpoint_rows,
                             window,
@@ -10429,7 +10415,6 @@ impl Element for EditorElement {
                             scroll_position,
                             &gutter_dimensions,
                             &gutter_hitbox,
-                            &display_hunks,
                             &snapshot,
                             breakpoint_rows,
                             &row_infos,
@@ -10440,17 +10425,46 @@ impl Element for EditorElement {
                         Vec::new()
                     };
 
+                    let git_gutter_width = Self::gutter_strip_width(line_height)
+                        + gutter_dimensions
+                            .git_blame_entries_width
+                            .unwrap_or_default();
+                    let available_width = gutter_dimensions.left_padding - git_gutter_width;
+
+                    let max_line_number_length = self
+                        .editor
+                        .read(cx)
+                        .buffer()
+                        .read(cx)
+                        .snapshot(cx)
+                        .widest_line_number()
+                        .ilog10()
+                        + 1;
+
                     let diff_review_button = self
                         .should_render_diff_review_button(start_row..end_row, &row_infos, cx)
-                        .map(|display_row| {
+                        .map(|(display_row, buffer_row)| {
+                            let is_wide = max_line_number_length
+                                >= EditorSettings::get_global(cx).gutter.min_line_number_digits
+                                    as u32
+                                && buffer_row.is_some_and(|row| {
+                                    (row + 1).ilog10() + 1 == max_line_number_length
+                                })
+                                || gutter_dimensions.right_padding == px(0.);
+
+                            let button_width = if is_wide {
+                                available_width - px(6.)
+                            } else {
+                                available_width + em_width - px(6.)
+                            };
+
                             prepaint_gutter_button(
-                                Self::diff_review_button(),
+                                Self::diff_review_button(button_width, cx),
                                 display_row,
                                 line_height,
                                 &gutter_dimensions,
                                 scroll_position,
                                 &gutter_hitbox,
-                                &display_hunks,
                                 window,
                                 cx,
                             )