Skip to content

Add olConfirm() for consistent confirmation dialogs - #13655

Draft
lokesh wants to merge 2 commits into
internetarchive:masterfrom
lokesh:feat/ol-confirm-dialog
Draft

lokesh wants to merge 2 commits into
internetarchive:masterfrom
lokesh:feat/ol-confirm-dialog

Conversation

@lokesh

@lokesh lokesh commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Closes #

feature

Open Library asks readers to confirm things in about a dozen places (deleting a note, returning a loan, removing a book from a list, merging authors), and each one does it differently. Some use the browser's native confirm(), which can't be styled or translated beyond its message and looks out of place on every platform. Others use jQuery UI dialogs, which pull in draggable, resizable, and button widgets just to show two buttons. There was no shared way to ask a yes/no question with our own components.

The idea is to keep <ol-dialog> as the only dialog component and add a small promise-based helper on top of it, so a confirmation is one awaited call instead of fifteen lines of open/close wiring copied into every caller:

if (await olConfirm({ title, message, confirmLabel, cancelLabel, destructive: true })) {
    await deleteList(listKey);
}

olConfirm() resolves true only when the confirm button is pressed; Escape, the backdrop, and the close button all count as cancel. olAlert() covers the acknowledge-only case. Destructive confirmations get a red button and initial focus on Cancel, so a stray Enter can't delete anything. This PR adds the helper and documents it; migrating the existing call sites is a follow-up.

Technical

Pattern for reviewers to weigh in on: the helper is only for yes/no questions and acknowledgements. As soon as a dialog collects input, compose <ol-dialog> directly. The design system docs now say this explicitly, and the helper deliberately has no "custom body" option so it doesn't grow back into a general dialog.

Supporting changes to <ol-dialog>, all backwards compatible:

  • An alert attribute renders role="alertdialog" with the body as the accessible description.
  • close(returnValue) mirrors native <dialog>: ol-close and ol-after-close now carry detail.returnValue (empty for Escape, backdrop, or the close button). Since ol-close is cancelable, a form dialog can validate and stay open.
  • label-close makes the close button's accessible name translatable; it was hardcoded to "Close dialog".
  • Fix: when one dialog opens another (for example a delete confirmation inside the notes modal), both dialogs' Tab traps ran. The inner one only won because its listener happened to register second, and focus passed through the parent on every Tab. A dialog now ignores Tab while focus is inside a different open dialog.

alert-dialog.js creates the <ol-dialog> element rather than importing the component, so legacy page scripts can import it without bundling a second copy (ol-components.js registers it site-wide). Message strings are always inserted as text; markup comes from a <template> in the page template, the same rule showToast() follows. Labels default to English like our other Lit components, so callers must pass translated strings.

Moving callers from confirm() to olConfirm() means going async: form-submit guards will need to preventDefault() up front and submit after the promise resolves.

Testing

  • npm run test:js:browser: 12 new tests in tests/browser/alertDialog.browser.test.js covering each dismissal path, role and accessible name, initial focus, translated labels, strings never parsed as HTML, focus restoration, olAlert, stacked dialogs, and returnValue. The stacked-dialog test fails without the OlDialog fix.
  • npm run test:js: existing suite passes.
  • Manually on /developers/design#dialog: open each Confirmation demo, check that confirm shows true, Escape/Cancel show false, and the template demo renders bold text. In the Form dialog demo, Save with an empty note keeps the dialog open; Cancel reports cancel.

Screenshot

Stakeholders

Promise-based replacements for window.confirm()/alert() and jQuery UI
confirmation dialogs, built on <ol-dialog>. ol-dialog gains an alert
role, close(returnValue), a translatable close label, and no longer
steals Tab from a dialog stacked above it.
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