diff --git a/frontend/src/components/confirm-dialog/confirm-dialog.ts b/frontend/src/components/confirm-dialog/confirm-dialog.ts index 220f5c8..6c138d5 100644 --- a/frontend/src/components/confirm-dialog/confirm-dialog.ts +++ b/frontend/src/components/confirm-dialog/confirm-dialog.ts @@ -81,23 +81,55 @@ export class ConfirmDialog extends LitElement { `, ]; + /** + * Which question is on screen. + * + * This is a singleton reused for every confirmation in the app, + * and `wa-dialog` reports its close *asynchronously* — `open = + * false` starts an animation and `wa-hide` arrives after it. So a + * hide belonging to a question that has already been answered can + * land after the *next* question has opened, and cancel it: the + * user is asked something, the dialog vanishes on its own, and the + * call site is told they said no. + * + * The counter is what tells one question from the next. Every + * close bumps it, and the `wa-hide` handler carries the id its + * template was rendered with. + */ + private askSeq = 0; + /** Ask. Resolves true if the user went ahead. */ ask(request: ConfirmRequest): Promise { this.close(false); + + const id = ++this.askSeq; + this.request = request; return new Promise((resolve) => { this.settle = resolve; void this.updateComplete.then(() => { - if (this.dialog) this.dialog.open = true; + // A third question could have arrived while this one + // was waiting for its own render. + if (this.askSeq === id && this.dialog) this.dialog.open = true; }); }); } - private close(ok: boolean): void { + /** + * Settle the current question, if `id` still names it. + * + * The button handlers pass nothing and always mean the question on + * screen; only `wa-hide` carries an id, because only `wa-hide` can + * arrive late. + */ + private close(ok: boolean, id = this.askSeq): void { + if (id !== this.askSeq) return; + const settle = this.settle; this.settle = null; + this.askSeq++; if (this.dialog) this.dialog.open = false; this.request = null; @@ -118,11 +150,15 @@ export class ConfirmDialog extends LitElement { if (!request) return nothing; + // Captured at render time, so the handler answers the question + // it was drawn for and not whichever one is up when it fires. + const id = this.askSeq; + return html` this.close(false)} + @wa-hide=${() => this.close(false, id)} >

${request.message}

${request.impact diff --git a/frontend/test/components/confirm-dialog.test.ts b/frontend/test/components/confirm-dialog.test.ts index ac3683c..9ab447d 100644 --- a/frontend/test/components/confirm-dialog.test.ts +++ b/frontend/test/components/confirm-dialog.test.ts @@ -80,3 +80,42 @@ describe('confirmAction', () => { await expect(Promise.all([first, second])).resolves.toEqual([false, true]); }); }); + +/** + * A late `wa-hide` must not answer the next question. + * + * This is one singleton for every confirmation in the app, and + * `wa-dialog` reports its close *asynchronously* — `open = false` + * starts an animation and `wa-hide` arrives after it. So a hide + * belonging to a question already answered can land after the next one + * has opened: the user is asked something, the dialog vanishes on its + * own, and the call site is told they said no. + * + * Found by writing two `confirmAction()` tests in one file — the + * second could not be accepted at all, because the first one's hide + * had cancelled it before the click landed. In the app it needs two + * confirmations close together, which "apply these tags" now makes + * reachable. + */ +describe('two questions in a row', () => { + it('does not let the first one answer the second', async () => { + const first = confirmAction({ title: 'First?', message: 'One.' }); + + await press('confirm-cancel'); + await expect(first).resolves.toBe(false); + + const second = confirmAction({ title: 'Second?', message: 'Two.' }); + + // Whatever the first dialog's hide animation is still doing, the + // second question is on screen and unanswered. + await new Promise((r) => setTimeout(r, 0)); + + // The title is a `label` on `wa-dialog` and lands in *its* shadow + // root; the message is the part this component renders. + expect(host().shadowRoot?.textContent ?? '').toContain('Two.'); + + await press('confirm-accept'); + + await expect(second).resolves.toBe(true); + }); +});