From b34c0fd71b4cb040c2d123683ccf035b2ae7be7d Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 3 Mar 2025 06:39:24 -0800 Subject: [PATCH] git_ui: Fix item heights in git panel (#25833) - Fixes items slightly overlapping in the git panel - Fixes commit button in the project diff not opening modal Release Notes: - N/A --------- Co-authored-by: Nate Butler Co-authored-by: Cole Miller --- crates/git/src/git.rs | 2 +- crates/git_ui/src/commit_modal.rs | 4 +- crates/git_ui/src/git_panel.rs | 245 +++++++++++++++++++----------- crates/git_ui/src/project_diff.rs | 7 +- 4 files changed, 166 insertions(+), 92 deletions(-) diff --git a/crates/git/src/git.rs b/crates/git/src/git.rs index 74e666dd4b193cfe904ce402d1ac02ed2d979d13..d26409a007c0b83925c997adbe76cf2a91303426 100644 --- a/crates/git/src/git.rs +++ b/crates/git/src/git.rs @@ -64,7 +64,7 @@ actions!( Pull, Fetch, Commit, - ExpandCommitEditor, + ShowCommitEditor, ] ); action_with_deprecated_aliases!(git, RestoreFile, ["editor::RevertFile"]); diff --git a/crates/git_ui/src/commit_modal.rs b/crates/git_ui/src/commit_modal.rs index 8df0d38a6ff3eb577129a413d07060442a0c9138..14bd3ab99c73092662d6e70147ff80491e970856 100644 --- a/crates/git_ui/src/commit_modal.rs +++ b/crates/git_ui/src/commit_modal.rs @@ -2,7 +2,7 @@ use crate::branch_picker::{self, BranchList}; use crate::git_panel::{commit_message_editor, GitPanel}; -use git::{Commit, ExpandCommitEditor}; +use git::{Commit, ShowCommitEditor}; use panel::{panel_button, panel_editor_style, panel_filled_button}; use project::Project; use ui::{prelude::*, KeybindingHint, PopoverButton, Tooltip, TriggerablePopover}; @@ -110,7 +110,7 @@ struct RestoreDock { impl CommitModal { pub fn register(workspace: &mut Workspace, _: &mut Window, _cx: &mut Context) { - workspace.register_action(|workspace, _: &ExpandCommitEditor, window, cx| { + workspace.register_action(|workspace, _: &ShowCommitEditor, window, cx| { let Some(git_panel) = workspace.panel::(cx) else { return; }; diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index b9eda964d65691c9824f5a3c91539ace5b3044cc..eed1e7cbd5ca56532b6a5a7f067c9184dc933f14 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/crates/git_ui/src/git_panel.rs @@ -40,8 +40,8 @@ use std::{collections::HashSet, sync::Arc, time::Duration, usize}; use strum::{IntoEnumIterator, VariantNames}; use time::OffsetDateTime; use ui::{ - prelude::*, ButtonLike, Checkbox, ContextMenu, ElevationIndex, ListItem, ListItemSpacing, - PopoverButton, PopoverMenu, Scrollbar, ScrollbarState, Tooltip, + prelude::*, ButtonLike, Checkbox, ContextMenu, ElevationIndex, PopoverButton, PopoverMenu, + Scrollbar, ScrollbarState, Tooltip, }; use util::{maybe, post_inc, ResultExt, TryFutureExt}; @@ -210,6 +210,7 @@ pub struct GitPanel { scroll_handle: UniformListScrollHandle, scrollbar_state: ScrollbarState, selected_entry: Option, + marked_entries: Vec, show_scrollbar: bool, tracked_count: usize, tracked_staged_count: usize, @@ -337,6 +338,7 @@ impl GitPanel { scroll_handle, scrollbar_state, selected_entry: None, + marked_entries: Vec::new(), show_scrollbar: false, tracked_count: 0, tracked_staged_count: 0, @@ -2077,7 +2079,7 @@ impl GitPanel { .on_click(cx.listener({ move |_, _, window, cx| { window.dispatch_action( - git::ExpandCommitEditor.boxed_clone(), + git::ShowCommitEditor.boxed_clone(), cx, ) } @@ -2309,7 +2311,7 @@ impl GitPanel { } }) .size_full() - .with_sizing_behavior(ListSizingBehavior::Infer) + .with_sizing_behavior(ListSizingBehavior::Auto) .with_horizontal_sizing_behavior(ListHorizontalSizingBehavior::Unconstrained) .track_scroll(self.scroll_handle.clone()), ) @@ -2326,6 +2328,10 @@ impl GitPanel { Label::new(label.into()).color(color).single_line() } + fn list_item_height(&self) -> Rems { + rems(1.75) + } + fn render_list_header( &self, ix: usize, @@ -2334,18 +2340,21 @@ impl GitPanel { _: &Window, _: &Context, ) -> AnyElement { - div() + let id: ElementId = ElementId::Name(format!("header_{}", ix).into()); + + h_flex() + .id(id) + .h(self.list_item_height()) .w_full() + .items_end() + .px(rems(0.75)) // ~12px + .pb(rems(0.3125)) // ~ 5px .child( - ListItem::new(ix) - .spacing(ListItemSpacing::Sparse) - .disabled(true) - .child( - Label::new(header.title()) - .color(Color::Muted) - .size(LabelSize::Small) - .single_line(), - ), + Label::new(header.title()) + .color(Color::Muted) + .size(LabelSize::Small) + .line_height_style(LineHeightStyle::UiLabel) + .single_line(), ) .into_any_element() } @@ -2435,7 +2444,7 @@ impl GitPanel { ix: usize, entry: &GitStatusEntry, has_write_access: bool, - window: &Window, + _: &Window, cx: &Context, ) -> AnyElement { let display_name = entry @@ -2446,6 +2455,7 @@ impl GitPanel { let repo_path = entry.repo_path.clone(); let selected = self.selected_entry == Some(ix); + let marked = self.marked_entries.contains(&ix); let status_style = GitPanelSettings::get_global(cx).status_style; let status = entry.status; let has_conflict = status.is_conflicted(); @@ -2473,7 +2483,11 @@ impl GitPanel { Color::Muted }; - let id: ElementId = ElementId::Name(format!("entry_{}", display_name).into()); + let id: ElementId = ElementId::Name(format!("entry_{}_{}", display_name, ix).into()); + let checkbox_wrapper_id: ElementId = + ElementId::Name(format!("entry_{}_{}_checkbox_wrapper", display_name, ix).into()); + let checkbox_id: ElementId = + ElementId::Name(format!("entry_{}_{}_checkbox", display_name, ix).into()); let is_entry_staged = self.entry_is_staged(entry); let mut is_staged: ToggleState = self.entry_is_staged(entry).into(); @@ -2482,84 +2496,143 @@ impl GitPanel { is_staged = ToggleState::Selected; } - let checkbox = Checkbox::new(id, is_staged) - .disabled(!has_write_access) - .fill() - .placeholder(!self.has_staged_changes() && !self.has_conflicts()) - .elevation(ElevationIndex::Surface) - .on_click({ - let entry = entry.clone(); - cx.listener(move |this, _, window, cx| { - this.toggle_staged_for_entry( - &GitListEntry::GitStatusEntry(entry.clone()), - window, - cx, - ); - cx.stop_propagation(); - }) - }); + let handle = cx.weak_entity(); + + let selected_bg_alpha = 0.08; + let marked_bg_alpha = 0.12; + let state_opacity_step = 0.04; + + let base_bg = match (selected, marked) { + (true, true) => cx + .theme() + .status() + .info + .alpha(selected_bg_alpha + marked_bg_alpha), + (true, false) => cx.theme().status().info.alpha(selected_bg_alpha), + (false, true) => cx.theme().status().info.alpha(marked_bg_alpha), + _ => cx.theme().colors().ghost_element_background, + }; - let start_slot = h_flex() - .id(("start-slot", ix)) - .gap(DynamicSpacing::Base04.rems(cx)) - .child(checkbox.tooltip(move |window, cx| { - let tooltip_name = if is_entry_staged.unwrap_or(false) { - "Unstage" - } else { - "Stage" - }; + let hover_bg = if selected { + cx.theme() + .status() + .info + .alpha(selected_bg_alpha + state_opacity_step) + } else { + cx.theme().colors().ghost_element_hover + }; - Tooltip::for_action(tooltip_name, &ToggleStaged, window, cx) - })) - .child(git_status_icon(status, cx)) - .on_mouse_down(MouseButton::Left, |_, _, cx| { - // prevent the list item active state triggering when toggling checkbox - cx.stop_propagation(); - }); + let active_bg = if selected { + cx.theme() + .status() + .info + .alpha(selected_bg_alpha + state_opacity_step * 2.0) + } else { + cx.theme().colors().ghost_element_active + }; - div() + h_flex() + .id(id) + .h(self.list_item_height()) .w_full() + .items_center() + .px(rems(0.75)) // ~12px + .overflow_hidden() + .flex_none() + .gap(DynamicSpacing::Base04.rems(cx)) + .bg(base_bg) + .hover(|this| this.bg(hover_bg)) + .active(|this| this.bg(active_bg)) + .on_click({ + cx.listener(move |this, event: &ClickEvent, window, cx| { + this.selected_entry = Some(ix); + cx.notify(); + if event.modifiers().secondary() { + this.open_file(&Default::default(), window, cx) + } else { + this.open_diff(&Default::default(), window, cx); + } + }) + }) + .on_mouse_down( + MouseButton::Right, + move |event: &MouseDownEvent, window, cx| { + // why isn't this happening automatically? we are passing MouseButton::Right to `on_mouse_down`? + if event.button != MouseButton::Right { + return; + } + + let Some(this) = handle.upgrade() else { + return; + }; + this.update(cx, |this, cx| { + this.deploy_entry_context_menu(event.position, ix, window, cx); + }); + cx.stop_propagation(); + }, + ) + // .on_secondary_mouse_down(cx.listener( + // move |this, event: &MouseDownEvent, window, cx| { + // this.deploy_entry_context_menu(event.position, ix, window, cx); + // cx.stop_propagation(); + // }, + // )) .child( - ListItem::new(ix) - .spacing(ListItemSpacing::Sparse) - .start_slot(start_slot) - .toggle_state(selected) - .focused(selected && self.focus_handle(cx).is_focused(window)) - .disabled(!has_write_access) - .on_click({ - cx.listener(move |this, event: &ClickEvent, window, cx| { - this.selected_entry = Some(ix); - cx.notify(); - if event.modifiers().secondary() { - this.open_file(&Default::default(), window, cx) - } else { - this.open_diff(&Default::default(), window, cx); - } - }) - }) - .on_secondary_mouse_down(cx.listener( - move |this, event: &MouseDownEvent, window, cx| { - this.deploy_entry_context_menu(event.position, ix, window, cx); - cx.stop_propagation(); - }, - )) + div() + .id(checkbox_wrapper_id) + .flex_none() + .occlude() + .cursor_pointer() .child( - h_flex() - .when_some(repo_path.parent(), |this, parent| { - let parent_str = parent.to_string_lossy(); - if !parent_str.is_empty() { - this.child( - self.entry_label(format!("{}/", parent_str), path_color) - .when(status.is_deleted(), |this| this.strikethrough()), - ) - } else { - this - } + Checkbox::new(checkbox_id, is_staged) + .disabled(!has_write_access) + .fill() + .placeholder(!self.has_staged_changes() && !self.has_conflicts()) + .elevation(ElevationIndex::Surface) + .on_click({ + let entry = entry.clone(); + cx.listener(move |this, _, window, cx| { + if !has_write_access { + return; + } + this.toggle_staged_for_entry( + &GitListEntry::GitStatusEntry(entry.clone()), + window, + cx, + ); + cx.stop_propagation(); + }) }) - .child( - self.entry_label(display_name.clone(), label_color) + .tooltip(move |window, cx| { + let tooltip_name = if is_entry_staged.unwrap_or(false) { + "Unstage" + } else { + "Stage" + }; + + Tooltip::for_action(tooltip_name, &ToggleStaged, window, cx) + }), + ), + ) + .child(git_status_icon(status, cx)) + .child( + h_flex() + .items_center() + .overflow_hidden() + .when_some(repo_path.parent(), |this, parent| { + let parent_str = parent.to_string_lossy(); + if !parent_str.is_empty() { + this.child( + self.entry_label(format!("{}/", parent_str), path_color) .when(status.is_deleted(), |this| this.strikethrough()), - ), + ) + } else { + this + } + }) + .child( + self.entry_label(display_name.clone(), label_color) + .when(status.is_deleted(), |this| this.strikethrough()), ), ) .into_any_element() diff --git a/crates/git_ui/src/project_diff.rs b/crates/git_ui/src/project_diff.rs index e71490edcee9e75ed2d101adf65b31dbbb436240..2627671e45f60644c9b52008fd1c5f06104003b9 100644 --- a/crates/git_ui/src/project_diff.rs +++ b/crates/git_ui/src/project_diff.rs @@ -10,7 +10,8 @@ use editor::{ use feature_flags::FeatureFlagViewExt; use futures::StreamExt; use git::{ - status::FileStatus, Commit, StageAll, StageAndNext, ToggleStaged, UnstageAll, UnstageAndNext, + status::FileStatus, ShowCommitEditor, StageAll, StageAndNext, ToggleStaged, UnstageAll, + UnstageAndNext, }; use gpui::{ actions, Action, AnyElement, AnyView, App, AppContext as _, AsyncWindowContext, Entity, @@ -952,11 +953,11 @@ impl Render for ProjectDiffToolbar { .disabled(!button_states.commit) .tooltip(Tooltip::for_action_title_in( "Commit", - &Commit, + &ShowCommitEditor, &focus_handle, )) .on_click(cx.listener(|this, _, window, cx| { - this.dispatch_action(&Commit, window, cx); + this.dispatch_action(&ShowCommitEditor, window, cx); })), ), )