Merge branch 'claude/remove-group-edit-delete-buttons-3d38ec'

docs/manual-checks.md

@@ -324,7 +324,8 @@ phone's, whose list has no room for a row of group cards and draws the whole tre
 host filed, the grid says so in a sentence rather than sitting empty.
 
 **Then press a group card once.** It is marked as chosen and **nothing else happens** — the grid is still the
-level it was, and EDIT and DELETE now aim at that group. **Then double-press it.** The group opens: its hosts
+level it was, and no buttons appear beside the GROUPS heading: editing and deleting a group are on the card's
+own right-click menu, which is 7.9. **Then double-press it.** The group opens: its hosts
 are the grid, the trail above the cards reads `ALL HOSTS › <name> ›`, each card carrying the group's name as
 an accent chip, and the card grid shows what is *inside* that group rather than every group in the keychain.
 Pressing ALL HOSTS goes back to the outermost level.
@@ -347,9 +348,9 @@ Make two groups and file one under the other with the parent picker in the group
 
 **Pass:** only the outer group has a card to start with. Double-press it and the inner one is the only card
 shown, with the trail reading `ALL HOSTS › <outer> ›`. Double-press that, and the cards disappear entirely —
-it has nothing inside it — while the trail, EDIT and DELETE stay: with no card selected the two buttons act
+it has nothing inside it — while the trail stays. Pressing the **middle** crumb goes back one level rather
-on the group the trail ends with, so a group with nothing in it can still be renamed after being opened.
+than all the way out, which is also how a group with nothing inside it is renamed: back out to the level
-Pressing the **middle** crumb goes back one level rather than all the way out.
+where it has a card, and right-click that.
 
 **Failure means:** cards for groups that are not at this level is `VisibleGroups` having been bound past —
 the flat `Groups` is the phone's and the lookups'. A group that cannot be reached at all is worse and is the
@@ -370,7 +371,7 @@ takes only the group that is open.
 
 ### 3.3 Deleting a group with hosts in it
 
-Select a group with hosts and press DELETE.
+Right-click a group with hosts in it and choose **Delete…**.
 
 **Pass:** the question names how many hosts are filed under it and says they stay. Agreeing removes the
 group; the hosts lose their chip and are otherwise unchanged.
@@ -802,8 +803,9 @@ Delete.
 card, rather than back out to ALL HOSTS. Right-clicking the space around the group cards opens no menu.
 
 **Failure means:** the menu is reading `GroupTarget`'s fallback, which is the group whose contents are on
-screen. That fallback is right for the EDIT and DELETE buttons beside the heading and wrong for a menu that
+screen rather than the card the pointer is on. This menu is the only way to edit or delete a group on the
-opened on a card.
+desktop — there are no buttons beside the GROUPS heading any more — so a menu aimed wrongly is the whole of
+the mistake.
 
 ### 7.10 Clicking a host in the palette connects
 

src/DodoSSH.Client.App/Views/HostsScreen.axaml

@@ -226,25 +226,14 @@
               </ItemsControl.ItemTemplate>
             </ItemsControl>
 
-            <Grid ColumnDefinitions="Auto,*,Auto">
-              <TextBlock Grid.Column="0" Classes="label" Text="GROUPS"
-                         Foreground="{StaticResource TextDim}" VerticalAlignment="Center" />
-
             <!--
-                The group's own two actions, beside the group cards rather than in the toolbar, because they
+              The heading and nothing else. This used to be a row with EDIT and DELETE at the far end of it,
-                act on the card that is selected — and this is where the selection is made. Hidden rather
+              and the card's own menu is where both live now — see the note on it below. They were a second
-                than disabled while DELETE's question is up, as every other pair in this application is, so
+              control for the job that menu already does, and the harder of the two to read: a button beside
-                it cannot be pressed twice, and hidden again when there is nothing for them to act on. What
+              a heading has to say which card it means, and with none selected it meant the group the trail
-                that is, with no card selected, is the group the trail above ends with; see
+              ends with rather than anything on screen.
-                VaultViewModel.GroupTarget.
             -->
