DodoTech-Public/DodoSSH
Public Access
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 2 Wiki Activity

Take the group's EDIT and DELETE off the heading row

Browse Source 
A pair of buttons sat at the end of the GROUPS heading, and the card's own
right-click menu arrived later offering the same two things. Two controls for one
job, and the buttons were the harder of the two to read: a button beside a
heading has no card under a pointer to mean, so it had to work its subject out —
the selected card, or failing that the group the trail ends with, which once a
group is open is not a card on screen at all. The menu never has that problem,
because opening it is what aims it.

The menu is the whole of Edit and Delete on the desktop now. ShowsGroupActions
went with the buttons, since hiding them was all it did. GroupTarget stays: the
menu's two entries read it after the code-behind has selected whatever was
right-clicked, and its fallback to the open group is what makes + NEW HOST open
on the group somebody is standing in rather than on none.

One case changes shape. Opening a group with nothing inside it folds the card
grid away, so from in there nothing can be right-clicked — renaming that group
means pressing the trail back one level, to where it has a card of its own. The
buttons used to cover it through the fallback. The trail was already the way out
of an empty group, and 3.2a says so now.

The test that pressed EDIT through its binding is replaced by one holding that
no button on the screen commands either of them, which is the failure worth
catching: a button coming back is not a compile error, and it would draw itself
in place, aimed at the group the trail ends with. What that test covered — a
command that has to accept an empty parameter, and act on the card the pointer
was on — the two menu tests beside it already do.
This commit is contained in:
jaap-jan
2026-08-04 16:39:14 +02:00
parent 742f65c204
commit d02d103569
5 changed files with 63 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/manual-checks.md
+9 -7
View File
@@ -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
on the group the trail ends with, so a group with nothing in it can still be renamed after being opened.
Pressing the **middle** crumb goes back one level rather than all the way out.
it has nothing inside it — while the trail stays. Pressing the **middle** crumb goes back one level rather
than all the way out, which is also how a group with nothing inside it is renamed: back out to the level
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
opened on a card.
screen rather than the card the pointer is on. This menu is the only way to edit or delete a group on the
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
+13 -24
View File
@@ -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
act on the card that is selected — and this is where the selection is made. Hidden rather
than disabled while DELETE's question is up, as every other pair in this application is, so
it cannot be pressed twice, and hidden again when there is nothing for them to act on. What
that is, with no card selected, is the group the trail above ends with; see
VaultViewModel.GroupTarget.
The heading and nothing else. This used to be a row with EDIT and DELETE at the far end of it,
and the card's own menu is where both live now — see the note on it below. They were a second
control for the job that menu already does, and the harder of the two to read: a button beside
a heading has to say which card it means, and with none selected it meant the group the trail
ends with rather than anything on screen.
-->
<StackPanel Grid.Column="2" Orientation="Horizontal" Spacing="6">
<Button Classes="ghost" Content="EDIT" Command="{Binding EditGroupCommand}"
IsVisible="{Binding ShowsGroupActions}" />
<Button Classes="ghost" Content="DELETE" Command="{Binding DeleteGroupCommand}"
IsVisible="{Binding ShowsGroupActions}" />
</StackPanel>
</Grid>
<TextBlock Classes="label" Text="GROUPS" Foreground="{StaticResource TextDim}" />
<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
buttons stay, because leaving it is a gesture and editing it is a button.
at a group with nothing inside it, which is an ordinary thing to open — the trail stays, and
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
buttons above, and neither is a duplicate for its own sake: a gesture is unreachable without
a pointer, and the buttons act on GroupTarget — which with no card selected is the group the
trail ends with rather than the one under the pointer. This menu is the one place all three
act on the card that was right-clicked.
Open is here as well as on the double-click, which is not a duplicate for its own sake: a
gesture is unreachable without a pointer. Edit and Delete are here and nowhere else — they
were also a pair of buttons beside the heading above, which acted on GroupTarget and so with
no card selected meant the group the trail ends with rather than any card on screen. All
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
+2 -2
View File
@@ -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
/// buttons that sit beside the heading and wrong for a menu that opened on a card.
/// contents are showing rather than the one the pointer is on. This menu is the only way to either of
/// 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
+17 -22
View File
@@ -1466,7 +1466,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>
@@ -2584,26 +2584,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,
/// which is what a click has always done; with nothing selected they aim at the group whose contents are
/// on screen — the one the trail ends with. Without that fallback a group with no groups inside it could
/// be opened and then never edited, because opening a group is exactly what takes its own card off the
/// screen. It is also what a file manager does: act on the selection, and on the current folder when
/// there is none.
/// Two answers, because a card is no longer where the user is. Selecting one aims at it, which is what a
/// click has always done; with nothing selected the answer is the group whose contents are on screen —
/// the one the trail ends with. That fallback is what makes + NEW HOST open on the group somebody is
/// standing in rather than on none, and it is what a file manager does: act on the selection, and on the
/// current folder when 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;
@@ -4743,8 +4740,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
/// means the second; the phone has no card to select and passes the group its heading names.
/// when no card is selected. See <see cref="GroupTarget"/>. The desktop's card menu passes nothing and
/// 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.
@@ -5110,7 +5108,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>
@@ -8067,7 +8065,6 @@ internal sealed partial class VaultViewModel(
DisarmIfAimedElsewhere(DeletionTarget.Group, GroupTarget?.EntityId);
OnPropertyChanged(nameof(GroupTarget));
OnPropertyChanged(nameof(ShowsGroupActions));
}
/// <remarks>
@@ -8088,7 +8085,6 @@ internal sealed partial class VaultViewModel(
DisarmIfAimedElsewhere(DeletionTarget.Group, GroupTarget?.EntityId);
OnPropertyChanged(nameof(GroupTarget));
OnPropertyChanged(nameof(ShowsGroupActions));
RebuildGroupLevel();
RebuildVisibleHosts();
@@ -8101,7 +8097,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
+20 -28
View File
@@ -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
/// right answer for the pair of buttons beside the heading, and the wrong one for a menu that opened on a
/// card, which would then offer to delete a group the pointer is nowhere near.
/// <c>GroupTarget</c> falls back to the group whose contents are on screen when no card is selected, and
/// a menu that opened on a card would then offer to delete a group the pointer is nowhere near. It is
/// 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
/// and does nothing else; opening one is what narrows the grid, and the trail is the way back out of it.
/// While a single press meant both, a group could not be named without every host outside it leaving the
/// screen at the same moment.
/// The rule one press was split into two gestures for. Selecting a group marks it and does nothing else;
/// opening one is what narrows the grid, and the trail is the way back out of it. While a single press
/// meant both, a group could not be named without every host outside it leaving the screen at the same
/// 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.ShowsGroupActions.ShouldBeTrue();
vault.GroupTarget.ShouldBeSameAs(production, "what a group command with no argument acts on");
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
/// selecting a group and losing the host selection to it — see <c>VaultViewModel.EditGroup</c>. The
/// button beside the cards passes nothing and means "the card that is selected", which is the half of
/// that change that would fail silently: a command refusing a null parameter is a button that never
/// fires, and nothing about the markup would say so.
/// A pair of EDIT and DELETE buttons used to sit beside the GROUPS heading, and the card's own menu is
/// the whole of both now — the menu came second and did the same job better, since it acts on the card
/// under the pointer rather than on <c>GroupTarget</c>. Held here because a button coming back is not a
/// compile error and barely a visible one: it would draw itself in place, aimed with no card selected at
/// 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));
edit.IsEffectivelyEnabled.ShouldBeTrue("the command has to accept the button's empty parameter");
edit.Command.ShouldNotBeNull().Execute(edit.CommandParameter);
vault.IsEditingGroup.ShouldBeTrue();
vault.GroupEditorLabel.ShouldBe("production", "the card that was selected");
.Where(button => ReferenceEquals(button.Command, vault.EditGroupCommand)
|| ReferenceEquals(button.Command, vault.DeleteGroupCommand))
.ShouldBeEmpty("a selected group card puts no buttons on the screen");
});
}