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 16183d3..0486b65 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