fix(ui): clear page-scoped detail state on paging where no keyed detail exists
Gating the detail modal/drawer on a held id rather than on the row resolving (d14e0ee4) made it the surface's own job to clear that id when navigation invalidates page-scoped state. ParkedMessages and ConfigurationAuditLog did not, so paging away from an open row left the surface mounted on a notice it could never recover from — reachable only by paging back. The criterion is per-surface and comes down to whether the modal has content of its own: ParkedMessages, ConfigurationAuditLog — no keyed detail fetch; content resolves from the loaded page alone. An entry paged out of view can never resolve again, so these must clear on paging. They already cleared it on Search/OnSiteChanged for the same reason, and clear _selectedIds on paging for the same reason again; paging was simply missed. NotificationReport, SiteCallsReport — fetch detail by id, so the modal still shows real content after its row leaves the page. These deliberately do NOT clear on paging and are unchanged. Clearing on an explicit navigation action is user intent, not a resolve-driven unmount, so this cannot reopen the handler-disposal race thatd14e0ee4closed. PageScopedDetailStateTests covers all three paging entry points and records the criterion so the next reader can tell why two surfaces clear and two do not. Run against both clears reverted, all three fail; restored, all three pass. CentralUI.Tests 994/994, solution build 0/0. Also corrects two comments in ParkedMessages left stale byd14e0ee4— they still described the drawer as self-closing when a row stops resolving, which is the behaviour that change deliberately removed.
This commit is contained in:
+6
-1
@@ -343,7 +343,12 @@
|
||||
await FetchPage();
|
||||
}
|
||||
|
||||
private async Task OnPageChanged(int page) { _page = page; await FetchPage(); }
|
||||
// The state modal is page-scoped here: its content resolves from _entries alone
|
||||
// (this surface has no keyed detail fetch), so an entry paged out of view can
|
||||
// never resolve again and the modal would sit on a permanently empty notice.
|
||||
// Clearing on an explicit navigation action is intent, not a resolve-driven
|
||||
// unmount, so it does not reopen the handler-disposal race.
|
||||
private async Task OnPageChanged(int page) { _page = page; _modalEntryId = null; await FetchPage(); }
|
||||
|
||||
private async Task FetchPage()
|
||||
{
|
||||
|
||||
+19
-8
@@ -388,14 +388,20 @@
|
||||
// Drawer — holds the row's ID, never the row object. FetchPage() replaces
|
||||
// _messages wholesale after every Retry/Discard and on every page change, so a
|
||||
// captured ParkedMessageEntry would outlive its row and keep rendering values
|
||||
// (attempt count, last-attempt time, error) that no longer exist server-side;
|
||||
// PrevPage/NextPage do not clear the drawer, so it could even sit open over a
|
||||
// page that never contained it. Mirrors ExecutionTreePage's _modalExecutionId.
|
||||
// (attempt count, last-attempt time, error) that no longer exist server-side.
|
||||
// Mirrors ExecutionTreePage's _modalExecutionId.
|
||||
//
|
||||
// ONLY user intent writes this field: OpenDrawer/CloseDrawer, plus the four
|
||||
// navigation actions that invalidate page-scoped state (Search, OnSiteChanged,
|
||||
// PrevPage, NextPage — the same places that clear _selectedIds). FetchPage()
|
||||
// must never touch it: a data refresh that could clear the id would unmount the
|
||||
// drawer mid-render and dispose its event handlers, which is the disposal race
|
||||
// documented on the drawer markup above.
|
||||
private string? _drawerMessageId;
|
||||
|
||||
// Re-resolves the open drawer's row from the CURRENT page on every render.
|
||||
// A row that has been retried, discarded or paged away resolves to null, which
|
||||
// renders the drawer away — the drawer self-closes rather than going stale.
|
||||
// Re-resolves the open drawer's row from the CURRENT page on every render, so
|
||||
// it can never render a stale row. This drives the drawer's CONTENT only — an
|
||||
// unresolvable row degrades to a notice; it does not unmount the drawer.
|
||||
private ParkedMessageEntry? DrawerMessage =>
|
||||
_drawerMessageId is null
|
||||
? null
|
||||
@@ -439,8 +445,13 @@
|
||||
await FetchPage();
|
||||
}
|
||||
|
||||
private async Task PrevPage() { _pageNumber--; _selectedIds.Clear(); await FetchPage(); }
|
||||
private async Task NextPage() { _pageNumber++; _selectedIds.Clear(); await FetchPage(); }
|
||||
// The drawer is page-scoped state, exactly like _selectedIds: paging away from
|
||||
// the open message invalidates it, so both are cleared here rather than leaving
|
||||
// the drawer mounted over a page that never contained its row. Clearing on an
|
||||
// explicit navigation action is intent, not a resolve-driven unmount, so it does
|
||||
// not reopen the disposal race.
|
||||
private async Task PrevPage() { _pageNumber--; _selectedIds.Clear(); _drawerMessageId = null; await FetchPage(); }
|
||||
private async Task NextPage() { _pageNumber++; _selectedIds.Clear(); _drawerMessageId = null; await FetchPage(); }
|
||||
|
||||
private async Task FetchPage()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user