Skip to content

Sales: both dialogs can be open at once, so the untagged dialog-error slot misattributes #480

Description

@mforce

#478 split SalesPage's failures into a page slot and a dialog slot, and left the dialog slot untagged, on this reasoning (written into the source):

One slot for both dialogs, deliberately: they are modal, so at most one is ever open.

Nothing enforces that. creatingOrder and paying are independent useStates, both triggers stay mounted and enabled while the other dialog is open, and the only thing stopping a second click is the backdrop's CSS occlusion — Dialog never marks the rest of the page inert/aria-hidden, so a screen reader's virtual cursor can activate the other trigger, as can a second click racing the paint.

Reproduced in a test: open a Confirmed order with an outstanding balance, click Record payment, then click New ordergetAllByRole("dialog") returns 2. With the untagged slot, a failed createOrder then renders its message inside the payment form as well — one form accusing the user about a submission made in another. That is the misattribution class #474 exists to remove, reachable through the one path the design assumed away.

Found by an internal reviewer after #478 merged.

Fix

dialogError carries its scope again; each dialog renders only its own; clearDialogError(scope) clears one form's message without touching the other's (on open, on dismissal, and at the start of each attempt). The guarantee is attribution, not two simultaneous messages — one slot holds one message, so a second failure replaces the first.

Also from the same review: the test named "keeps someone else's error when a dialog is dismissed" never asserted the dialog closed, so it would pass with Cancel wired to nothing. The regression was caught elsewhere, but the name promised more than the assertions checked.

Note for #479

The sibling screens this is being rolled out to have more than two dialogs each (Inventory 4, Users 4, Flocks 3, Products 3), so the same assumption is even less safe there. Whatever shape #479 lands on should be tagged, not positional.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions