Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider replacing usages of Window.confirm() with Modal #16583

Closed
noisysocks opened this issue Jul 15, 2019 · 5 comments · Fixed by #60385
Closed

Consider replacing usages of Window.confirm() with Modal #16583

noisysocks opened this issue Jul 15, 2019 · 5 comments · Fixed by #60385
Assignees
Labels
General Interface Parts of the UI which don't fall neatly under other labels. Needs Accessibility Feedback Need input from accessibility [Type] Enhancement A suggestion for improvement.

Comments

@noisysocks
Copy link
Member

We're currently using Window.confirm() in four places. This isn't an ideal experience as we're unable to customise the OK button to have a more descriptive label.

Moving these instances to use a Modal component might provide a better experience. Here's an example of a Modal being used this way, from @kjellr's mockup in #16315 (comment):

Screen Shot 2019-07-15 at 16 03 13


1. Switching a published post to draft

Current:

Screen Shot 2019-07-15 at 15 57 17

Proposed:

Switch to Draft

Are you sure you want to unpublish this post?

[ Cancel ] [ Unpublish ]

2. Switching a published post to private

Current:

Screen Shot 2019-07-15 at 15 57 42

Proposed:

Privately Publish

Are you sure you want to privately publish this post now?

[ Cancel ] [ Privately Publish ]

3. Deleting a reusable block

Current:

Screen Shot 2019-07-15 at 15 58 12

Proposed:

Delete Reusable Block

Are you sure you want to delete this Reusable Block? 

It will be permanently removed from all posts and pages that use it.

[ Cancel ] [ Delete ]

4. Pressing Reset the template when template doesn't match post content

Current:

Screen Shot 2019-07-15 at 15 59 21

Proposed:

Reset Template

Resetting the template may result in loss of content, do you want to continue?

[ Cancel ] [ Reset Template ]
@noisysocks noisysocks added [Type] Enhancement A suggestion for improvement. General Interface Parts of the UI which don't fall neatly under other labels. Needs Design Needs design efforts. labels Jul 15, 2019
@Soean Soean added the Needs Accessibility Feedback Need input from accessibility label Jul 15, 2019
@kjellr
Copy link
Contributor

kjellr commented Jul 15, 2019

Do we currently use modals as a stand-in for standard alerts elsewhere? I do find modals like the one pictured to be a little less alarming than the standard browser one, but I'm definitely be curious about any a11y considerations for this.

@aduth
Copy link
Member

aduth commented Jul 15, 2019

This is technically impossible to implement, at least for the confirmations shown on beforeunload (i.e. the unsaved changes prompt when reloading or navigating away). The browser does not give you an opportunity to prevent navigation except by the standard prompts.

@noisysocks
Copy link
Member Author

@aduth: None of the confirm()s shown above are triggered when reloading or navigating away 🙂

@aduth
Copy link
Member

aduth commented Jul 16, 2019

@aduth: None of the confirm()s shown above are triggered when reloading or navigating away 🙂

You're totally right! I skimmed the text of the last screenshot and the "result in content loss" had me jump the gun on assuming that's what we were discussing.

For the ones which don't involve navigation, it's totally possible to re-style. I guess it's worth clarifying at least that it cannot be applied for the prompt which asks the user to confirm unsaved changes before navigating away or refreshing a "dirty" post.

@t-hamano
Copy link
Contributor

t-hamano commented Apr 2, 2024

I searched the entire project for this issue with the keyword window.alert|window.confirm|window.prompt. Currently, there appears to be only one implementation left, excluding Storybook.

window.confirm(
__(
'Resetting the template may result in loss of content, do you want to continue?'
)
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
General Interface Parts of the UI which don't fall neatly under other labels. Needs Accessibility Feedback Need input from accessibility [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants