Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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: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:alertattribute rendersrole="alertdialog"with the body as the accessible description.close(returnValue)mirrors native<dialog>:ol-closeandol-after-closenow carrydetail.returnValue(empty for Escape, backdrop, or the close button). Sinceol-closeis cancelable, a form dialog can validate and stay open.label-closemakes the close button's accessible name translatable; it was hardcoded to "Close dialog".alert-dialog.jscreates the<ol-dialog>element rather than importing the component, so legacy page scripts can import it without bundling a second copy (ol-components.jsregisters it site-wide). Message strings are always inserted as text; markup comes from a<template>in the page template, the same ruleshowToast()follows. Labels default to English like our other Lit components, so callers must pass translated strings.Moving callers from
confirm()toolConfirm()means going async: form-submit guards will need topreventDefault()up front and submit after the promise resolves.Testing
npm run test:js:browser: 12 new tests intests/browser/alertDialog.browser.test.jscovering each dismissal path, role and accessible name, initial focus, translated labels, strings never parsed as HTML, focus restoration,olAlert, stacked dialogs, andreturnValue. The stacked-dialog test fails without theOlDialogfix.npm run test:js: existing suite passes./developers/design#dialog: open each Confirmation demo, check that confirm showstrue, Escape/Cancel showfalse, and the template demo renders bold text. In the Form dialog demo, Save with an empty note keeps the dialog open; Cancel reportscancel.Screenshot
Stakeholders