git_ui: Commit modal editor cleanup (#25645)

Nate Butler created

- Fixes cursor style in the commit modal
- Use commit button instead of kb hint
- Update layout to scale better for large commit messages

No message:

![CleanShot 2025-02-26 at 10 33
22@2x](https://github.com/user-attachments/assets/fb6fc4ff-1e1d-46ae-aee0-7e21d73aaf70)

Long Message:
![CleanShot 2025-02-26 at 10 33
55@2x](https://github.com/user-attachments/assets/71a7773c-386b-47d5-abed-9a301360c35b)

![CleanShot 2025-02-26 at 10 34
06@2x](https://github.com/user-attachments/assets/dc41f0b9-2277-4034-9af2-d77fec0488d8)

Release Notes:

- N/A

Change summary

crates/git_ui/src/commit_modal.rs | 438 ++++++++++----------------------
crates/git_ui/src/git_panel.rs    |   4 
2 files changed, 148 insertions(+), 294 deletions(-)

Detailed changes

crates/git_ui/src/commit_modal.rs 🔗

@@ -1,28 +1,57 @@
-#![allow(unused, dead_code)]
+// #![allow(unused, dead_code)]
 
 use crate::git_panel::{commit_message_editor, GitPanel};
-use crate::repository_selector::RepositorySelector;
-use anyhow::Result;
 use git::Commit;
-use language::language_settings::LanguageSettings;
-use language::Buffer;
-use panel::{
-    panel_button, panel_editor_container, panel_editor_style, panel_filled_button,
-    panel_icon_button,
-};
-use settings::Settings;
-use theme::ThemeSettings;
+use panel::{panel_button, panel_editor_style, panel_filled_button};
 use ui::{prelude::*, KeybindingHint, Tooltip};
 
-use editor::{Direction, Editor, EditorElement, EditorMode, EditorSettings, MultiBuffer};
+use editor::{Editor, EditorElement};
 use gpui::*;
-use project::git::Repository;
-use project::{Fs, Project};
-use std::sync::Arc;
-use workspace::dock::{Dock, DockPosition, PanelHandle};
-use workspace::{ModalView, Workspace};
+use util::ResultExt;
+use workspace::{
+    dock::{Dock, PanelHandle},
+    ModalView, Workspace,
+};
 
-// actions!(commit_modal, [NextSuggestion, PrevSuggestion]);
+// nate: It is a pain to get editors to size correctly and not overflow.
+//
+// this can get replaced with a simple flex layout with more time/a more thoughtful approach.
+#[derive(Debug, Clone, Copy)]
+pub struct ModalContainerProperties {
+    pub modal_width: f32,
+    pub editor_height: f32,
+    pub footer_height: f32,
+    pub container_padding: f32,
+    pub modal_border_radius: f32,
+}
+
+impl ModalContainerProperties {
+    pub fn new(window: &Window, preferred_char_width: usize) -> Self {
+        let container_padding = 5.0;
+
+        // Calculate width based on character width
+        let mut modal_width = 460.0;
+        let style = window.text_style().clone();
+        let font_id = window.text_system().resolve_font(&style.font());
+        let font_size = style.font_size.to_pixels(window.rem_size());
+
+        if let Ok(em_width) = window.text_system().em_width(font_id, font_size) {
+            modal_width = preferred_char_width as f32 * em_width.0 + (container_padding * 2.0);
+        }
+
+        Self {
+            modal_width,
+            editor_height: 300.0,
+            footer_height: 24.0,
+            container_padding,
+            modal_border_radius: 12.0,
+        }
+    }
+
+    pub fn editor_border_radius(&self) -> Pixels {
+        px(self.modal_border_radius - self.container_padding / 2.0)
+    }
+}
 
 pub fn init(cx: &mut App) {
     cx.observe_new(|workspace: &mut Workspace, window, cx| {
@@ -38,8 +67,7 @@ pub struct CommitModal {
     git_panel: Entity<GitPanel>,
     commit_editor: Entity<Editor>,
     restore_dock: RestoreDock,
-    current_suggestion: Option<usize>,
-    suggested_messages: Vec<SharedString>,
+    properties: ModalContainerProperties,
 }
 
 impl Focusable for CommitModal {
@@ -58,12 +86,15 @@ impl ModalView for CommitModal {
         self.git_panel.update(cx, |git_panel, cx| {
             git_panel.set_modal_open(false, cx);
         });
-        self.restore_dock.dock.update(cx, |dock, cx| {
-            if let Some(active_index) = self.restore_dock.active_index {
-                dock.activate_panel(active_index, window, cx)
-            }
-            dock.set_open(self.restore_dock.is_open, window, cx)
-        });
+        self.restore_dock
+            .dock
+            .update(cx, |dock, cx| {
+                if let Some(active_index) = self.restore_dock.active_index {
+                    dock.activate_panel(active_index, window, cx)
+                }
+                dock.set_open(self.restore_dock.is_open, window, cx)
+            })
+            .log_err();
         workspace::DismissDecision::Dismiss(true)
     }
 }
@@ -75,13 +106,13 @@ struct RestoreDock {
 }
 
 impl CommitModal {
-    pub fn register(workspace: &mut Workspace, _: &mut Window, cx: &mut Context<Workspace>) {
+    pub fn register(workspace: &mut Workspace, _: &mut Window, _cx: &mut Context<Workspace>) {
         workspace.register_action(|workspace, _: &Commit, window, cx| {
             let Some(git_panel) = workspace.panel::<GitPanel>(cx) else {
                 return;
             };
 
-            let (can_commit, conflict) = git_panel.update(cx, |git_panel, cx| {
+            let (can_commit, conflict) = git_panel.update(cx, |git_panel, _cx| {
                 let can_commit = git_panel.can_commit();
                 let conflict = git_panel.has_unstaged_conflicts();
                 (can_commit, conflict)
@@ -151,145 +182,56 @@ impl CommitModal {
 
         let focus_handle = commit_editor.focus_handle(cx);
 
-        cx.on_focus_out(&focus_handle, window, |this, _, window, cx| {
+        cx.on_focus_out(&focus_handle, window, |_, _, _, cx| {
             cx.emit(DismissEvent);
         })
         .detach();
 
+        let properties = ModalContainerProperties::new(window, 50);
+
         Self {
             git_panel,
             commit_editor,
             restore_dock,
-            current_suggestion: None,
-            suggested_messages: vec![],
-        }
-    }
-
-    /// Returns container `(width, x padding, border radius)`
-    fn container_properties(&self, window: &mut Window, cx: &mut Context<Self>) -> (f32, f32, f32) {
-        // TODO: Let's set the width based on your set wrap guide if possible
-
-        // let settings = EditorSettings::get_global(cx);
-
-        // let first_wrap_guide = self
-        //     .commit_editor
-        //     .read(cx)
-        //     .wrap_guides(cx)
-        //     .iter()
-        //     .next()
-        //     .map(|(guide, active)| if *active { Some(*guide) } else { None })
-        //     .flatten();
-
-        // let preferred_width = if let Some(guide) = first_wrap_guide {
-        //     guide
-        // } else {
-        //     80
-        // };
-
-        let border_radius = 16.0;
-
-        let preferred_width = 50; // (chars wide)
-
-        let mut width = 460.0;
-        let padding_x = 16.0;
-
-        let mut snapshot = self
-            .commit_editor
-            .update(cx, |editor, cx| editor.snapshot(window, cx));
-        let style = window.text_style().clone();
-
-        let font_id = window.text_system().resolve_font(&style.font());
-        let font_size = style.font_size.to_pixels(window.rem_size());
-        let line_height = style.line_height_in_pixels(window.rem_size());
-        if let Ok(em_width) = window.text_system().em_width(font_id, font_size) {
-            width = preferred_width as f32 * em_width.0 + (padding_x * 2.0);
-            cx.notify();
+            properties,
         }
-
-        // cx.notify();
-
-        (width, padding_x, border_radius)
     }
 
-    // fn cycle_suggested_messages(&mut self, direction: Direction, cx: &mut Context<Self>) {
-    //     let new_index = match direction {
-    //         Direction::Next => {
-    //             (self.current_suggestion.unwrap_or(0) + 1).rem_euclid(self.suggested_messages.len())
-    //         }
-    //         Direction::Prev => {
-    //             (self.current_suggestion.unwrap_or(0) + self.suggested_messages.len() - 1)
-    //                 .rem_euclid(self.suggested_messages.len())
-    //         }
-    //     };
-    //     self.current_suggestion = Some(new_index);
-
-    //     cx.notify();
-    // }
-
-    // fn next_suggestion(&mut self, _: &NextSuggestion, window: &mut Window, cx: &mut Context<Self>) {
-    //     self.current_suggestion = Some(1);
-    //     self.apply_suggestion(window, cx);
-    // }
-
-    // fn prev_suggestion(&mut self, _: &PrevSuggestion, window: &mut Window, cx: &mut Context<Self>) {
-    //     self.current_suggestion = Some(0);
-    //     self.apply_suggestion(window, cx);
-    // }
-
-    // fn set_commit_message(&mut self, message: &str, window: &mut Window, cx: &mut Context<Self>) {
-    //     self.commit_editor.update(cx, |editor, cx| {
-    //         editor.set_text(message.to_string(), window, cx)
-    //     });
-    //     self.current_suggestion = Some(0);
-    //     cx.notify();
-    // }
-
-    // fn apply_suggestion(&mut self, window: &mut Window, cx: &mut Context<Self>) {
-    //     let suggested_messages = self.suggested_messages.clone();
-
-    //     if let Some(suggestion) = self.current_suggestion {
-    //         let suggested_message = &suggested_messages[suggestion];
-
-    //         self.set_commit_message(suggested_message, window, cx);
-    //     }
-
-    //     cx.notify();
-    // }
-
     fn commit_editor_element(&self, window: &mut Window, cx: &mut Context<Self>) -> EditorElement {
-        let mut editor = self.commit_editor.clone();
-
         let editor_style = panel_editor_style(true, window, cx);
-
         EditorElement::new(&self.commit_editor, editor_style)
     }
 
     pub fn render_commit_editor(
         &self,
-        name_and_email: Option<(SharedString, SharedString)>,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> impl IntoElement {
-        let (width, padding_x, modal_border_radius) = self.container_properties(window, cx);
-
-        let border_radius = modal_border_radius - padding_x / 2.0;
-
-        let editor = self.commit_editor.clone();
-        let editor_focus_handle = editor.focus_handle(cx);
-
-        let settings = ThemeSettings::get_global(cx);
-        let line_height = relative(settings.buffer_line_height.value())
-            .to_pixels(settings.buffer_font_size(cx).into(), window.rem_size());
+        let properties = self.properties;
+        let padding_t = 3.0;
+        let padding_b = 6.0;
+        // magic number for editor not to overflow the container??
+        let extra_space_hack = 1.5 * window.line_height();
 
-        let mut snapshot = self
-            .commit_editor
-            .update(cx, |editor, cx| editor.snapshot(window, cx));
-        let style = window.text_style().clone();
+        v_flex()
+            .h(px(properties.editor_height + padding_b + padding_t) + extra_space_hack)
+            .w_full()
+            .flex_none()
+            .rounded(properties.editor_border_radius())
+            .overflow_hidden()
+            .px_1p5()
+            .pt(px(padding_t))
+            .pb(px(padding_b))
+            .child(
+                div()
+                    .h(px(properties.editor_height))
+                    .w_full()
+                    .child(self.commit_editor_element(window, cx)),
+            )
+    }
 
-        let font_id = window.text_system().resolve_font(&style.font());
-        let font_size = style.font_size.to_pixels(window.rem_size());
-        let line_height = style.line_height_in_pixels(window.rem_size());
-        let em_width = window.text_system().em_width(font_id, font_size);
+    fn render_footer(&self, window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
+        let git_panel = self.git_panel.clone();
 
         let (branch, tooltip, commit_label, co_authors) =
             self.git_panel.update(cx, |git_panel, cx| {
@@ -327,8 +269,6 @@ impl CommitModal {
             }))
             .style(ButtonStyle::Transparent);
 
-        let changes_count = self.git_panel.read(cx).total_staged_count();
-
         let close_kb_hint =
             if let Some(close_kb) = ui::KeyBinding::for_action(&menu::Cancel, window, cx) {
                 Some(
@@ -339,151 +279,44 @@ impl CommitModal {
                 None
             };
 
-        let fake_commit_kb =
-            ui::KeyBinding::new(gpui::KeyBinding::new("cmd-enter", gpui::NoAction, None), cx);
-
-        let commit_hint =
-            KeybindingHint::new(fake_commit_kb, cx.theme().colors().editor_background)
-                .suffix(commit_label);
-
-        let focus_handle = self.focus_handle(cx);
-
-        // let next_suggestion_kb =
-        //     ui::KeyBinding::for_action_in(&NextSuggestion, &focus_handle.clone(), window, cx);
-        // let next_suggestion_hint = next_suggestion_kb.map(|kb| {
-        //     KeybindingHint::new(kb, cx.theme().colors().editor_background).suffix("Next Suggestion")
-        // });
-
-        // let prev_suggestion_kb =
-        //     ui::KeyBinding::for_action_in(&PrevSuggestion, &focus_handle.clone(), window, cx);
-        // let prev_suggestion_hint = prev_suggestion_kb.map(|kb| {
-        //     KeybindingHint::new(kb, cx.theme().colors().editor_background)
-        //         .suffix("Previous Suggestion")
-        // });
-
-        v_flex()
-            .id("editor-container")
-            .bg(cx.theme().colors().editor_background)
-            .flex_1()
-            .size_full()
-            .rounded(px(border_radius))
-            .overflow_hidden()
-            .border_1()
-            .border_color(cx.theme().colors().border_variant)
-            .py_2()
-            .px_3()
-            .on_click(cx.listener(move |_, _: &ClickEvent, window, _cx| {
-                window.focus(&editor_focus_handle);
-            }))
-            .child(
-                div()
-                    .size_full()
-                    .flex_1()
-                    .child(self.commit_editor_element(window, cx)),
-            )
-            .child(
-                h_flex()
-                    .group("commit_editor_footer")
-                    .flex_none()
-                    .w_full()
-                    .items_center()
-                    .justify_between()
-                    .w_full()
-                    .pt_2()
-                    .pb_0p5()
-                    .gap_1()
-                    .child(h_flex().gap_1().child(branch_selector).children(co_authors))
-                    .child(div().flex_1())
-                    .child(
-                        h_flex()
-                            .opacity(0.7)
-                            .group_hover("commit_editor_footer", |this| this.opacity(1.0))
-                            .items_center()
-                            .justify_end()
-                            .flex_none()
-                            .px_1()
-                            .gap_4()
-                            .children(close_kb_hint)
-                            // .children(next_suggestion_hint)
-                            .child(commit_hint),
-                    ),
-            )
-    }
-
-    pub fn render_footer(&self, window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
-        let (branch, tooltip, title, co_authors) = self.git_panel.update(cx, |git_panel, cx| {
-            let branch = git_panel
-                .active_repository
-                .as_ref()
-                .and_then(|repo| {
-                    repo.read(cx)
-                        .repository_entry
-                        .branch()
-                        .map(|b| b.name.clone())
-                })
-                .unwrap_or_else(|| "<no branch>".into());
-            let tooltip = if git_panel.has_staged_changes() {
-                "Commit staged changes"
-            } else {
-                "Commit changes to tracked files"
-            };
-            let title = if git_panel.has_staged_changes() {
-                "Commit"
-            } else {
-                "Commit All"
-            };
-            let co_authors = git_panel.render_co_authors(cx);
-            (branch, tooltip, title, co_authors)
+        let (panel_editor_focus_handle, can_commit) = git_panel.update(cx, |git_panel, cx| {
+            (git_panel.editor_focus_handle(cx), git_panel.can_commit())
         });
 
-        let branch_selector = panel_button(branch)
-            .icon(IconName::GitBranch)
-            .icon_size(IconSize::Small)
-            .icon_color(Color::Muted)
-            .icon_position(IconPosition::Start)
-            .tooltip(Tooltip::for_action_title(
-                "Switch Branch",
-                &zed_actions::git::Branch,
-            ))
-            .on_click(cx.listener(|_, _, window, cx| {
-                window.dispatch_action(zed_actions::git::Branch.boxed_clone(), cx);
-            }))
-            .style(ButtonStyle::Transparent);
-
-        let changes_count = self.git_panel.read(cx).total_staged_count();
-
-        let close_kb_hint =
-            if let Some(close_kb) = ui::KeyBinding::for_action(&menu::Cancel, window, cx) {
-                Some(
-                    KeybindingHint::new(close_kb, cx.theme().colors().editor_background)
-                        .suffix("Cancel"),
-                )
-            } else {
-                None
-            };
+        let commit_button = panel_filled_button(commit_label)
+            .tooltip(move |window, cx| {
+                Tooltip::for_action_in(tooltip, &Commit, &panel_editor_focus_handle, window, cx)
+            })
+            .disabled(!can_commit)
+            .on_click(cx.listener(move |this, _: &ClickEvent, window, cx| {
+                this.git_panel
+                    .update(cx, |git_panel, cx| git_panel.commit_changes(window, cx));
+                cx.emit(DismissEvent);
+            }));
 
         h_flex()
-            .items_center()
-            .h(px(36.0))
+            .group("commit_editor_footer")
+            .flex_none()
             .w_full()
+            .items_center()
             .justify_between()
-            .px_3()
-            .child(h_flex().child(branch_selector))
+            .w_full()
+            .h(px(self.properties.footer_height))
+            .gap_1()
+            .child(h_flex().gap_1().child(branch_selector).children(co_authors))
+            .child(div().flex_1())
             .child(
-                h_flex().gap_1p5().children(co_authors).child(
-                    Button::new("stage-button", title)
-                        .tooltip(Tooltip::for_action_title(tooltip, &git::Commit))
-                        .on_click(cx.listener(|this, _, window, cx| {
-                            this.commit(&Default::default(), window, cx);
-                        })),
-                ),
+                h_flex()
+                    .items_center()
+                    .justify_end()
+                    .flex_none()
+                    .px_1()
+                    .gap_4()
+                    .children(close_kb_hint)
+                    .child(commit_button),
             )
     }
 
-    fn border_radius(&self) -> f32 {
-        8.0
-    }
-
     fn dismiss(&mut self, _: &menu::Cancel, _: &mut Window, cx: &mut Context<Self>) {
         cx.emit(DismissEvent);
     }
@@ -496,33 +329,50 @@ impl CommitModal {
 
 impl Render for CommitModal {
     fn render(&mut self, window: &mut Window, cx: &mut Context<'_, Self>) -> impl IntoElement {
-        let (width, _, border_radius) = self.container_properties(window, cx);
+        let properties = self.properties;
+        let width = px(properties.modal_width);
+        let container_padding = px(properties.container_padding);
+        let border_radius = properties.modal_border_radius;
+        let editor_focus_handle = self.commit_editor.focus_handle(cx);
 
         v_flex()
             .id("commit-modal")
             .key_context("GitCommit")
-            .elevation_3(cx)
-            .overflow_hidden()
             .on_action(cx.listener(Self::dismiss))
             .on_action(cx.listener(Self::commit))
-            // .on_action(cx.listener(Self::next_suggestion))
-            // .on_action(cx.listener(Self::prev_suggestion))
+            .elevation_3(cx)
+            .overflow_hidden()
+            .flex_none()
             .relative()
-            .justify_between()
             .bg(cx.theme().colors().elevated_surface_background)
             .rounded(px(border_radius))
             .border_1()
             .border_color(cx.theme().colors().border)
-            .w(px(width))
-            .h(px(360.))
-            .flex_1()
-            .overflow_hidden()
+            .w(width)
+            .p(container_padding)
             .child(
                 v_flex()
-                    .flex_1()
+                    .id("editor-container")
+                    .justify_between()
                     .p_2()
-                    .child(self.render_commit_editor(None, window, cx)),
+                    .size_full()
+                    .gap_2()
+                    .rounded(properties.editor_border_radius())
+                    .overflow_hidden()
+                    .cursor_text()
+                    .bg(cx.theme().colors().editor_background)
+                    .border_1()
+                    .border_color(cx.theme().colors().border_variant)
+                    .on_click(cx.listener(move |_, _: &ClickEvent, window, _cx| {
+                        window.focus(&editor_focus_handle);
+                    }))
+                    .child(
+                        div()
+                            .flex_1()
+                            .size_full()
+                            .child(self.render_commit_editor(window, cx)),
+                    )
+                    .child(self.render_footer(window, cx)),
             )
-        // .child(self.render_footer(window, cx))
     }
 }

crates/git_ui/src/git_panel.rs 🔗

@@ -621,6 +621,10 @@ impl GitPanel {
         }
     }
 
+    pub(crate) fn editor_focus_handle(&self, cx: &mut Context<Self>) -> FocusHandle {
+        self.commit_editor.focus_handle(cx).clone()
+    }
+
     fn focus_editor(&mut self, _: &FocusEditor, window: &mut Window, cx: &mut Context<Self>) {
         self.commit_editor.update(cx, |editor, cx| {
             window.focus(&editor.focus_handle(cx));