From 3f419cb19bbeb0a0a53947736678a3cfe8d97ce2 Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit | DodoTech Date: Tue, 4 Aug 2026 16:27:20 +0200 Subject: [PATCH] Let one card be selected at a time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hosts screen draws two grids, one above the other, and each is a ListBox with a selection of its own. Nothing joined them, so a group card and a host card could be lit at the same moment — two chosen things, under two pairs of buttons of which only one would act on whichever the eye had settled on. Both grids mark a selection the same way, so there was nothing on screen to say which of the two the next press belonged to. They share one mark now. Selecting a host clears the group and selecting a group clears the host, and that second one takes the detail pane down with it: a pane about one machine cannot go on standing beside a marked group, because nothing on it would be about what is selected. Losing a selection deliberately clears nothing. A null arrives whenever either list is rebuilt — every keystroke in the filter box and every background sync — and treating that as somebody deselecting would take the mark off a group card because a search emptied the grid beneath it. The reload needed the same guard for the same reason. It falls back to the first host when nothing is selected, which is what puts a target under CONNECT on a fresh unlock; with one mark between the two grids that fallback would have unselected a group nobody had touched, once a minute. It is skipped while a group holds the selection, and it moved into a method of its own because the comment saying why pushed ReloadHostsAsync past sixty lines. One consequence needed handling rather than accepting. The phone's only route into the group editor is a button on a heading in the host list, and it worked by selecting the group first — which under this rule takes the highlight off the machine somebody was about to connect to, on a screen that draws no group cards to say where it has gone. EditGroup takes the group as an argument now: the heading passes its own row, and the desktop's button beside the cards passes nothing and still means "the card that is selected". What the tests hold is the half that 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 — so the rule is driven on the real screen, in both directions, against the ListBoxes' own SelectedItem. The desktop's EDIT is pressed through its binding for the same kind of reason: a command refusing the button's empty parameter would be a button that never fires, and nothing about the markup would say so. --- .../Views/HostsScreen.axaml | 5 + .../ViewModels/VaultViewModel.cs | 82 ++++++++++++++-- .../HostGridTests.cs | 93 +++++++++++++++++++ .../ShellFlowTests.cs | 29 ++++++ 4 files changed, 201 insertions(+), 8 deletions(-) diff --git a/src/DodoSSH.Client.App/Views/HostsScreen.axaml b/src/DodoSSH.Client.App/Views/HostsScreen.axaml index 77700f9..40bead6 100644 --- a/src/DodoSSH.Client.App/Views/HostsScreen.axaml +++ b/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 diff --git a/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs index 65373b8..284aac9 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs @@ -1365,6 +1365,14 @@ internal sealed partial class VaultViewModel( [ObservableProperty] private VaultChoiceViewModel? selectedTargetVault; + /// + /// The host card that is selected, or null when none is. + /// + /// + /// The application's selection, which everything that acts on a host reads — connecting, editing, + /// deleting. It shares one selection with : selecting a host takes the + /// mark off a group card and the other way about. See . + /// [ObservableProperty] private HostRowViewModel? selectedHost; @@ -1392,8 +1400,14 @@ internal sealed partial class VaultViewModel( /// The group card that is selected, or null when none is. /// /// + /// /// 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 . + /// + /// + /// It shares one selection with . Setting either clears the other, so + /// exactly one card on the screen is ever lit. See . + /// /// [ObservableProperty] private HostGroupRowViewModel? selectedGroup; @@ -2915,9 +2929,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 - // under the user. - SelectedHost = Hosts.FirstOrDefault(row => row.EntityId == selectedId) ?? Hosts.FirstOrDefault(); + SelectedHost = SelectionAfterReload(selectedId); // Both, in this order: the group rows carry a host count, and the sidebar's headings are built from // the group rows. @@ -2927,6 +2939,25 @@ internal sealed partial class VaultViewModel( return unreadable; } + /// Which host a freshly filled leaves selected. + /// Whatever was selected before the list was refilled. + /// + /// + /// 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. + /// + /// + /// That fallback is skipped while a group card holds the selection, and it has to be: the two grids + /// share one mark — see — so a sync that invented a host would + /// quietly unselect a group nobody had touched, once a minute. Read before + /// runs, which is where is re-resolved against the rows this pass makes. + /// + /// + private HostRowViewModel? SelectionAfterReload(Guid? selectedId) => + Hosts.FirstOrDefault(row => row.EntityId == selectedId) + ?? (SelectedGroup is null ? Hosts.FirstOrDefault() : null); + /// How many buckets would not decrypt. /// /// The selection survives a reload and a reload never invents one, as the key and credential lists do and @@ -4527,11 +4558,21 @@ internal sealed partial class VaultViewModel( } /// Loads the group being acted on into the box, so saving renames it. - /// The selected card, or the open group when no card is selected. See . + /// + /// 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 . 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. + /// + /// + /// 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 — so a command + /// reachable only through would deselect the machine somebody was about to + /// connect to, on a screen that draws no group cards at all. + /// [RelayCommand] - private void EditGroup() + private void EditGroup(HostGroupRowViewModel? group) { - if (GroupTarget is not { } row) + if ((group ?? GroupTarget) is not { } row) { return; } @@ -4577,6 +4618,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. /// + /// + /// The row is handed to 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. + /// /// [RelayCommand] private void EditGroupFromHeading(SidebarGroupHeader? header) @@ -4587,8 +4634,7 @@ internal sealed partial class VaultViewModel( return; } - SelectedGroup = row; - EditGroupCommand.Execute(null); + EditGroupCommand.Execute(row); } /// Starts a new group, inside whichever one the screen is showing. @@ -7633,6 +7679,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)); @@ -7711,8 +7767,18 @@ internal sealed partial class VaultViewModel( } } + /// + /// The other half of the shared selection; see . 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. + /// partial void OnSelectedGroupChanged(HostGroupRowViewModel? value) { + if (value is not null) + { + SelectedHost = null; + } + DisarmIfAimedElsewhere(DeletionTarget.Group, GroupTarget?.EntityId); OnPropertyChanged(nameof(GroupTarget)); diff --git a/tests/DodoSSH.Client.App.Layout.Tests/HostGridTests.cs b/tests/DodoSSH.Client.App.Layout.Tests/HostGridTests.cs index c193c7a..29a2789 100644 --- a/tests/DodoSSH.Client.App.Layout.Tests/HostGridTests.cs +++ b/tests/DodoSSH.Client.App.Layout.Tests/HostGridTests.cs @@ -259,6 +259,99 @@ public sealed class HostGridTests : IAsyncLifetime vault.ShowsGroupActions.ShouldBeFalse("a pair of buttons with no subject is hidden rather than shown"); } + /// + /// The two grids share one selection, so at most one card on the screen is ever lit. + /// + /// + /// + /// They are two ListBoxes, 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. + /// + /// + /// 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. + /// + /// + [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"); + }); + } + + /// + /// 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 VaultViewModel.EditGroup. 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. + /// + [Fact] + public async Task TheGroupsEditButtonStillActsOnTheSelectedCard() + { + await OnTheGridAsync((screen, _) => + { + vault.SelectedGroup = vault.VisibleGroups.Single(); + Dispatcher.UIThread.RunJobs(); + + var edit = screen.GetVisualDescendants() + .OfType