Merge branch 'claude/card-selection-state-sharing-f9348c'

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

@@ -262,6 +262,11 @@
               opens a shell on a host. One click used to mean both, and a card was then the only place a
               group could be named while also being the control that threw the rest of the grid away.
 
+              ◆ ONE SELECTION, TWO LISTS. Selecting a card here takes the mark off the host grid below, and
+              selecting a host takes it off this one. Two controls each keep their own SelectedItem, so the
+              vault is what joins them — see VaultViewModel.OnSelectedHostChanged. Without it both grids
+              could be lit at once, which is two chosen things under two pairs of buttons.
+
               ◆ 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

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

@@ -1408,6 +1408,14 @@ internal sealed partial class VaultViewModel(
     [ObservableProperty]
     private VaultChoiceViewModel? selectedTargetVault;
 
+    /// <summary>
+    /// The host card that is selected, or null when none is.
+    /// </summary>
+    /// <remarks>
+    /// The application's selection, which everything that acts on a host reads — connecting, editing,
+    /// deleting. <b>It shares one selection with <see cref="SelectedGroup"/>:</b> selecting a host takes the
+    /// mark off a group card and the other way about. See <see cref="OnSelectedHostChanged"/>.
+    /// </remarks>
     [ObservableProperty]
     private HostRowViewModel? selectedHost;
 
@@ -1435,8 +1443,14 @@ internal sealed partial class VaultViewModel(
     /// The group card that is selected, or null when none is.
     /// </summary>
     /// <remarks>
+    /// <para>
     /// One click, and nothing more than a highlight: it is what the group's own EDIT and DELETE act on. What
     /// it deliberately no longer does is narrow the grid — see <see cref="OpenGroup"/>.
+    /// </para>
+    /// <para>
+    /// <b>It shares one selection with <see cref="SelectedHost"/>.</b> Setting either clears the other, so
+    /// exactly one card on the screen is ever lit. See <see cref="OnSelectedHostChanged"/>.
+    /// </para>
     /// </remarks>
     [ObservableProperty]
     private HostGroupRowViewModel? selectedGroup;
@@ -3023,9 +3037,7 @@ internal sealed partial class VaultViewModel(
             Hosts.Add(host);
         }
 
-        // Selection survives a reload. Losing it on every sync would move the terminal's target out from
+        SelectedHost = SelectionAfterReload(selectedId);
-        // under the user.
-        SelectedHost = Hosts.FirstOrDefault(row => row.EntityId == selectedId) ?? Hosts.FirstOrDefault();
 
         // Both, in this order: the group rows carry a host count, and the sidebar's headings are built from
         // the group rows.
@@ -3035,6 +3047,25 @@ internal sealed partial class VaultViewModel(
         return unreadable;
     }
 
+    /// <summary>Which host a freshly filled <see cref="Hosts"/> leaves selected.</summary>
+    /// <param name="selectedId">Whatever was selected before the list was refilled.</param>
+    /// <remarks>
+    /// <para>
+    /// The selection survives a reload, because losing it on every sync would move the terminal's target out
+    /// from under the user. The first host is the fallback rather than nothing, so that a fresh unlock has
+    /// something under CONNECT.
+    /// </para>
+    /// <para>
+    /// That fallback is skipped while a group card holds the selection, and it has to be: the two grids
+    /// share one mark — see <see cref="OnSelectedHostChanged"/> — so a sync that invented a host would
+    /// quietly unselect a group nobody had touched, once a minute. Read before <see cref="RebuildGroups"/>
+    /// runs, which is where <see cref="SelectedGroup"/> is re-resolved against the rows this pass makes.
+    /// </para>
+    /// </remarks>
+    private HostRowViewModel? SelectionAfterReload(Guid? selectedId) =>
+        Hosts.FirstOrDefault(row => row.EntityId == selectedId)
+        ?? (SelectedGroup is null ? Hosts.FirstOrDefault() : null);
+
     /// <returns>How many buckets would not decrypt.</returns>
     /// <remarks>
     /// The selection survives a reload and a reload never invents one, as the key and credential lists do and
@@ -4710,11 +4741,21 @@ internal sealed partial class VaultViewModel(
     }
 
     /// <summary>Loads the group being acted on into the box, so saving renames it.</summary>
-    /// <remarks>The selected card, or the open group when no card is selected. See <see cref="GroupTarget"/>.</remarks>
+    /// <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.
+    /// </param>
+    /// <remarks>
+    /// Taking it as an argument is what keeps the phone from having to select a group in order to edit one.
+    /// A selection is shared with the host grid now — see <see cref="OnSelectedHostChanged"/> — so a command
+    /// reachable only through <see cref="SelectedGroup"/> would deselect the machine somebody was about to
+    /// connect to, on a screen that draws no group cards at all.
+    /// </remarks>
     [RelayCommand]
-    private void EditGroup()
+    private void EditGroup(HostGroupRowViewModel? group)
     {
-        if (GroupTarget is not { } row)
+        if ((group ?? GroupTarget) is not { } row)
         {
             return;
         }
@@ -4769,6 +4810,12 @@ internal sealed partial class VaultViewModel(
     /// deleted by a sync between the list being drawn and the button being pressed — is ignored rather than
     /// opening an editor on nothing.
     /// </para>
+    /// <para>
+    /// The row is handed to <see cref="EditGroup"/> rather than selected first, which it used to be. A group
+    /// selection now clears the host selection — the two grids share one mark — and the phone draws no group
+    /// cards, so selecting one here would have taken the highlight off the machine in the list with nothing
+    /// on screen to say where it had gone.
+    /// </para>
     /// </remarks>
     [RelayCommand]
     private void EditGroupFromHeading(SidebarGroupHeader? header)
@@ -4779,8 +4826,7 @@ internal sealed partial class VaultViewModel(
             return;
         }
 
-        SelectedGroup = row;
+        EditGroupCommand.Execute(row);
-        EditGroupCommand.Execute(null);
     }
 
     /// <summary>Starts a new group, inside whichever one the screen is showing.</summary>
@@ -7918,6 +7964,16 @@ internal sealed partial class VaultViewModel(
 
     partial void OnSelectedHostChanged(HostRowViewModel? value)
     {
+        // One selection, across both grids. The two lists are drawn one above the other and they are marked
+        // the same way, so two lit cards read as two things chosen — and the buttons underneath them are two
+        // pairs, only one of which would act. Losing a selection leaves the other alone: a null here is what
+        // a filter matching nothing writes, and taking the mark off a group card because a search box
+        // emptied the grid beneath it would be this rule firing at something that is not a choice.
+        if (value is not null)
+        {
+            SelectedGroup = null;
+        }
+
         OnPropertyChanged(nameof(SelectedHostAsksForAPassword));
         OnPropertyChanged(nameof(SelectedHostAuthenticationNote));
         OnPropertyChanged(nameof(ShowsConnectBar));
@@ -7996,8 +8052,18 @@ internal sealed partial class VaultViewModel(
         }
     }
 
+    /// <remarks>
+    /// The other half of the shared selection; see <see cref="OnSelectedHostChanged"/>. Clearing the host
+    /// takes the drawer with it, and that is the point rather than a side effect: a pane about one machine
+    /// cannot go on standing beside a marked group, since nothing on it would be about what is selected.
+    /// </remarks>
     partial void OnSelectedGroupChanged(HostGroupRowViewModel? value)
     {
+        if (value is not null)
+        {
+            SelectedHost = null;
+        }
+
         DisarmIfAimedElsewhere(DeletionTarget.Group, GroupTarget?.EntityId);
 
         OnPropertyChanged(nameof(GroupTarget));

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

@@ -336,6 +336,99 @@ public sealed class HostGridTests : IAsyncLifetime
         vault.ShowsGroupActions.ShouldBeFalse("a pair of buttons with no subject is hidden rather than shown");
     }
 
+    /// <summary>
+    /// The two grids share one selection, so at most one card on the screen is ever lit.
+    /// </summary>
+    /// <remarks>
+    /// <para>
+    /// They are two <c>ListBox</c>es, each holding a selection of its own and each drawing it the same way.
+    /// Left to themselves both stay marked — a group above and a host below — under two pairs of buttons of
+    /// which only one acts on whichever card the eye has settled on. The vault is what joins them.
+    /// </para>
+    /// <para>
+    /// Driven on the screen rather than on the view model alone, because half of the rule lives in the
+    /// controls: clearing the property has to reach the list that is drawing the card, and a selection
+    /// nulled in the view model while the card stays highlighted is the exact failure this is about.
+    /// </para>
+    /// </remarks>
+    [Fact]
+    public async Task TheHostAndGroupGridsShareOneSelection()
+    {
+        await OnTheGridAsync((screen, _) =>
+        {
+            var host = Row(vault, "stage-web");
+
+            vault.OpenHostPaneCommand.Execute(host);
+            Dispatcher.UIThread.RunJobs();
+
+            vault.SelectedGroup = vault.VisibleGroups.Single();
+            Dispatcher.UIThread.RunJobs();
+
+            vault.SelectedHost.ShouldBeNull("choosing a group is choosing something else");
+            vault.SelectedSidebarRow.ShouldBeNull("and the list that draws the hosts is told");
+            screen.HostGrid.SelectedItem.ShouldBeNull();
+            CardFor(screen, host).IsSelected.ShouldBeFalse("the card the pointer left has to go dark");
+            vault.IsDrawerOpen.ShouldBeFalse("a pane about one host cannot stand beside a marked group");
+
+            vault.SelectedHost = host;
+            Dispatcher.UIThread.RunJobs();
+
+            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.
+    /// </remarks>
+    [Fact]
+    public async Task TheGroupsEditButtonStillActsOnTheSelectedCard()
+    {
+        await OnTheGridAsync((screen, _) =>
+        {
+            vault.SelectedGroup = vault.VisibleGroups.Single();
+            Dispatcher.UIThread.RunJobs();
+
+            var edit = 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");
+        });
+    }
+
+    /// <remarks>
+    /// The half of the shared selection that is nobody's gesture. A reload falls back to the first host when
+    /// nothing is selected, which is what puts a target under CONNECT on a fresh unlock — and with one mark
+    /// between the two grids that fallback would quietly unselect a group card every time a sync landed.
+    /// </remarks>
+    [Fact]
+    public async Task ASyncDoesNotTakeTheSelectionOffAGroupCard()
+    {
+        var production = vault.VisibleGroups.Single();
+
+        vault.SelectedGroup = production;
+        vault.SelectedHost.ShouldBeNull("the seed's load left a host selected, and the group took the mark");
+
+        await vault.LoadAsync(Token);
+
+        vault.SelectedHost.ShouldBeNull("the reload invented none under the card that was chosen");
+        vault.SelectedGroup
+            .ShouldNotBeNull("re-found by id, since the reload replaces every row object in the list")
+            .EntityId.ShouldBe(production.EntityId);
+    }
+
     /// <remarks>
     /// The gesture, performed. It is wired in the control rather than bound in the markup — which is exactly
     /// the sort of wiring that compiles whether or not it is connected to anything — and the first half of

tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs

@@ -4333,6 +4333,35 @@ public sealed class ShellFlowTests : IAsyncLifetime
         vault.GroupEditorLabel.ShouldBe("production", "the heading pressed, not the group selected");
     }
 
+    /// <remarks>
+    /// The heading hands its group to the editor rather than selecting it first, and this is why. A group
+    /// selection clears the host selection — the desktop's two grids share one mark — and the phone draws no
+    /// group cards at all, so selecting one here would take the highlight off the machine in the list with
+    /// nothing on screen to say where it had gone, or how to get it back.
+    /// </remarks>
+    [Fact]
+    public async Task AGroupsHeading_LeavesTheChosenMachineChosen()
+    {
+        await UnlockedAsync();
+        var vault = shell.Vault!;
+
+        await AddHostAsync(vault, "prod-db");
+        await AddGroupAsync(vault, "production");
+        await FileAsync(vault, "prod-db", "production");
+
+        var host = vault.Hosts.Single();
+        vault.SelectedHost = host;
+
+        var heading = vault.SidebarRows.OfType<SidebarGroupHeader>().Single(
+            row => string.Equals(row.Label, "production", StringComparison.Ordinal));
+
+        vault.EditGroupFromHeadingCommand.Execute(heading);
+
+        vault.IsEditingGroup.ShouldBeTrue("the editor still opens on the group the heading names");
+        vault.GroupEditorLabel.ShouldBe("production");
+        vault.SelectedHost.ShouldBeSameAs(host, "and the list is still on the machine it was on");
+    }
+
     [Fact]
     public async Task TheUngroupedHeading_OpensNothing()
     {