From 0c0ac94312bbd6415436e58303969dce56cabc6b Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit | DodoTech Date: Mon, 3 Aug 2026 16:16:18 +0200 Subject: [PATCH] Open a group by double-clicking it, and say where you are MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ONE PRESS WAS DOING TWO JOBS. A group card was the only place a group could be selected — it is what EDIT and DELETE aim at — and it was also the control that narrowed the grid to that group. So there was no way to name a group in order to rename it without every host outside it leaving the screen at the same moment, and no way back except a SHOW ALL button that appeared beside the heading. Two gestures instead. A click selects and does nothing else; a double-click opens, which is what the host cards below already do to get a shell and what the transfers screen's directories already do to go inside one. The grid now has one vocabulary rather than one per list. The gesture is wired in the code-behind beside the host one, and guarded the same way: a double-click on the space around the cards must not open whichever group happened to be selected. THE CARDS ARE ONE LEVEL NOW, not every group in the keychain. Groups nest, and drawing all of them flat was the only honest thing to do while a card was a filter — a filter nobody can see is a filter nobody can turn off. Once opening one became navigation the cards became its contents, and VisibleGroups is that level beside Groups the way VisibleHosts sits beside Hosts. Groups itself is untouched: it is what every lookup reads and what the phone's headings are built from, and the phone binds none of the new members. Which is what the trail is for. A level with no name and no way out is a grid that has quietly hidden things, so a breadcrumb sits above the cards — drawn exactly as the transfers screen draws a directory path, same flat crumbs and same separator, because it is the same control answering the same question and a window with two breadcrumbs that look different has two ideas of what a path is. The first crumb is always there and always goes back to every host, which is what SHOW ALL was; that button went with it, because a control that only says "stop" beside a trail that says where you are is a second control for one job, and this one also gets you back one level rather than all the way. EDIT AND DELETE AIM AT GroupTarget: the selected card, or the open group when no card is selected. Without the fallback a group with nothing 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 — and the pair is now hidden with nothing to act on rather than shown doing nothing. A DANGLING PARENT AND A CYCLE BOTH HAD TO END UP SOMEWHERE REACHABLE. Neither is prevented: a parent id may point at a group deleted on another machine, and two clients can each re-parent A under B and B under A while offline, which no merge can see because the pointer is inside the payload. EffectiveParents promotes both to the outermost level, which is the same degradation the resolver's visited set produces for inheritance. The repair for either is the group's own editor and the editor is opened from its card, so a group drawn nowhere would be a broken state with the fix locked inside it. Three tests in HostGridTests: the split rule through the properties the cards bind, the pointer gesture itself in two windows so that "one press still only selects" is asserted separately from the pair, and a nested group opened, emptied of cards and walked back out of one level. The last presses the trail as it is actually rendered rather than calling the command, because a crumb reaches the vault through a $parent binding — a string that compiles whether or not it resolves, and would otherwise leave a row of buttons that do nothing. 85 layout tests and 234 shell-flow tests pass. Manual-checks 3.2, 7.6 and 7.7 follow the new gestures, and 3.2a and 3.4a are new: nesting, and the two states above, both of which need two machines and neither of which headless Avalonia can reach. --- docs/design-import-gaps.md | 5 +- docs/manual-checks.md | 45 ++- .../Views/HostsScreen.axaml | 72 +++- .../Views/HostsScreen.axaml.cs | 23 ++ .../ViewModels/VaultViewModel.cs | 315 +++++++++++++++--- .../HostGridTests.cs | 169 +++++++++- 6 files changed, 549 insertions(+), 80 deletions(-) diff --git a/docs/design-import-gaps.md b/docs/design-import-gaps.md index c6c7092..170e643 100644 --- a/docs/design-import-gaps.md +++ b/docs/design-import-gaps.md @@ -101,8 +101,9 @@ the chrome, hosts and terminals, file transfer, the vault, teams, and preference > **The hosts screen became a grid of cards** — groups above, hosts below — and the 268-pixel host sidebar > went with it. That column was choosing among forty machines *and* editing one of them at two-thirds > width; the grid took the first job at full width and a 304-pixel right-hand drawer took the second. The -> drawer collapses when nothing is selected, which is most of the time. Pressing a group card narrows the -> grid to that group; `SHOW ALL` is the way back. +> drawer collapses when nothing is selected, which is most of the time. Pressing a group card selects it; +> double-pressing one opens it, and a breadcrumb trail above the cards is where you are and the way back — +> the same trail the transfers screen draws over a directory. > > **The type scale went up a point and the text ramp went white.** `#E3E7F4` was a blue-tinted white on > blue-black surfaces, which costs contrast twice — once for being darker than white and once for sharing diff --git a/docs/manual-checks.md b/docs/manual-checks.md index 59dac04..b64a08e 100644 --- a/docs/manual-checks.md +++ b/docs/manual-checks.md @@ -283,11 +283,42 @@ Make two groups and file some hosts into each through the host editor. flat wrap, in one order, with nothing between them. **There is no heading and no fold on the desktop** — the headings, their chevrons and UNGROUPED are the phone's, whose list has no room for a row of group cards. -**Then press a group card.** The grid narrows to that group's hosts, the card is marked as chosen, and -SHOW ALL appears beside GROUPS. Pressing it brings the rest back and unmarks the card. +**Then press a group card once.** It is marked as chosen and **nothing else happens** — the grid still holds +every host, and EDIT and DELETE now aim at that group. **Then double-press it.** The group opens: the hosts +narrow to the ones filed under it, the trail above the cards reads `ALL HOSTS › ›`, and the card grid +shows what is *inside* that group rather than every group in the keychain. Pressing ALL HOSTS brings the rest +back. -**Failure means:** a full-width bar with a chevron between the cards is the old grouping coming back through -`SidebarRows` — the desktop grid binds `VisibleHosts`. See `HostsScreen.axaml`. +**Failure means:** if one press still narrows the grid, the card `ListBox` is bound to the wrong property — +`SelectedItem` is `SelectedGroup`, and only `OpenGroupCommand` writes `GroupFilter`. A full-width bar with a +chevron between the cards is the old grouping coming back through `SidebarRows`; the desktop grid binds +`VisibleHosts`. See `HostsScreen.axaml`. + +### 3.2a A group inside a group, and the way back out + +Make two groups and file one under the other with the parent picker in the group editor. + +**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 › ›`. 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. + +**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 +case `EffectiveParents` promotes: see 3.4a. + +### 3.4a A parent that dangles, or a cycle · **needs two machines** + +Rare, and the reason it is here is that the repair for both is inside the group's own editor, which is +opened from its card. On A, file group X under group Y and sync. On B, sync, delete Y, sync. Back on A, sync. +For the cycle: with both machines offline, file X under Y on A and Y under X on B, then sync both. + +**Pass:** in both cases every group still has a card at the outermost level, so every one of them can be +opened and its parent cleared. Nothing is missing from the grid, and the trail never loops. + +**Failure means:** a group drawn nowhere is a group that can never be repaired — the fix is locked inside the +state that needs fixing. See `VaultViewModel.EffectiveParents`. ### 3.3 Deleting a group with hosts in it @@ -653,6 +684,9 @@ new group's name, the host counts under both group cards change, and the status group the host is *already* in; over another **host** card, which is deliberately not a target now that there are no headings to say which group it would mean; and over the empty space around the cards. +**The targets are the cards on screen, which are one level** — see 3.2a. Filing into a group nested under +another means opening the outer one first, exactly as moving a file into a subfolder does. + **And getting a host back out** is the host's own editor — pick "No group" in its picker. There is no UNGROUPED target on the desktop any more, because there is no UNGROUPED heading for it to be. @@ -677,6 +711,9 @@ space around the cards connects to nothing. as 2.16 on the other screen, and here it would make the grid unusable. Empty space that connects means the double-tap handler has lost its check that the pointer was over a card. +**And the same two gestures on the group cards above**, where they mean select and open rather than select +and connect — the space around *those* cards must open nothing, for the same reason. See 3.2. + ### 7.8 The highlight clears after a drag that goes nowhere Drag a host over a group card and release outside the grid, or press Escape mid-drag. diff --git a/src/DodoSSH.Client.App/Views/HostsScreen.axaml b/src/DodoSSH.Client.App/Views/HostsScreen.axaml index 2a589f7..96e1d88 100644 --- a/src/DodoSSH.Client.App/Views/HostsScreen.axaml +++ b/src/DodoSSH.Client.App/Views/HostsScreen.axaml @@ -177,15 +177,13 @@ - + + + + + + @@ -204,21 +239,28 @@ every other list in this application draws a selected row. Buttons would have needed a Classes.active binding per card and a second copy of "which one". - SelectedItem is GroupFilter and not SelectedGroup. The two are nearly the same thing here — - GroupFilter assigns SelectedGroup — but only one of them may narrow the grid; see the property - for why the phone's own use of SelectedGroup must not. + ◆ ONE CLICK SELECTS, TWO OPEN. SelectedItem is the selection and nothing else now — the + gesture that opens a group is a double-click, wired in the code-behind beside the one that + 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. + + ◆ 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. ◆ 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 grid, and the headings are gone — see the note on the host grid below. A group is a thing that - is drawn once, at the top, where the eye already goes to narrow the grid; a target that is also - the label of what you are aiming at needs no explaining. See the code-behind for the gesture. + is drawn once, at the top, where the eye already goes; a target that is also the label of what + you are aiming at needs no explaining. See the code-behind for the gesture. --> + IsVisible="{Binding HasVisibleGroups}" + ItemsSource="{Binding VisibleGroups}" + SelectedItem="{Binding SelectedGroup}" + ToolTip.Tip="Press a group to select it, double-press to open it. Drag a host card onto one to file it there."> diff --git a/src/DodoSSH.Client.App/Views/HostsScreen.axaml.cs b/src/DodoSSH.Client.App/Views/HostsScreen.axaml.cs index 8c8734a..e89c726 100644 --- a/src/DodoSSH.Client.App/Views/HostsScreen.axaml.cs +++ b/src/DodoSSH.Client.App/Views/HostsScreen.axaml.cs @@ -83,6 +83,13 @@ internal sealed partial class HostsScreen : UserControl // password box above it, and a host that asks for a password still needs it typed first. HostGrid.DoubleTapped += OnHostActivated; + // And a group card opens the group, on the same gesture, for the same reason: going inside + // something by double-clicking it is what the transfers screen's directories do and what every file + // manager does. One click used to open a group, which made the card that names a group and the + // control that narrows the grid to it the same press — so there was no way to select a group in + // order to rename it without also losing sight of every host outside it. + GroupGrid.DoubleTapped += OnGroupActivated; + // Tunnelled, so the card under the pointer is read before the ListBox has answered the press itself. // Bubbling would work for the drag but not for the menu: by then the control has already decided // what is selected, and the menu is about to open against it. @@ -154,6 +161,22 @@ internal sealed partial class HostsScreen : UserControl } } + /// + /// Opens the group card that was double-clicked. + /// + /// + /// Guarded over the space around the cards exactly as the host grid's is, and it is the same mistake + /// being guarded against: an unguarded handler would open whichever group happened to be selected when + /// somebody double-clicked the gap beside it, throwing every host outside that group off the screen. + /// + private void OnGroupActivated(object? sender, TappedEventArgs e) + { + if (Vault is { } vault && RowUnder(e.Source) is HostGroupRowViewModel group) + { + vault.OpenGroupCommand.Execute(group); + } + } + /// /// Points the menu at whatever was right-clicked. /// diff --git a/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs index 7d1d5a7..a6f3370 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs @@ -67,6 +67,17 @@ internal sealed class HostGroupRowViewModel(VaultItem group, in internal string Description => hostCount == 1 ? "1 host" : $"{hostCount} hosts"; } +/// One step of the path into the groups, as a button in a breadcrumb trail. +/// What the step is called. +/// The group it opens, or null for the step that shows every host again. +/// +/// The same shape the transfers screen's trail uses — see CrumbViewModel — and drawn the same way, +/// because it answers the same question: a directory pane and a grid of groups both have to say where the +/// thing on screen came from and offer a way back out. The row rather than its id, because +/// holds a row, and an id would only be looked up again. +/// +internal sealed record GroupCrumbViewModel(string Name, HostGroupRowViewModel? Group); + /// An entry in the host editor's group picker. /// The group, or null for "no group". /// What to show. @@ -1076,8 +1087,8 @@ internal sealed partial class VaultViewModel( "No hosts yet. Press + NEW HOST to add one, or import the machines already in this computer's " + "~/.ssh/config from Preferences.", (_, not null, 0) => - "Nothing is filed under this group yet. Press SHOW ALL, then drag a host card onto this group's " - + "card — or choose the group in a host's own editor.", + "Nothing is filed under this group yet. Press ALL HOSTS above, then drag a host card onto this " + + "group's card — or choose the group in a host's own editor.", _ => "No host matches that. The name, the address and the notes are all searched.", }; @@ -1100,8 +1111,51 @@ internal sealed partial class VaultViewModel( internal ObservableCollection SidebarRows { get; } = []; /// The groups in this vault, with the number of hosts filed under each. + /// + /// Every one of them, flat. This is what a group is looked up in and what the phone's headings are built + /// from; is the desktop's one level of it. + /// internal ObservableCollection Groups { get; } = []; + /// + /// The group cards the desktop is drawing: what is inside the group that is open, or the outermost + /// groups when none is. + /// + /// + /// + /// is to this what is to : the whole + /// collection beside the part of it on screen. The grid used to draw every group at once, which was the + /// only honest thing to do while pressing a card meant nothing but "narrow the list" — a card was a + /// filter, and every filter has to be reachable. Opening one is navigation, so the cards became the + /// contents of wherever the trail says you are. + /// + /// + /// A group whose parent this vault has not got is drawn at the outermost level rather than nowhere, and + /// so is one caught in a parent cycle. Both are states two offline edits can produce and neither can be + /// repaired from a screen that will not draw the group — see . + /// + /// + internal ObservableCollection VisibleGroups { get; } = []; + + /// + /// The path down to the group that is open: every host, then each group above it, then it. + /// + /// + /// Always at least one crumb, and the first one is the way back to every host — which is what it is for. + /// A grid whose cards are one level of a tree needs somewhere to say which level, and the same control + /// is the way out of it; without that the only way back would be a button that says so, which is what + /// SHOW ALL was and what this replaces. + /// + internal ObservableCollection GroupTrail { get; } = []; + + /// Whether there are any group cards to draw at this level. + /// + /// Separate from , which is about the vault. A group with nothing inside it is an + /// ordinary thing to open, and the trail and the group's own buttons have to stay on screen when it is — + /// so it is the card grid alone that folds away, not the panel around it. + /// + internal bool HasVisibleGroups => VisibleGroups.Count > 0; + /// The saved commands in this vault, unpushed local state included. /// /// Held here rather than on the screen that shows them, for the reason every other list is: this is where @@ -1205,41 +1259,59 @@ internal sealed partial class VaultViewModel( [ObservableProperty] private ISidebarRow? selectedSidebarRow; + /// + /// 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 . + /// [ObservableProperty] private HostGroupRowViewModel? selectedGroup; /// - /// The group the hosts grid is narrowed to, or null for every host. + /// The group that is open: the one whose contents the screen is showing, or null for every host. /// /// /// - /// The desktop draws its groups as cards above the hosts, and pressing one narrows what is under it. - /// This is that choice. is the way back to all of them, and it is - /// an explicit control rather than a second press on the chosen card: the cards are a - /// ListBox so that the selected one is marked by the same style every other list in this - /// application uses, and a ListBox does not unselect on a second click. + /// Set by and by nothing else. It decides three things at once — which hosts the + /// grid holds, which groups the cards hold, and what the trail says — which is what makes it "where you + /// are" rather than a filter that happens to be on. /// /// - /// Separate from , and it sets it. The two answer different questions — - /// "what is the grid showing" and "what would EDIT and DELETE act on" — and on the desktop pressing a - /// card means both, which is why the change handler assigns one from the other. They are not one - /// property because the phone sets on its own account: - /// selects a group in order to open its editor, and a single property - /// would have made opening that editor silently filter the phone's host list to the group being renamed. + /// Separate from , and no longer sets it. The two answer different + /// questions — "what is on screen" and "what would EDIT and DELETE act on" — 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; is where the two meet. /// /// [ObservableProperty] private HostGroupRowViewModel? groupFilter; - /// Whether the grid is showing one group rather than every host. - internal bool IsFilteredByGroup => GroupFilter is not null; - - /// Shows every host again. + /// + /// Opens a group, or every host when handed null. + /// + /// + /// + /// A double-click on a card, or a press on a crumb of the trail. Deliberately not a single click, which + /// is what it was: a card is the only place a group can be selected, and a gesture that both selected a + /// group and threw the rest of the grid away left no way to rename one without first losing sight of + /// everything else. Double-clicking to go inside something is what the transfers screen's directories do + /// and what the host cards beneath these do to open a shell, so the grid now has one vocabulary rather + /// than one per list. + /// + /// + /// The selection is dropped first, and it has to be: the cards are about to be redrawn one level along, + /// and a selection pointing at a card that is no longer on screen would aim EDIT and DELETE at something + /// nobody can see. Null is a real argument here rather than a missing one — it is the trail's first + /// crumb, and it is the way back out. + /// + /// [RelayCommand] - private void ClearGroupFilter() + private void OpenGroup(HostGroupRowViewModel? group) { - GroupFilter = null; SelectedGroup = null; + GroupFilter = group; } /// What the group name box holds, for both creating and renaming. @@ -2089,8 +2161,26 @@ internal sealed partial class VaultViewModel( /// internal bool IsConfirmingGroupDeletion => PendingDeletion?.Target is DeletionTarget.Group; + /// + /// What the group panel's EDIT and DELETE act on: the card that is selected, or the group that is open. + /// + /// + /// 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. + /// + internal HostGroupRowViewModel? GroupTarget => SelectedGroup ?? GroupFilter; + /// Whether the group panel's buttons are showing. - internal bool ShowsGroupActions => !IsConfirmingGroupDeletion; + /// + /// 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 . + /// + internal bool ShowsGroupActions => GroupTarget is not null && !IsConfirmingGroupDeletion; /// Whether this vault has any groups, which is what makes the sidebar draw headings. internal bool HasGroups => Groups.Count > 0; @@ -2769,27 +2859,148 @@ internal sealed partial class VaultViewModel( Groups.Add(new HostGroupRowViewModel(group, count)); } - // Never defaulted to the first row, as the key and credential lists are not: this selection is what - // RENAME and DELETE aim at, and a background sync that picked a group would point them at one nobody - // chose. - SelectedGroup = Groups.FirstOrDefault(row => row.EntityId == selectedId); - - // Re-resolved by id for the reason the selection above is: every row object here is replaced on - // every reload, so a filter holding the old one would go on narrowing the grid to a group that is no - // longer in the list — and the card the user could press to clear it would be a different object - // that never matched. A group deleted by a sync clears the filter, which is the honest answer: the - // grid comes back to every host rather than to none. + // Re-resolved by id rather than kept: every row object here is replaced on every reload, so an open + // group holding the old one would go on showing a group that is no longer in the list — and the + // crumb the user could press to leave it would be a different object that never matched. A group + // deleted by a sync closes itself, which is the honest answer: the screen comes back to every host + // rather than to none. // - // This assignment is a new row object whenever there is a filter at all, so it always fires + // This assignment is a new row object whenever a group is open at all, so it always fires // OnGroupFilterChanged and therefore an extra RebuildVisibleHosts before the caller's own. That is // wasted work rather than a bug — Hosts is already filled by the time this runs, so both passes see // the same thing — and it is left rather than dodged by writing the backing field, because writing - // the field would skip SelectedGroup and IsFilteredByGroup with it. + // the field would skip the cards, the trail and GroupTarget with it. GroupFilter = Groups.FirstOrDefault(row => row.EntityId == filteredId); + // Unconditionally, because the assignment above is a no-op — and fires nothing — whenever no group + // was open, and the cards still have to be rebuilt out of the row objects this pass just made. + RebuildGroupLevel(); + + // Out of the cards on screen rather than out of every group, and after the level has been rebuilt: + // this is the card ListBox's own selection, and a row it is not showing is one the control would + // null straight back out again. + // + // Never defaulted to the first row, as the key and credential lists are not: this selection is what + // EDIT and DELETE aim at, and a background sync that picked a group would point them at one nobody + // chose. + SelectedGroup = VisibleGroups.FirstOrDefault(row => row.EntityId == selectedId); + OnPropertyChanged(nameof(HasGroups)); } + /// + /// Refills the group cards and the trail from whichever group is open. + /// + /// + /// Both together because they are two halves of one answer: the cards are what is inside the open group + /// and the trail is how it was reached, and a pass that rebuilt one without the other would draw a level + /// under a path that does not lead to it. + /// + private void RebuildGroupLevel() + { + var parents = EffectiveParents(); + var open = GroupFilter?.EntityId; + + VisibleGroups.Clear(); + + foreach (var row in Groups.Where(row => parents.GetValueOrDefault(row.EntityId) == open)) + { + VisibleGroups.Add(row); + } + + GroupTrail.Clear(); + + // Always first, always there, and it is the way out — see GroupTrail. The name is what the grid + // below shows when nothing is open, rather than the vault's, because that is the choice being + // offered: this crumb widens the screen back to every machine in it. + GroupTrail.Add(new GroupCrumbViewModel("ALL HOSTS", null)); + + foreach (var row in Ancestry(open, parents)) + { + GroupTrail.Add(new GroupCrumbViewModel(row.Label, row)); + } + + OnPropertyChanged(nameof(HasVisibleGroups)); + } + + /// + /// Which group each one sits under, with anything a walk upwards cannot get out of promoted to the + /// outermost level. + /// + /// + /// + /// Two things are promoted, and both are states this application has decided to survive rather than + /// prevent. A parent id this vault has not got is a group deleted on another machine — the reference is + /// allowed to dangle, because preventing it would mean one delete rewriting every item naming the + /// deleted thing. A cycle is two clients each re-parenting A under B and B under A while offline, which + /// no merge can see because the pointer is inside the payload. See + /// . + /// + /// + /// Both have to end up somewhere the user can reach. The repair for either is the group's own + /// editor, and the editor is opened from the card — so a group left inside a card nobody can open, or + /// inside a cycle no walk terminates in, would be a broken state with the fix locked inside it. Promoting + /// to a root is the same degradation the resolver's visited set produces for inheritance: a cycle reads + /// as a flat run of top-level groups. + /// + /// + private Dictionary EffectiveParents() + { + var stated = Groups.ToDictionary(row => row.EntityId, row => row.Group.ParentId); + var parents = new Dictionary(stated.Count); + + foreach (var (id, parent) in stated) + { + parents[id] = parent is { } wanted && stated.ContainsKey(wanted) ? wanted : null; + } + + foreach (var id in stated.Keys) + { + if (!ReachesTheTop(id)) + { + parents[id] = null; + } + } + + return parents; + + bool ReachesTheTop(Guid id) + { + var visited = new HashSet(); + Guid? current = id; + + while (current is { } step && visited.Add(step)) + { + current = parents[step]; + } + + return current is null; + } + } + + /// The groups from the outermost down to the one that is open, or nothing when none is. + /// + /// Walked against rather than against the stated parents, so the trail + /// cannot lead through a group the cards will not draw — and so that it terminates, which is what the + /// promotion above buys: a cycle has no parent left to follow. + /// + private List Ancestry(Guid? open, Dictionary parents) + { + var trail = new List(); + var current = open; + + while (current is { } id + && Groups.FirstOrDefault(row => row.EntityId == id) is { } row) + { + trail.Add(row); + current = parents.GetValueOrDefault(id); + } + + trail.Reverse(); + + return trail; + } + /// Refills the sidebar's list from and the filter. /// /// The selection is captured and restored around the rebuild, and that is not tidiness — it is what @@ -3690,11 +3901,12 @@ internal sealed partial class VaultViewModel( await AutoSyncAsync(cancellationToken).ConfigureAwait(true); } - /// Loads the selected group's name into the box, so saving renames it. + /// 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 . [RelayCommand] private void EditGroup() { - if (SelectedGroup is not { } row) + if (GroupTarget is not { } row) { return; } @@ -3923,17 +4135,21 @@ internal sealed partial class VaultViewModel( Status = string.Empty; } - /// Asks whether the selected group should go. + /// Asks whether the group being acted on should go. /// /// The count is the whole reason this asks rather than acting. Deleting a group does not delete the hosts /// in it and deliberately does not rewrite them either — they keep an id that no longer resolves and turn /// 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. + /// + /// Aims where EDIT does: at the selected card, or at the open group when no card is selected. See + /// . + /// /// [RelayCommand] private void DeleteGroup() { - if (SelectedGroup is not { } row) + if (GroupTarget is not { } row) { return; } @@ -6553,20 +6769,33 @@ internal sealed partial class VaultViewModel( partial void OnSelectedGroupChanged(HostGroupRowViewModel? value) { - DisarmIfAimedElsewhere(DeletionTarget.Group, value?.EntityId); + DisarmIfAimedElsewhere(DeletionTarget.Group, GroupTarget?.EntityId); + + OnPropertyChanged(nameof(GroupTarget)); + OnPropertyChanged(nameof(ShowsGroupActions)); } /// - /// Sets the selection as well as the filter, because on the desktop pressing a card means both — see the - /// property. Assigning the same value again is a no-op, so this and - /// cannot chase each other. + /// + /// Leaves the selection alone, which is the change that split one click into two gestures: this fires + /// from , and that command has already dropped the selection before assigning + /// here — see the property. + /// + /// + /// The cards are rebuilt before the hosts because both are the same move, and the deletion is disarmed + /// against rather than against the group itself: a reload hands this a new row + /// object for the group already open, and taking a question away from under somebody because the row + /// behind it was replaced is exactly what compares ids to avoid. + /// /// partial void OnGroupFilterChanged(HostGroupRowViewModel? value) { - SelectedGroup = value; + DisarmIfAimedElsewhere(DeletionTarget.Group, GroupTarget?.EntityId); - OnPropertyChanged(nameof(IsFilteredByGroup)); + OnPropertyChanged(nameof(GroupTarget)); + OnPropertyChanged(nameof(ShowsGroupActions)); + RebuildGroupLevel(); RebuildVisibleHosts(); } diff --git a/tests/DodoSSH.Client.App.Layout.Tests/HostGridTests.cs b/tests/DodoSSH.Client.App.Layout.Tests/HostGridTests.cs index f34a3cc..2008ee3 100644 --- a/tests/DodoSSH.Client.App.Layout.Tests/HostGridTests.cs +++ b/tests/DodoSSH.Client.App.Layout.Tests/HostGridTests.cs @@ -3,6 +3,7 @@ using Avalonia.Controls; using Avalonia.Headless; using Avalonia.Input; using Avalonia.Interactivity; +using Avalonia.Threading; using Avalonia.VisualTree; using DodoSSH.Client.App.Views; using DodoSSH.Client.Session; @@ -208,32 +209,141 @@ public sealed class HostGridTests : IAsyncLifetime } /// - /// Pressing a group card narrows the grid to that group, and pressing SHOW ALL brings the rest back. - /// Driven through the property the card's ListBox binds rather than through a click, because - /// what is worth holding is the rule — the filter is a property of the grid, and it also moves the - /// selection the group's own EDIT and DELETE act on. A click would test Avalonia's SelectedItem - /// binding, which is not this application's code. + /// + /// 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. + /// + /// + /// Driven through the properties the cards bind rather than through a click, because what is worth + /// holding here is the rule; the pointer is put on the gesture itself in the test below. A click would + /// otherwise be testing Avalonia's SelectedItem binding, which is not this application's code. + /// /// [Fact] - public async Task ChoosingAGroupNarrowsTheGridAndAimsTheGroupButtonsAtIt() + public async Task SelectingAGroupAimsItsButtonsAtItAndOpeningOneNarrowsTheGrid() { - var production = vault.Groups.Single(); + await vault.MoveHostToGroupCommand.ExecuteAsync( + new HostGroupMove(Row(vault, "prod-db"), vault.Groups.Single().EntityId)); - vault.MoveHostToGroupCommand.Execute( - new HostGroupMove(Row(vault, "prod-db"), production.EntityId)); + // Re-found after the move, because the reload it ends with replaces every row in the list. + var production = vault.VisibleGroups.Single(); - vault.GroupFilter = production; + vault.SelectedGroup = production; + + vault.GroupFilter.ShouldBeNull("one press selects a group and does not open it"); + vault.VisibleHosts.Count.ShouldBe(2, "so the grid still holds the hosts outside it"); + vault.GroupTarget.ShouldBeSameAs(production, "what EDIT and DELETE act on"); + vault.ShowsGroupActions.ShouldBeTrue(); + + vault.OpenGroupCommand.Execute(production); vault.VisibleHosts.Select(row => row.Label) - .ShouldBe(["prod-db"], "only what is filed under the chosen group"); + .ShouldBe(["prod-db"], "only what is filed under the group that is open"); - vault.SelectedGroup.ShouldBeSameAs(production, "what EDIT and DELETE act on"); - vault.IsFilteredByGroup.ShouldBeTrue(); + vault.GroupTrail.Select(crumb => crumb.Name).ShouldBe(["ALL HOSTS", "production"]); - vault.ClearGroupFilterCommand.Execute(null); + 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"); - vault.VisibleHosts.Count.ShouldBe(2, "SHOW ALL brings back the hosts outside the group"); - vault.SelectedGroup.ShouldBeNull("nothing is aimed at once the filter is off"); + // 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. + vault.OpenGroupCommand.Execute(vault.GroupTrail[0].Group); + + vault.VisibleHosts.Count.ShouldBe(2, "ALL HOSTS brings back the hosts outside the group"); + 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"); + } + + /// + /// 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 + /// what it asserts is the half that would go unnoticed: one press must still only select, or the split + /// bought nothing. + /// + [Fact] + public async Task DoubleClickingAGroupCardOpensIt() + { + // One press, in a window of its own, because a second pair of clicks in the same one is the gesture + // this is separating that press from. + await OnTheGridAsync((screen, window) => + { + var centre = Centre(GroupCard(screen), window); + + window.MouseDown(centre, MouseButton.Left); + window.MouseUp(centre, MouseButton.Left); + + Dispatcher.UIThread.RunJobs(); + + vault.SelectedGroup.ShouldNotBeNull("one press selects the card"); + vault.GroupFilter.ShouldBeNull("and opens nothing"); + }); + + await OnTheGridAsync((screen, window) => + { + var centre = Centre(GroupCard(screen), window); + + window.MouseDown(centre, MouseButton.Left); + window.MouseUp(centre, MouseButton.Left); + window.MouseDown(centre, MouseButton.Left); + window.MouseUp(centre, MouseButton.Left); + + Dispatcher.UIThread.RunJobs(); + + vault.GroupFilter.ShouldNotBeNull().Label.ShouldBe("production"); + vault.GroupTrail.Select(crumb => crumb.Name).ShouldBe(["ALL HOSTS", "production"]); + + // And out again through the trail as it is actually drawn, rather than through the command. A + // crumb is an item in a template and the command it presses is the vault's, so the two are + // joined by a $parent binding — which is a string that compiles whether or not it resolves, and + // would leave a trail of buttons that do nothing. + var back = screen.GetVisualDescendants() + .OfType