diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstancePicker.razor b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstancePicker.razor index 15f9ac8d..c2d66c62 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstancePicker.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstancePicker.razor @@ -2,7 +2,9 @@ Instance Picker — a combobox whose dropdown is the site's area tree (design 2026-09-08). The field shows the selected instance; the panel hosts the shared TreeView over InstanceTreeBuilder's forest with every area collapsed. Clicking an - area row toggles it, clicking an instance row selects it and closes the panel. + area row toggles it, activating an instance row selects it and closes the panel — + and "activating" is one path for both input modes, because the tree runs in + Selectable mode and the mouse click reaches it through TreeView's own content span. Typing filters the forest to the matching instances and their ancestors; Enter picks the first visible match, ArrowDown hands keyboard navigation to the tree and Escape closes without changing the selection. @@ -10,14 +12,13 @@ an outside click, the same pattern TreeView uses for its context menu. *@ @namespace ZB.MOM.WW.ScadaBridge.CentralUI.Components.Shared -@using ZB.MOM.WW.ScadaBridge.Commons.Entities.Instances
@* ip-field lifts the field above the open panel's backdrop — see the .razor.css note. *@
@@ -30,14 +31,25 @@ {
+ @* Selectable is what makes the tree's own Enter/Space activation reach us: it + routes BOTH a click on TreeView's content span and a key press on the + treeitem through SelectedKeyChanged, so mouse and keyboard share one path + (the row label below therefore carries no @onclick of its own — a second + handler there would fire twice on a click, since the click bubbles into the + content span). SelectedKey is deliberately left unbound: a combobox shows + its pick in the field, not as a row highlight, and binding it would also arm + TreeView's "selected key no longer in the tree" reset — which fires on every + render while the pick is filtered out, and this component would never clear + it. SelectedCssClass is emptied to match; with no SelectedKey nothing is ever + marked selected, so it is belt-and-braces. *@ + KeySelector="n => n.Key" IndentPx="16" + Selectable="true" SelectedCssClass="" SelectedKeyChanged="OnTreeKeyActivatedAsync"> @RowLabel(node) + data-test="@($"{DataTestPrefix}-row")" data-instance-id="@(node.Instance?.Id)">@RowLabel(node) @(_hasFilter ? "No instances match." : "No instances at this site.") diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstancePicker.razor.cs b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstancePicker.razor.cs index 198d9cff..f9303110 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstancePicker.razor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstancePicker.razor.cs @@ -27,6 +27,13 @@ namespace ZB.MOM.WW.ScadaBridge.CentralUI.Components.Shared; /// /// /// +/// Inside the tree, Enter/Space on the focused row does exactly what a click on +/// it does — picks an instance, toggles an area — because the tree runs in +/// Selectable mode and both gestures arrive at +/// through TreeView's single selection callback. +/// +/// +/// /// A parent that swaps / (a site change) /// closes the panel and re-derives the field text from the current selection, clearing /// it when the selected instance is not in the new list. A parent that changes @@ -86,6 +93,22 @@ public partial class InstancePicker [Parameter] public string Placeholder { get; set; } = "Search or browse instances…"; + /// + /// The combobox's accessible name, rendered as aria-label on the field. A page + /// that renders its own visible <label for> should point it at + /// ; the label wins for a screen reader either way, so this stays + /// as the fallback name rather than being suppressed. + /// + [Parameter] + public string AriaLabel { get; set; } = "Instance"; + + /// + /// DOM id for the text field, so a page can address it from a <label for>. + /// Unset means the per-instance generated id — two pickers on one page never collide. + /// + [Parameter] + public string? InputId { get; set; } + private readonly string _idSuffix = Guid.NewGuid().ToString("N")[..8]; private bool _open; @@ -126,7 +149,7 @@ public partial class InstancePicker private int _lastSelectedId; private bool _parametersSeen; - private string _inputId => $"ip-{_idSuffix}-input"; + private string _inputId => InputId is { Length: > 0 } id ? id : $"ip-{_idSuffix}-input"; private string _panelId => $"ip-{_idSuffix}-panel"; @@ -309,11 +332,34 @@ public partial class InstancePicker return null; } - private Task OnRowClickAsync(InstanceTreeNode node) + /// + /// The single activation path for a tree row, reached from TreeView's + /// SelectedKeyChanged — which a mouse click on the row and an + /// Enter/Space on the focused treeitem both run through, so the two input + /// modes can never diverge. An instance key picks that instance; an area key toggles the + /// branch (there is nothing to select on an area, and a keyboard user who cannot open a + /// branch cannot reach the instances inside it). + /// + /// + /// Keys are the ones mints: i:{instanceId} and + /// a:{areaId}. Anything else — including the null TreeView raises when it clears a + /// selection — is ignored rather than guessed at. + /// + /// + private Task OnTreeKeyActivatedAsync(object? key) { - if (node.Instance is { } instance) return SelectAsync(instance); + if (Disabled || key?.ToString() is not { Length: > 0 } keyStr) return Task.CompletedTask; + + if (keyStr.StartsWith("i:", StringComparison.Ordinal)) + { + return int.TryParse(keyStr.AsSpan(2), out var instanceId) + && Instances?.FirstOrDefault(i => i.Id == instanceId) is { } instance + ? SelectAsync(instance) + : Task.CompletedTask; + } + + if (keyStr.StartsWith("a:", StringComparison.Ordinal)) _tree?.ToggleExpanded(keyStr); - _tree?.ToggleExpanded(node.Key); return Task.CompletedTask; } diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Shared/InstancePickerTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Shared/InstancePickerTests.cs index 07dec031..d67a77df 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Shared/InstancePickerTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Shared/InstancePickerTests.cs @@ -65,6 +65,10 @@ public class InstancePickerTests : BunitContext private static string[] Rows(IRenderedComponent cut) => cut.FindAll("[data-test='t-row']").Select(r => r.TextContent.Trim()).ToArray(); + /// The nth treeitem in document order - i.e. the nth row the operator can see. + private static AngleSharp.Dom.IElement TreeItem(IRenderedComponent cut, int index) => + cut.FindAll("[role='treeitem']")[index]; + [Fact] public void Closed_ShowsNothingButTheField() { @@ -450,4 +454,106 @@ public class InstancePickerTests : BunitContext Assert.Single(cut.FindAll("[data-test='t-panel']")); Assert.Equal(new[] { "Zulu" }, Rows(cut)); } + + // -- Keyboard activation on the tree itself -------------------------------- + + [Fact] + public void KeyboardActivation_OnATreeRow_SelectsTheInstance() + { + int? picked = null; + var cut = RenderPicker(onChanged: id => picked = id); + + Open(cut); + Press(cut, "ArrowDown"); + + // Areas render before root instances, so Zulu is the SECOND treeitem - the first + // one is the Line1 area (see KeyboardActivation_OnAnAreaRow_TogglesIt). + TreeItem(cut, 1).KeyDown(new KeyboardEventArgs { Key = "Enter" }); + + Assert.Equal(8, picked); + Assert.Empty(cut.FindAll("[data-test='t-panel']")); + Assert.Equal("Zulu", InputValue(cut)); + } + + [Fact] + public void KeyboardActivation_OnAnAreaRow_TogglesIt() + { + int? picked = null; + var cut = RenderPicker(onChanged: id => picked = id); + + Open(cut); + Press(cut, "ArrowDown"); + + TreeItem(cut, 0).KeyDown(new KeyboardEventArgs { Key = "Enter" }); + + // An area activates as an expand, not as a pick: its children appear and the + // panel stays open with no callback raised. + Assert.Contains("Cell A", Rows(cut)); + Assert.Single(cut.FindAll("[data-test='t-panel']")); + Assert.Null(picked); + + // The same key on the same row closes it again - activation is a toggle. + TreeItem(cut, 0).KeyDown(new KeyboardEventArgs { Key = "Enter" }); + Assert.DoesNotContain("Cell A", Rows(cut)); + } + + [Fact] + public void KeyboardActivation_AndMouseClick_RaiseExactlyOneSelection() + { + // The label click and Enter both run through TreeView's single selection path, + // so neither input mode may double-fire the callback. + var picks = new List(); + var cut = RenderPicker(onChanged: picks.Add); + + Open(cut); + ClickRow(cut, "Zulu"); + Assert.Equal(new[] { 8 }, picks); + + Open(cut); + TreeItem(cut, 1).KeyDown(new KeyboardEventArgs { Key = "Enter" }); + Assert.Equal(new[] { 8, 8 }, picks); + } + + // -- Accessible name / labelling ------------------------------------------- + + [Fact] + public void Input_CarriesAnAccessibleName_ByDefaultAndFromTheParameter() + { + var cut = RenderPicker(); + Assert.Equal("Instance", cut.Find("[data-test='t-input']").GetAttribute("aria-label")); + + cut.Render(ps => ps.Add(p => p.AriaLabel, "Target instance")); + Assert.Equal("Target instance", cut.Find("[data-test='t-input']").GetAttribute("aria-label")); + } + + [Fact] + public void InputId_DefaultsToAGeneratedId_AndHonoursTheParameter() + { + // A page that wants its own