code_review: PR #1412

- Use `ViewModels.StashesPage.SelectedStash` instead of `sender is not ListBox { SelectedValue: Models.Stash stash }`
- In tags view, `SelectedItem` can be `Models.Tag` or `ViewModels.TagTreeNode`
- In logs window, `vm.SelectedLog` may be null

Signed-off-by: leo <[email protected]>

Author: love-linger

src/ViewModels/Repository.cs

@@ -1365,12 +1365,6 @@ public void DeleteBranch(Models.Branch branch)
                 ShowPopup(new DeleteBranch(this, branch));
         }
 
-        public void DeleteRemote(Models.Remote remote)
-        {
-            if (CanCreatePopup())
-                ShowPopup(new DeleteRemote(this, remote));
-        }
-
         public void DeleteMultipleBranches(List<Models.Branch> branches, bool isLocal)
         {
             if (CanCreatePopup())
@@ -1407,6 +1401,12 @@ public void AddRemote()
                 ShowPopup(new AddRemote(this));
         }
 
+        public void DeleteRemote(Models.Remote remote)
+        {
+            if (CanCreatePopup())
+                ShowPopup(new DeleteRemote(this, remote));
+        }
+
         public void AddSubmodule()
         {
             if (CanCreatePopup())

src/ViewModels/StashesPage.cs

@@ -295,6 +295,12 @@ public void ClearSearchFilter()
             SearchFilter = string.Empty;
         }
 
+        public void Drop(Models.Stash stash)
+        {
+            if (stash != null && _repo.CanCreatePopup())
+                _repo.ShowPopup(new DropStash(_repo, stash));
+        }
+
         private void RefreshVisible()
         {
             if (string.IsNullOrEmpty(_searchFilter))
@@ -314,12 +320,6 @@ private void RefreshVisible()
             }
         }
 
-        public void Drop(Models.Stash stash)
-        {
-            if (_repo.CanCreatePopup())
-                _repo.ShowPopup(new DropStash(_repo, stash));
-        }
-
         private Repository _repo = null;
         private List<Models.Stash> _stashes = [];
         private List<Models.Stash> _visibleStashes = [];

src/Views/BranchTree.axaml

@@ -13,7 +13,7 @@
            ItemsSource="{Binding #ThisControl.Rows}"
            SelectionMode="Multiple"
            SelectionChanged="OnNodesSelectionChanged"
-           KeyDown="OnListKeyDown"
+           KeyDown="OnTreeKeyDown"
            ContextRequested="OnTreeContextRequested">
     <ListBox.ItemsPanel>
       <ItemsPanelTemplate>

src/Views/BranchTree.axaml.cs

@@ -450,7 +450,7 @@ private void OnTreeContextRequested(object _1, ContextRequestedEventArgs _2)
             }
         }
 
-        private void OnListKeyDown(object _, KeyEventArgs e)
+        private void OnTreeKeyDown(object _, KeyEventArgs e)
         {
             if (e.Key is not (Key.Delete or Key.Back))
                 return;
@@ -463,7 +463,7 @@ private void OnListKeyDown(object _, KeyEventArgs e)
             if (selected == null || selected.Count == 0)
                 return;
 
-            if (selected is [ViewModels.BranchTreeNode { Backend: Models.Remote remote }])
+            if (selected.Count == 1 && selected[0] is ViewModels.BranchTreeNode { Backend: Models.Remote remote })
             {
                 repo.DeleteRemote(remote);
                 e.Handled = true;

src/Views/StashesPage.axaml

@@ -65,7 +65,7 @@
                ItemsSource="{Binding VisibleStashes}"
                SelectedItem="{Binding SelectedStash, Mode=TwoWay}"
                SelectionMode="Single"
-               KeyDown="OnStashKeyDown"
+               KeyDown="OnStashListKeyDown"
                ScrollViewer.HorizontalScrollBarVisibility="Disabled"
                ScrollViewer.VerticalScrollBarVisibility="Auto">
         <ListBox.Styles>

src/Views/StashesPage.axaml.cs

@@ -24,28 +24,25 @@ private void OnMainLayoutSizeChanged(object sender, SizeChangedEventArgs e)
                 layout.StashesLeftWidth = new GridLength(maxLeft, GridUnitType.Pixel);
         }
 
-        private void OnStashContextRequested(object sender, ContextRequestedEventArgs e)
-        {
-            if (DataContext is ViewModels.StashesPage vm && sender is Border border)
-            {
-                var menu = vm.MakeContextMenu(border.DataContext as Models.Stash);
-                menu?.Open(border);
-            }
-            e.Handled = true;
-        }
-
-        private void OnStashKeyDown(object sender, KeyEventArgs e)
+        private void OnStashListKeyDown(object sender, KeyEventArgs e)
         {
             if (e.Key is not (Key.Delete or Key.Back))
                 return;
 
             if (DataContext is not ViewModels.StashesPage vm)
                 return;
 
-            if (sender is not ListBox { SelectedValue: Models.Stash stash })
-                return;
+            vm.Drop(vm.SelectedStash);
+            e.Handled = true;
+        }
 
-            vm.Drop(stash);
+        private void OnStashContextRequested(object sender, ContextRequestedEventArgs e)
+        {
+            if (DataContext is ViewModels.StashesPage vm && sender is Border border)
+            {
+                var menu = vm.MakeContextMenu(border.DataContext as Models.Stash);
+                menu?.Open(border);
+            }
             e.Handled = true;
         }
 

src/Views/TagsView.axaml.cs

@@ -217,13 +217,15 @@ private void OnSelectionChanged(object sender, SelectionChangedEventArgs _)
 
         private void OnKeyDown(object sender, KeyEventArgs e)
         {
-            if (sender is not ListBox { SelectedValue: Models.Tag tag })
-                return;
-
             if (DataContext is not ViewModels.Repository repo)
                 return;
-         
-            repo.DeleteTag(tag);
+
+            var selected = (sender as ListBox)?.SelectedItem;
+            if (selected is ViewModels.TagTreeNode { Tag: { } tagInNode })
+                repo.DeleteTag(tagInNode);
+            else if (selected is Models.Tag tag)
+                repo.DeleteTag(tag);
+
             e.Handled = true;
         }
     }