-              <StackPanel Grid.Column="2" Orientation="Horizontal" Spacing="6">
+            <TextBlock Classes="label" Text="GROUPS" Foreground="{StaticResource TextDim}" />
-                <Button Classes="ghost" Content="EDIT" Command="{Binding EditGroupCommand}"
-                        IsVisible="{Binding ShowsGroupActions}" />
-                <Button Classes="ghost" Content="DELETE" Command="{Binding DeleteGroupCommand}"
-                        IsVisible="{Binding ShowsGroupActions}" />
-              </StackPanel>
-            </Grid>
 
             <Border Padding="10" Background="{StaticResource DangerWash}" CornerRadius="6"
                     IsVisible="{Binding IsConfirmingGroupDeletion}">
@@ -269,8 +258,8 @@
 
               ◆ THIS IS ONE LEVEL, NOT EVERY GROUP. It binds VisibleGroups: what is inside the group the
               trail above ends with, or the outermost groups when it ends at ALL HOSTS. Folded away entirely
-              at a group with nothing inside it, which is an ordinary thing to open — the trail and the two
+              at a group with nothing inside it, which is an ordinary thing to open — the trail stays, and
-              buttons stay, because leaving it is a gesture and editing it is a button.
+              it is also the way back to the level where that group has a card of its own to be edited from.
 
               ◆ THESE CARDS ARE THE DROP TARGET. A host card dragged onto one is filed under that group, and
               that is the whole of what a drag does on this screen. It used to be a heading inside the host
@@ -295,11 +284,11 @@
                 context; the code-behind selects whatever was right-clicked before the menu opens, and
                 cancels it outright over the space around the cards.
 
-                Open is here as well as on the double-click, and Edit and Delete as well as on the two
+                Open is here as well as on the double-click, which is not a duplicate for its own sake: a
-                buttons above, and neither is a duplicate for its own sake: a gesture is unreachable without
+                gesture is unreachable without a pointer. Edit and Delete are here and nowhere else — they
-                a pointer, and the buttons act on GroupTarget — which with no card selected is the group the
+                were also a pair of buttons beside the heading above, which acted on GroupTarget and so with
-                trail ends with rather than the one under the pointer. This menu is the one place all three
+                no card selected meant the group the trail ends with rather than any card on screen. All
-                act on the card that was right-clicked.
+                three act on the card that was right-clicked, which is the whole of what this menu is for.
 
                 Only Open takes a parameter, because OpenGroupCommand's null means ALL HOSTS rather than
                 nothing; Edit and Delete read GroupTarget, which the selection the code-behind has just made

src/DodoSSH.Client.App/Views/HostsScreen.axaml.cs

@@ -217,8 +217,8 @@ internal sealed partial class HostsScreen : UserControl
     /// The host grid's rule, applied to the cards above it — see <see cref="OnContextRequested"/>. What is
     /// different is what an unaimed menu would have done: <c>GroupTarget</c> falls back to the open group
     /// when no card is selected, so Edit and Delete over a card would have been offered about the group whose
-    /// contents are showing rather than the one the pointer is on. That fallback is right for a pair of
+    /// contents are showing rather than the one the pointer is on. This menu is the only way to either of
-    /// buttons that sit beside the heading and wrong for a menu that opened on a card.
+    /// them now, so aiming it is the whole of aiming them.
     /// </para>
     /// <para>
     /// Cancelled outright over the space around the cards, as the host grid's is. That is not a group, and

src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs

