Skip to content

refactor(ui): replace every native confirmation with the in-app dialog - #336

Merged
HugoFara merged 1 commit into
developfrom
refactor/in-app-confirm-dialogs
Nov 4, 2026
Merged

HugoFara merged 1 commit into
developfrom
refactor/in-app-confirm-dialogs

Conversation

@HugoFara

@HugoFara HugoFara commented Nov 4, 2026

Copy link
Copy Markdown
Owner

Follows #335. Moves the remaining 35 confirm() calls to the in-app dialog, so a native confirmation now appears only as the fallback inside confirm_dialog.ts. The browser's leave-page warning is unchanged.

What changed

  • confirmAction() and confirmDeletion() replace confirm() across texts, terms, feeds, tags, books, dictionaries, the admin pages and offline data.
  • Prompts that were English in every locale are now translated, in 14 new common.dialog.* keys across 11 locales. Bulk action prompts use the action's own label as the heading.
  • The dialog's heading is now optional, because most prompts are a single question.

The parts worth reviewing

The answer now arrives later, and that is where this could go wrong.

  • Forms. A handler can no longer wait and then cancel, because the browser has already submitted. confirmBeforeSubmit() cancels first, then resubmits with requestSubmit(submitter) once confirmed. The marker is cleared after the replay, so a submission refused by validation asks again.
  • Clicks with data-confirm. These cancel and replay the same way. My first version cleared the marker in the first listener, which looped forever when the handler was registered twice. A test run hung on it, and it is fixed.
  • Capture phase. data-confirm-submit now asks before the form's own handlers. The backup page's @submit="restoring = true" ran first, so cancelling a restore left the page stuck on "restoring". That bug existed before this change.
  • Promises in conditions. !confirmDelete() is always false, and tsc only flagged one of the two cases, so bulk-deleting texts would have skipped the question. Both are fixed, and a test now rejects that shape.

Tests

  • tests/frontend/helpers/confirm_answers.ts lets tests click the real dialog rather than mock it. trackSubmissions() counts the submissions that go ahead.
  • confirm_dialog.test.ts covers holding a submission until the answer, never submitting when declined, keeping the pressed button, and asking again. jsdom has no submitter and does not validate in requestSubmit, so those two cases use a stubbed requestSubmit.
  • Two source scans fail on any new native confirm() and on an unawaited confirmation in a condition.
  • 13-books and 14-dictionaries used to rely on Cypress auto-accepting. They now click the dialog, and the dictionary spec also checks that Cancel leaves the row.

I also tried the Restore form on the backup page in Chrome: the dialog appeared, and Cancel left restoring false. I did not confirm a restore or empty the database, and that path is covered only by the unit tests.

Vitest, ESLint and TypeScript pass. Cypress passes except 24-anki-live.cy.ts, which needs npm run anki:up and was not run. The new translations have not been reviewed by native speakers.

prompt() and alert() are not part of this change.

The remaining 35 confirm() calls move to confirmAction() and
confirmDeletion(), and prompts that were English in every locale are
translated.

The answer now arrives later, which changes three things:

- A handler can no longer wait and then cancel a form submission, so
  confirmBeforeSubmit() cancels first and resubmits with the same button
  once confirmed. Clicks with data-confirm replay the same way.
- data-confirm-submit asks in the capture phase, before the form's own
  handlers. The backup page set `restoring` before the question, so
  cancelling a restore left the page stuck.
- `!confirmDelete()` on a promise is always false and tsc does not flag it,
  which would have deleted without asking. A test now rejects that shape
  and any new native confirm().

The tests answer the real dialog rather than a stub.
@HugoFara
HugoFara merged commit 1fcaaa4 into develop Nov 4, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant