From 11d1ae1d17462ea5ed62d120d0156615c4f29f65 Mon Sep 17 00:00:00 2001 From: Loretta Date: Fri, 17 Jul 2026 15:23:24 +0200 Subject: [PATCH] Improve filter panel restore logic and doc clarity Major overhaul of MgGridFilterPanelTagBox restore/healing logic: - Level-triggered healing prevents lost filter criteria after async loads. - Healing now only triggers when needed; delayed assert ensures stability. - ValuesChanged event guarded to avoid accidental filter clearing. - Diagnostics dormant but retained for future debugging. - Comments and summaries updated for clarity. Docs (MGGRID_PARAMETERS.md, 0004-signalr-datasource-server-side-filtering.md) updated to explain new logic, rationale, and future plans. Minor refactor in GridShippingDocument.razor for reload clarity. No functional change in ShippingsAdmin.razor (comment only). --- .../Grids/MgGridFilterPanelTagBox.cs | 141 ++++++++++++------ .../docs/MGGRID/MGGRID_PARAMETERS.md | 9 ++ 2 files changed, 108 insertions(+), 42 deletions(-) diff --git a/AyCode.Blazor.Components/Components/Grids/MgGridFilterPanelTagBox.cs b/AyCode.Blazor.Components/Components/Grids/MgGridFilterPanelTagBox.cs index 4d9eebe..c578ee0 100644 --- a/AyCode.Blazor.Components/Components/Grids/MgGridFilterPanelTagBox.cs +++ b/AyCode.Blazor.Components/Components/Grids/MgGridFilterPanelTagBox.cs @@ -46,14 +46,15 @@ public class MgGridFilterPanelTagBox : MgTagBox, I InheritSizeModeFromGrid(); TrackEditorSnapshot(); - Diag("OnParametersSet"); + //Diag("OnParametersSet"); SyncSelectionToCriteria(); } - // ======================= TEMP DIAGNOSTIC — REMOVE after the restore-race investigation ======================= - // Purpose: prove/disprove that DevExpress raises ValuesChanged with an EMPTY list around Reload() (dropping - // not-yet-resolvable tags), which OnSelectionChanged would misread as a user edit and clear the criteria. - // Blazor WASM: Console.WriteLine lands in the browser console. Filter the console by "[MgFP". + // ---- Dormant diagnostics ------------------------------------------------------------------------------- + // Console tracing for the restore/heal flow (Blazor WASM: Console.WriteLine lands in the browser console; + // filter by "[MgFP"). The call sites are commented out — uncomment them when tracing is needed again + // (e.g. during the v2 envelope/pull-model work, ADR 0004). This tooling proved the ValuesChanged-drop and + // the fill-timing root causes; keep it. private static string Fmt(IEnumerable? values) => values is null ? "null" : $"[{string.Join(",", values)}]"; @@ -77,7 +78,6 @@ public class MgGridFilterPanelTagBox : MgTagBox, I Console.WriteLine($"[MgFP:{FieldName}] {message} | dataCount={dataCount} snapWasEmpty={_snapshotTakenWhileDataEmpty} " + $"values={Fmt(Values)} criteria={criteria}"); } - // ===================== /TEMP DIAGNOSTIC ===================================================================== // ---- Editor-snapshot state ----------------------------------------------------------------------------- // A restore has TWO inputs that arrive in arbitrary order: the criteria (async layout restore via JS interop) @@ -91,10 +91,14 @@ public class MgGridFilterPanelTagBox : MgTagBox, I private IEnumerable? _snapshotData; private bool _snapshotTakenWhileDataEmpty; - private bool _reassertAfterReloadPending; private bool IsDataEmpty => Data is null || !Data.Any(); + // The measured editor-internal reload (a JS hop) settles in ~300 ms; the delay is a generous cover, not a + // contract — the v2 envelope/pull model removes the need for it entirely (see ADR 0004 → planned v2). + private const int HealAssertDelayMs = 750; + private bool _healAssertScheduled; + /// /// Records whether the editor's current item snapshot was taken against an EMPTY Data. Runs on every /// parameter pass REGARDLESS of criteria presence — arming must not depend on the criteria arriving first; @@ -119,25 +123,64 @@ public class MgGridFilterPanelTagBox : MgTagBox, I /// /// Forces the editor to re-read its Data source when its snapshot is known-stale (taken while empty) - /// and Data has since been filled in place. Returns true when a reload was issued, and queues ONE - /// deferred re-assert: DevExpress documents no settle guarantee for Reload() - /// (DxListEditorBase.Reload is a bare void), so a same-pass Values assignment could still - /// race the item list. + /// and Data has since been filled in place. SAFE ORDER (console-proven, 2026-07-17): the reload runs + /// against an EMPTY editor — no value sits in it (see the data-empty gates in + /// and ) — so the editor's async revalidation has nothing to drop and raises no + /// spurious ValuesChanged. The selection is then asserted ONCE, delayed, into the settled editor + /// (); its echo rewrites the same criteria, which is harmless. /// - private bool HealEditorSnapshotIfStale() + private void HealEditorSnapshotIfStale() { - if (!_snapshotTakenWhileDataEmpty || IsDataEmpty) return false; + if (!_snapshotTakenWhileDataEmpty || IsDataEmpty) return; _snapshotTakenWhileDataEmpty = false; + //Diag(">>> HEAL: calling DevExpress Reload() on an EMPTY editor"); Reload(); - _reassertAfterReloadPending = true; + //Diag("<<< HEAL: Reload() returned"); - return true; + ScheduleHealAssert(); } /// - /// Brings the displayed selection in line with our field's active criteria: heals a stale editor snapshot - /// first, then asserts the values when the snapshot was just healed or the selection drifted. + /// Schedules the ONE selection assert that follows a heal, delayed until the editor's internal reload has + /// settled. Re-reads the criteria AT FIRE TIME — if it is gone by then (user cleared it, panel hidden), no-op. + /// + private void ScheduleHealAssert() + { + if (_healAssertScheduled) return; + _healAssertScheduled = true; + + _ = InvokeAsync(async () => + { + try + { + await Task.Delay(HealAssertDelayMs); + _healAssertScheduled = false; + + if (Grid is null || string.IsNullOrWhiteSpace(FieldName) || IsDataEmpty) return; + if (Grid.GetFilterPanelCriteria(FieldName) is not { } criteria) return; + if (!TryExtractInValues(criteria, out var values)) return; + + //Diag($"HEAL-assert (delayed {HealAssertDelayMs} ms) -> {Fmt(values)}"); + SetValuesCore(values); + StateHasChanged(); + } + catch + { + // Best-effort: a failed delayed assert leaves the box empty — the criteria is untouched and the + // Clear button still signals it; a later reconcile/parameter pass re-asserts. + //Diag("HEAL-assert FAILED"); + _healAssertScheduled = false; + } + }); + } + + /// + /// Brings the displayed selection in line with our field's active criteria. GATE: while Data is empty + /// NOTHING is asserted — an unresolvable value sitting in the editor is exactly the ammunition the editor's + /// async revalidation needs to raise a spurious empty ValuesChanged (console-proven to destroy the + /// criteria). The criteria itself stays untouched meanwhile (rows filter, the Clear button signals it); the + /// assert happens once the data is there — immediately when no heal was needed, delayed after a heal. /// private void SyncSelectionToCriteria() { @@ -145,10 +188,14 @@ public class MgGridFilterPanelTagBox : MgTagBox, I if (Grid.GetFilterPanelCriteria(FieldName) is not { } criteria) return; if (!TryExtractInValues(criteria, out var criteriaValues)) return; - var healed = HealEditorSnapshotIfStale(); - var drifted = Values is null || !Values.SequenceEqual(criteriaValues); + if (IsDataEmpty) return; // no value may enter an empty editor - if (healed || drifted) SetValuesCore(criteriaValues); + HealEditorSnapshotIfStale(); // reloads the still-empty editor + schedules the delayed assert + + if (_healAssertScheduled) return; // the delayed assert owns the write — don't race the editor's reload + + var drifted = Values is null || !Values.SequenceEqual(criteriaValues); + if (drifted) SetValuesCore(criteriaValues); } /// @@ -181,27 +228,17 @@ public class MgGridFilterPanelTagBox : MgTagBox, I // Register once, after the first render (grid API calls are unsafe mid-render). The grid holds the // registration weakly and reconciles this component immediately — restoring a persisted panel filter // into the UI, or clearing it while the panel is hidden. See IMgGridFilterPanelComponent / ADR 0004. - if (firstRender) Grid?.RegisterFilterPanelComponent(this); - - // Deferred re-assert after a Reload(): the reload-triggered render has completed by now, so the editor's - // item list is settled even though Reload() itself gives no such guarantee. Flag-gated and idempotent — - // the re-assert's own render finds the flag cleared, so this cannot loop. - if (_reassertAfterReloadPending) + if (firstRender) { - _reassertAfterReloadPending = false; - - if (Grid is not null && !string.IsNullOrWhiteSpace(FieldName) - && Grid.GetFilterPanelCriteria(FieldName) is { } criteria - && TryExtractInValues(criteria, out var values)) - { - SetValuesCore(values); // fresh instance => the editor re-resolves its tags - StateHasChanged(); - } + //Diag("OnAfterRender(firstRender) -> RegisterFilterPanelComponent"); + Grid?.RegisterFilterPanelComponent(this); } } private void OnSelectionChanged(IEnumerable? values) { + //Diag($"*** ValuesChanged CALLBACK — incoming={Fmt(values)}"); + SetValuesCore(values); ApplyFilterPanelCriteria(values); } @@ -215,6 +252,21 @@ public class MgGridFilterPanelTagBox : MgTagBox, I // Empty selection => null => clears this field's criteria. Otherwise IN(FieldName, selected...). CriteriaOperator? criteria = selected.Length > 0 ? new InOperator(FieldName, selected) : null; + // OWNERSHIP: an empty selection may only clear OUR criteria. The per-field slot is shared with the + // grid's filter row — a not-owned criteria (e.g. Contains("EUR") typed there) must survive any empty + // ValuesChanged, be it a genuine user clear (nothing of ours is active anyway) or the echo of a + // programmatic clear. + if (criteria is null + && Grid.GetFilterPanelCriteria(FieldName) is { } current && !OwnsCriteria(current)) + { + //Diag("WRITE skipped — empty selection over a NOT-OWNED criteria (filter-row value protected)"); + return; + } + + //Diag(criteria is null + // ? "!!! WRITE criteria -> NULL (CLEARING the field's criteria)" + // : $"WRITE criteria -> {criteria}"); + Grid.SetFilterPanelCriteria(FieldName, criteria); } @@ -229,21 +281,26 @@ public class MgGridFilterPanelTagBox : MgTagBox, I // NOTE: CriteriaOperator overloads == / != (they BUILD criteria) — null checks must be pattern-based. var criteria = Grid.GetFilterPanelCriteria(FieldName); + //Diag($"SyncFromGrid (from grid reconcile) — criteria={(criteria is null ? "null" : criteria.ToString())}"); + if (criteria is not null && TryExtractInValues(criteria, out var values)) { - // The grid may be telling us about a criteria whose lookup data has only just arrived — e.g. the - // consumer calling RefreshFilterPanel() right after an in-place fill. Without healing the editor's - // (empty) snapshot first, the assert below could never resolve into tags: this is what made - // RefreshFilterPanel fall short of its documented contract. + // Same gate as SyncSelectionToCriteria: no value may enter an empty editor — the criteria stays + // untouched and the assert happens once the lookup data exists (this is also what made the consumer's + // RefreshFilterPanel call after an in-place fill actually work: the heal below reloads the editor). + if (IsDataEmpty) return; + HealEditorSnapshotIfStale(); - SetValuesCore(values); + + if (!_healAssertScheduled) SetValuesCore(values); } else { // Null criteria, or a criteria that is not ours: the per-field slot is SHARED with the grid's filter // row (e.g. Contains("EUR") typed into the column filter) — leave it untouched (it is visible in the - // grid's own filter UI, so it is not invisible filtering) and just show no selection. - SetValuesCore(null); + // grid's own filter UI, so it is not invisible filtering) and just show no selection. Only write when + // there is something to clear: a null-over-null write can still echo a spurious ValuesChanged. + if (Values is not null && Values.Any()) SetValuesCore(null); } // May be called from the grid (outside this component's render path) — request our own re-render. diff --git a/AyCode.Blazor.Components/docs/MGGRID/MGGRID_PARAMETERS.md b/AyCode.Blazor.Components/docs/MGGRID/MGGRID_PARAMETERS.md index 7d79da3..77136b2 100644 --- a/AyCode.Blazor.Components/docs/MGGRID/MGGRID_PARAMETERS.md +++ b/AyCode.Blazor.Components/docs/MGGRID/MGGRID_PARAMETERS.md @@ -92,6 +92,15 @@ The panel content is always rendered (CSS-hidden when toggled off) so the compon > **Why level-triggered:** two earlier **edge-triggered** designs (detect the `Data` reference change; then reference **or** item-count change) each looked correct and each shipped a bug: an edge detector needs an *arming* pass that records the pre-fill state, so the **last-filled** lookup on a page — whose fill lands after its final parameter pass — was systematically stranded (rows filtered, `Clear` enabled, chips empty). Do not reintroduce an edge gate here. The v2 layout envelope removes the need for this heal entirely (see ADR 0004 → planned v2). +**`ValuesChanged` is not only the user — safe-order restore.** The editor also raises `ValuesChanged` for the **echo** of a programmatic `Values` write and for an **internal revalidation drop** (console-proven: `ValuesChanged([])` after a heal, via a JS-interop hop whose latency is unbounded in render ticks — a windowed callback guard shipped and was refuted the same day). Unhandled, the drop was misread as a user edit and **cleared the restored criteria**, which the AutoSave then persisted (saved filter permanently lost). The fix removes the drop's *precondition* instead of filtering callbacks: + +- **no value ever enters an empty editor** — while the lookup `Data` is empty, nothing is asserted; the criteria stays untouched (rows filter, the Clear button signals it); +- the heal `Reload()`s the **empty** editor — nothing to drop ⇒ no spurious callback at all; +- **one delayed assert** (~750 ms > the measured ~300 ms hop; re-reads the criteria at fire time) selects into the settled editor — its echo rewrites the same criteria (harmless); +- **ownership-guarded clearing**: an empty selection only clears criteria the component OWNS — a filter-row value can never be wiped by an empty callback. + +Every ambiguous path resolves toward **clearing** (never an invisible filter); the v2 envelope removes the heal + delay entirely (ADR 0004 → planned v2). + Implementation note: imperative parameter writes inside DevExpress editors (`Values`, `SizeMode`) must be wrapped in `BeginUpdate()`/`EndUpdate()` (the parameter tracker throws otherwise) — `MgGridFilterPanelTagBox` handles this internally. Design + the server-side forward path (`FilterMode.Server`): AyCode.Core repo `docs/adr/0004-signalr-datasource-server-side-filtering.md`.