src/Views/ViewLogs.axaml.cs

@@ -45,11 +45,10 @@ private void OnLogKeyDown(object _, KeyEventArgs e)
         {
             if (e.Key is not (Key.Delete or Key.Back))
                 return;
-            
-            if (DataContext is not ViewModels.ViewLogs vm)
-                return;
 
-            vm.Logs.Remove(vm.SelectedLog);
+            if (DataContext is ViewModels.ViewLogs { SelectedLog: { } log } vm)
+                vm.Logs.Remove(log);
+
             e.Handled = true;
         }
     }

src/Views/Welcome.axaml

@@ -61,8 +61,7 @@
                            ItemsSource="{Binding Rows}"
                            SelectionMode="Single"
                            Loaded="SetupTreeViewDragAndDrop"
-                           LostFocus="OnTreeViewLostFocus"
-                           KeyDown="OnTreeViewKeyDown">
+                           LostFocus="OnTreeViewLostFocus">
         <ListBox.Styles>
           <Style Selector="ListBox">
             <Setter Property="FocusAdorner">

src/Views/Welcome.axaml.cs

@@ -31,9 +31,25 @@ public class RepositoryListBox : ListBox
 
         protected override void OnKeyDown(KeyEventArgs e)
         {
-            if (SelectedItem is ViewModels.RepositoryNode { IsRepository: false } node && e.KeyModifiers == KeyModifiers.None)
+            if (SelectedItem is ViewModels.RepositoryNode node && e.KeyModifiers == KeyModifiers.None)
             {
-                if ((node.IsExpanded && e.Key == Key.Left) || (!node.IsExpanded && e.Key == Key.Right))
+                if (e.Key is Key.Delete or Key.Back)
+                {
+                    node.Delete();
+                    e.Handled = true;
+                }
+                else if (node.IsRepository)
+                {
+                    if (e.Key == Key.Enter)
+                    {
+                        var parent = this.FindAncestorOfType<Launcher>();
+                        if (parent is { DataContext: ViewModels.Launcher launcher })
+                            launcher.OpenRepositoryInTab(node, null);
+
+                        e.Handled = true;
+                    }
+                }
+                else if ((node.IsExpanded && e.Key == Key.Left) || (!node.IsExpanded && e.Key == Key.Right) || e.Key == Key.Enter)
                 {
                     ViewModels.Welcome.Instance.ToggleNodeIsExpanded(node);
                     e.Handled = true;
@@ -92,33 +108,6 @@ private void SetupTreeNodeDragAndDrop(object sender, RoutedEventArgs _)
             }
         }
 
-        private void OnTreeViewKeyDown(object _, KeyEventArgs e)
-        {
-            if (TreeContainer.SelectedItem is ViewModels.RepositoryNode node)
-            {
-                if (e.Key == Key.Enter)
-                {
-                    if (node.IsRepository)
-                    {
-                        var parent = this.FindAncestorOfType<Launcher>();
-                        if (parent is { DataContext: ViewModels.Launcher launcher })
-                            launcher.OpenRepositoryInTab(node, null);
-                    }
-                    else
-                    {
-                        ViewModels.Welcome.Instance.ToggleNodeIsExpanded(node);
-                    }
-
-                    e.Handled = true;
-                }
-                else if (e.Key is Key.Delete or Key.Back)
-                {
-                    node.Delete();
-                    e.Handled = true;
-                }
-            }
-        }
-
         private void OnTreeNodeContextRequested(object sender, ContextRequestedEventArgs e)
         {
             if (sender is Grid { DataContext: ViewModels.RepositoryNode node } grid)