From 3c3a7cbbc18c88856c5c5730abbc7317d462e946 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 8 Sep 2026 09:39:46 -0400 Subject: [PATCH] =?UTF-8?q?fix(central-ui):=20InstancePicker=20=E2=80=94?= =?UTF-8?q?=20field=20above=20the=20backdrop;=20site=20change=20always=20c?= =?UTF-8?q?loses;=20reference=20contract=20documented;=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three Task 2 review findings: 1. The outside-click backdrop is a fixed full-viewport layer at z-index 1040, but the input-group was unpositioned — so once the panel was open a click on the field or the chevron hit the backdrop and closed the panel instead of letting the operator keep typing. The field now carries .ip-field (position:relative, z-index 1042), stacking backdrop < panel < field. Pinned in bUnit by the class (the z-index itself is scoped CSS, verified visually). 2. OnParametersSet closes the panel on ANY lists change, not only when the selection disappeared: an open panel is showing the previous site's tree, which is wrong even when the selected instance happens to exist in both. The reference-equality contract on Areas/Instances is now stated on both parameters — pass a stable reference, replace it only when the set changed. 3. Tests: the chevron closes an open panel; two pickers mint distinct panel ids (and matching aria-controls); an unrelated parameter change with the SAME Instances reference leaves a half-typed filter and the open panel alone. Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29 --- .../Components/Shared/InstancePicker.razor | 3 +- .../Components/Shared/InstancePicker.razor.cs | 20 ++++-- .../Shared/InstancePicker.razor.css | 14 +++- .../Shared/InstancePickerTests.cs | 65 +++++++++++++++++++ 4 files changed, 95 insertions(+), 7 deletions(-) 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 0e5144ca..15f9ac8d 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstancePicker.razor +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstancePicker.razor @@ -13,7 +13,8 @@ @using ZB.MOM.WW.ScadaBridge.Commons.Entities.Instances
-
+ @* ip-field lifts the field above the open panel's backdrop — see the .razor.css note. *@ +
public partial class InstancePicker { - /// The site's areas, flat and linked by ; null is treated as empty. + /// + /// The site's areas, flat and linked by ; null is treated + /// as empty. Compared by REFERENCE: a new list object means "the site changed" and closes + /// the panel, drops the filter and re-derives the field text. Hold a stable reference and + /// only hand the picker a new list when the underlying set actually changed — rebuilding + /// the list on every parent render would close the panel out from under the operator. + /// [Parameter] public IReadOnlyList? Areas { get; set; } - /// The site's instances; null is treated as empty. + /// + /// The site's instances; null is treated as empty. Compared by REFERENCE on the same + /// contract as — pass a stable reference and only replace it when the + /// underlying set changed. + /// [Parameter] public IReadOnlyList? Instances { get; set; } @@ -139,10 +149,12 @@ public partial class InstancePicker if (listsChanged) { - // A site change: drop any filter, rebuild, and close if the selection went away. + // A site change: drop any filter, rebuild, and close unconditionally — the panel + // is showing the OLD site's tree, so leaving it open would be showing a lie even + // when the selected instance happens to exist in the new list too. _filter = string.Empty; Rebuild(); - if (selected is null) _open = false; + _open = false; _text = selected is null ? string.Empty : LabelOf(selected); } else if (!_open) diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstancePicker.razor.css b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstancePicker.razor.css index cb7575e9..7a756125 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstancePicker.razor.css +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Components/Shared/InstancePicker.razor.css @@ -1,5 +1,15 @@ -/* Instance Picker combobox — the field is a plain input-group; only the dropdown - panel and its outside-click backdrop need styling. */ +/* Instance Picker combobox — the field, the dropdown panel and the outside-click + backdrop are stacked deliberately: backdrop (1040) < panel (1041) < field (1042). */ + +/* The backdrop is a full-viewport fixed layer, so an UNPOSITIONED input-group would be + painted underneath it and every click on the field or the chevron would be swallowed + by the backdrop (= panel closes, cannot keep typing). Giving the field its own + positioned layer above both keeps it clickable while the panel is open; the panel + sits at top:100% so being one level lower never hides it. */ +.ip-field { + position: relative; + z-index: 1042; +} /* Transparent full-viewport catcher: a click anywhere outside the panel closes it. Sits just under the panel and above page content (Bootstrap's dropdown z-index 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 29ce6ed0..765bf160 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Shared/InstancePickerTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Shared/InstancePickerTests.cs @@ -355,4 +355,69 @@ public class InstancePickerTests : BunitContext Assert.Equal(new[] { "Line1", "Zulu" }, Rows(cut)); Assert.Empty(cut.FindAll("[data-instance-id='7']")); } + + [Fact] + public void Toggle_WhileOpen_ClosesThePanel() + { + var cut = RenderPicker(); + + Open(cut); + Assert.Single(cut.FindAll("[data-test='t-panel']")); + + Open(cut); + + Assert.Empty(cut.FindAll("[data-test='t-panel']")); + Assert.Equal("false", cut.Find("[data-test='t-input']").GetAttribute("aria-expanded")); + } + + [Fact] + public void TheFieldSitsAboveTheBackdrop_SoItStaysClickableWhileOpen() + { + // The backdrop is a fixed full-viewport layer; the stylesheet lifts the field above + // it through this class. Without it a click on the input or the chevron would land on + // the backdrop instead and close the panel. Only the class can be pinned here — the + // z-index itself is scoped CSS, verified visually. + var cut = RenderPicker(); + Open(cut); + + var field = cut.Find("[data-test='t-picker'] .input-group"); + Assert.Contains("ip-field", field.ClassList); + Assert.Single(cut.FindAll("[data-test='t-backdrop']")); + } + + [Fact] + public void TwoPickers_GetDistinctPanelIds() + { + // The panel id (and the field's aria-controls pointing at it) is minted per component + // instance, so two pickers on one page cannot collide on a DOM id. + var first = RenderPicker(); + var second = RenderPicker(); + + Open(first); + Open(second); + + var firstPanel = first.Find("[data-test='t-panel']").Id; + var secondPanel = second.Find("[data-test='t-panel']").Id; + + Assert.False(string.IsNullOrEmpty(firstPanel)); + Assert.NotEqual(firstPanel, secondPanel); + Assert.Equal(firstPanel, first.Find("[data-test='t-input']").GetAttribute("aria-controls")); + Assert.Equal(secondPanel, second.Find("[data-test='t-input']").GetAttribute("aria-controls")); + } + + [Fact] + public void SameInstancesReference_LeavesTheTypedTextAndOpenPanelAlone() + { + // Complement of SiteChange_NewInstancesList_ClearsAStaleSelectionText: an unrelated + // parameter change is NOT a site change, so a half-typed filter must survive it. + var cut = RenderPicker(); + Open(cut); + Type(cut, "zu"); + + cut.Render(ps => ps.Add(p => p.Placeholder, "Pick an instance")); + + Assert.Equal("zu", InputValue(cut)); + Assert.Single(cut.FindAll("[data-test='t-panel']")); + Assert.Equal(new[] { "Zulu" }, Rows(cut)); + } }