@@ -1483,7 +1483,7 @@ internal sealed partial class VaultViewModel(
     /// </para>
     /// <para>
     /// <b>Separate from <see cref="SelectedGroup"/>, and no longer sets it.</b> The two answer different
-    /// questions — "what is on screen" and "what would EDIT and DELETE act on" — and while one click meant
+    /// questions — "what is on screen" and "which card is chosen" — and while one click meant
     /// both there was no way to name a group without also narrowing the grid to it. Two gestures, two
     /// properties; <see cref="GroupTarget"/> is where the two meet.
     /// </para>
@@ -2601,26 +2601,23 @@ internal sealed partial class VaultViewModel(
     internal bool IsConfirmingGroupDeletion => PendingDeletion?.Target is DeletionTarget.Group;
 
     /// <summary>
-    /// What the group panel's EDIT and DELETE act on: the card that is selected, or the group that is open.
+    /// What a group command with no argument acts on: the card that is selected, or the group that is open.
     /// </summary>
     /// <remarks>
-    /// Two answers, because a card is no longer where the user is. Selecting one aims the buttons at it,
+    /// Two answers, because a card is no longer where the user is. Selecting one aims at it, which is what a
-    /// which is what a click has always done; with nothing selected they aim at the group whose contents are
+    /// click has always done; with nothing selected the answer is the group whose contents are on screen —
-    /// on screen — the one the trail ends with. Without that fallback a group with no groups inside it could
+    /// the one the trail ends with. That fallback is what makes + NEW HOST open on the group somebody is
-    /// be opened and then never edited, because opening a group is exactly what takes its own card off the
+    /// standing in rather than on none, and it is what a file manager does: act on the selection, and on the
-    /// screen. It is also what a file manager does: act on the selection, and on the current folder when
+    /// current folder when there is none.
-    /// there is none.
+    /// <para>
+    /// The desktop's Edit and Delete reach this through the card menu, which selects whatever was
+    /// right-clicked first, so the fallback is not what they read — see <c>HostsScreen.OnGroupContextRequested</c>.
+    /// They used to be a pair of buttons beside the GROUPS heading, which had no card under a pointer to
+    /// mean and so leaned on it.
+    /// </para>
     /// </remarks>
     internal HostGroupRowViewModel? GroupTarget => SelectedGroup ?? GroupFilter;
 
-    /// <summary>Whether the group panel's buttons are showing.</summary>
-    /// <remarks>
-    /// Hidden with nothing to act on as well as while the question is up. A pair of buttons over a grid of
-    /// cards nobody has chosen between is a pair whose subject the user has to work out, and the answer
-    /// would be "neither" — see <see cref="GroupTarget"/>.
-    /// </remarks>
-    internal bool ShowsGroupActions => GroupTarget is not null && !IsConfirmingGroupDeletion;
-
     /// <summary>Whether this vault has any groups, which is what makes the sidebar draw headings.</summary>
     internal bool HasGroups => Groups.Count > 0;
 
@@ -4859,8 +4856,9 @@ internal sealed partial class VaultViewModel(
     /// <summary>Loads the group being acted on into the box, so saving renames it.</summary>
     /// <param name="group">
     /// The group to edit, or null for whatever the screen is aimed at — the selected card, or the open group
-    /// when no card is selected. See <see cref="GroupTarget"/>. The desktop's EDIT button passes nothing and
+    /// when no card is selected. See <see cref="GroupTarget"/>. The desktop's card menu passes nothing and
-    /// means the second; the phone has no card to select and passes the group its heading names.
+    /// means the card that was right-clicked, which opening the menu has already selected; the phone has no
+    /// card to select and passes the group its heading names.
     /// </param>
     /// <remarks>
     /// Taking it as an argument is what keeps the phone from having to select a group in order to edit one.
@@ -5226,7 +5224,7 @@ internal sealed partial class VaultViewModel(
     /// up under the ungrouped heading — so what the user needs to know is exactly how many machines are about
     /// to move, and that none of them are going anywhere else.
     /// <para>
-    /// Aims where EDIT does: at the selected card, or at the open group when no card is selected. See
+    /// Aims where Edit does: at the selected card, which on the desktop is the one the menu opened on. See
     /// <see cref="GroupTarget"/>.
     /// </para>
     /// </remarks>
@@ -8183,7 +8181,6 @@ internal sealed partial class VaultViewModel(
         DisarmIfAimedElsewhere(DeletionTarget.Group, GroupTarget?.EntityId);
 
         OnPropertyChanged(nameof(GroupTarget));
-        OnPropertyChanged(nameof(ShowsGroupActions));
     }
 
     /// <remarks>
@@ -8204,7 +8201,6 @@ internal sealed partial class VaultViewModel(
         DisarmIfAimedElsewhere(DeletionTarget.Group, GroupTarget?.EntityId);
 
         OnPropertyChanged(nameof(GroupTarget));
-        OnPropertyChanged(nameof(ShowsGroupActions));
 
         RebuildGroupLevel();
         RebuildVisibleHosts();
@@ -8217,7 +8213,6 @@ internal sealed partial class VaultViewModel(
         OnPropertyChanged(nameof(IsConfirmingGroupDeletion));
         OnPropertyChanged(nameof(ShowsHostActions));
         OnPropertyChanged(nameof(ShowsHostPaneActions));
-        OnPropertyChanged(nameof(ShowsGroupActions));
         OnPropertyChanged(nameof(ShowsItemActions));
     }
 

tests/DodoSSH.Client.App.Layout.Tests/HostGridTests.cs

@@ -166,9 +166,9 @@ public sealed class HostGridTests : IAsyncLifetime
     /// <remarks>
     /// <para>
     /// The host grid's menu acts on nothing when it is not aimed; this one acts on the <em>wrong group</em>.
-    /// <c>GroupTarget</c> falls back to the group whose contents are on screen when no card is selected — the
+    /// <c>GroupTarget</c> falls back to the group whose contents are on screen when no card is selected, and
-    /// right answer for the pair of buttons beside the heading, and the wrong one for a menu that opened on a
+    /// a menu that opened on a card would then offer to delete a group the pointer is nowhere near. It is
-    /// card, which would then offer to delete a group the pointer is nowhere near.
+    /// also the only way to Edit or Delete a group on the desktop, so this is the only place it is aimed.
     /// </para>
     /// <para>
     /// Open is the one entry that takes a parameter, because <c>OpenGroupCommand</c>'s null is a real
@@ -287,10 +287,10 @@ public sealed class HostGridTests : IAsyncLifetime
 
     /// <remarks>
     /// <para>
-    /// The rule one press was split into two gestures for. Selecting a group aims its EDIT and DELETE at it
+    /// The rule one press was split into two gestures for. Selecting a group marks it and does nothing else;
-    /// and does nothing else; opening one is what narrows the grid, and the trail is the way back out of it.
+    /// opening one is what narrows the grid, and the trail is the way back out of it. While a single press
-    /// While a single press meant both, a group could not be named without every host outside it leaving the
+    /// meant both, a group could not be named without every host outside it leaving the screen at the same
-    /// screen at the same moment.
+    /// moment.
     /// </para>
     /// <para>
     /// Driven through the properties the cards bind rather than through a click, because what is worth
@@ -299,7 +299,7 @@ public sealed class HostGridTests : IAsyncLifetime
     /// </para>
     /// </remarks>
     [Fact]
-    public async Task SelectingAGroupAimsItsButtonsAtItAndOpeningOneNarrowsTheGrid()
+    public async Task SelectingAGroupMarksItAndOpeningOneNarrowsTheGrid()
     {
         await vault.MoveHostToGroupCommand.ExecuteAsync(
             new HostGroupMove(Row(vault, "prod-db"), vault.Groups.Single().EntityId));
@@ -312,8 +312,7 @@ public sealed class HostGridTests : IAsyncLifetime
         vault.GroupFilter.ShouldBeNull("one press selects a group and does not open it");
         vault.VisibleHosts.Select(row => row.Label)
             .ShouldBe(["stage-web"], "so the grid is still the outermost level, and prod-db is inside a group");
-        vault.GroupTarget.ShouldBeSameAs(production, "what EDIT and DELETE act on");
+        vault.GroupTarget.ShouldBeSameAs(production, "what a group command with no argument acts on");
-        vault.ShowsGroupActions.ShouldBeTrue();
 
         vault.OpenGroupCommand.Execute(production);
 
@@ -324,7 +323,7 @@ public sealed class HostGridTests : IAsyncLifetime
 
         vault.SelectedGroup.ShouldBeNull("the card it was on is not one of the cards on screen any more");
         vault.GroupTarget.ShouldBeSameAs(
-            production, "so the buttons fall back to the group whose contents are showing");
+            production, "so an unaimed command falls back to the group whose contents are showing");
 
         // Back out, which is the trail's first crumb and nothing else: SHOW ALL was a second control for the
         // same job and went with the change.
@@ -333,7 +332,6 @@ public sealed class HostGridTests : IAsyncLifetime
         vault.VisibleHosts.Select(row => row.Label)
             .ShouldBe(["stage-web"], "ALL HOSTS is the outermost level, not every host in the keychain");
         vault.GroupTarget.ShouldBeNull("and nothing is aimed at once no group is open or selected");
-        vault.ShowsGroupActions.ShouldBeFalse("a pair of buttons with no subject is hidden rather than shown");
     }
 
     /// <summary>
@@ -376,35 +374,29 @@ public sealed class HostGridTests : IAsyncLifetime
             vault.SelectedGroup.ShouldBeNull("and the same in the other direction");
             screen.GroupGrid.SelectedItem.ShouldBeNull();
             GroupCard(screen).IsSelected.ShouldBeFalse();
-            vault.ShowsGroupActions.ShouldBeFalse("so the group's own two buttons have nothing to act on");
         });
     }
 
     /// <remarks>
-    /// EDIT takes the group as an argument now, so that the phone can open the editor on a heading without
+    /// A pair of EDIT and DELETE buttons used to sit beside the GROUPS heading, and the card's own menu is
-    /// selecting a group and losing the host selection to it — see <c>VaultViewModel.EditGroup</c>. The
+    /// the whole of both now — the menu came second and did the same job better, since it acts on the card
-    /// button beside the cards passes nothing and means "the card that is selected", which is the half of
+    /// under the pointer rather than on <c>GroupTarget</c>. Held here because a button coming back is not a
-    /// that change that would fail silently: a command refusing a null parameter is a button that never
+    /// compile error and barely a visible one: it would draw itself in place, aimed with no card selected at
-    /// fires, and nothing about the markup would say so.
+    /// the group the trail ends with, which is the mistake the two menu tests above exist to catch.
     /// </remarks>
     [Fact]
-    public async Task TheGroupsEditButtonStillActsOnTheSelectedCard()
+    public async Task AGroupsEditAndDeleteAreOnItsCardsMenuAndNowhereElse()
     {
         await OnTheGridAsync((screen, _) =>
         {
             vault.SelectedGroup = vault.VisibleGroups.Single();
             Dispatcher.UIThread.RunJobs();
 
-            var edit = screen.GetVisualDescendants()
+            screen.GetVisualDescendants()
                 .OfType<Button>()
-                .Single(button => ReferenceEquals(button.Command, vault.EditGroupCommand));
+                .Where(button => ReferenceEquals(button.Command, vault.EditGroupCommand)
-
+                    || ReferenceEquals(button.Command, vault.DeleteGroupCommand))
-            edit.IsEffectivelyEnabled.ShouldBeTrue("the command has to accept the button's empty parameter");
+                .ShouldBeEmpty("a selected group card puts no buttons on the screen");
-
-            edit.Command.ShouldNotBeNull().Execute(edit.CommandParameter);
-
-            vault.IsEditingGroup.ShouldBeTrue();
-            vault.GroupEditorLabel.ShouldBe("production", "the card that was selected");
         });
     }