Theme Delete Button: (Prevent this page from creating additional dialogs)

[emiluzelac]

Hey all,

Not sure if this is something you guys are willing to improve, but it might be worth looking at it.

So here's the story.

When you delete more than one theme in a row, some browsers will offer you this alert box, where you can check "Prevent this page from creating additional dialogs".

Once this is checked, a theme delete button no longer works.

After talking to @swissspidy we realized that this is a browser feature and from what I could gather online, it affects Chrome and Firefox as well.

Attaching screenshots.

[emiluzelac]

Prevent this page from creating additional dialogs

[emiluzelac]

Delete button

[afercia]

Testing this, after I checked "Prevent this page..." in the JS alert, the "Delete" button still worked pretty well for me the first time I clicked on it: the theme got deleted without any warning :(

After that, the "Delete" button stopped working. Though preventing JS alert is a temporary browser setting that only lasts for the current browser session (at least in Chrome, close the browser tab, reopen, go in the same page and alerts work again), the unintentional theme deletion should be definitely avoided.

[Kopepasah]

My findings:

Safari
When clicking Delete, the alert window appears and, one the second alert window, I am able to check the "Don't show more alerts from this webpage" checkbox. Once checked, I am no longer able to Delete the same theme; however, I am still able to Delete other themes without a problem (as the alert box shows).

Chrome
Similar to above, but I am not able to delete any theme once I have checked the "Prevent this page from creating additional dialogs" checkbox.

---

My suggestion, remove the alert window and use a different method for verifying a users intentions (e.g. an overlay dialog instead of a window alert).

[swissspidy]

My suggestion, remove the alert window and use a different method for verifying a user's intentions (e.g. an overlay dialog instead of a window alert).

Perhaps we can leverage the code used for the post lock dialog so we can use these dialogs in more situations where needed.

[vtieu]

Displays dialog when attempting to delete a theme.

[vtieu]

Great idea @swissspidy

Here's my first attempt at resolving the bug using the post lock dialog as suggested. Let me know what I can improve.

[swissspidy]

Replying to vtieu:

Great idea @swissspidy

Here's my first attempt at resolving the bug using the post lock dialog as suggested. Let me know what I can improve.

Awesome! Easier than I thought :)

I slightly updated the patch to include the translated text in the PHP instead of inserting it with JavaScript. Also, I changed the HTML slightly (h3 and paragraphs).

[swissspidy]

@afercia Got time to look at this from an a11y perspective?

[afercia]

I'd suggest to have a look at what was done for the wp-embed-share-dialog, see the aria role "dialog", the aria-label attribute used on the modal container, focus handling and constraining tabbing.

• when opening the modal, focus should be moved to the modal: probably making the container itself focusable since this modal doesn't contain just form elements but also text that needs to be made available to assistive technologies
• maybe add a Close button on the top right, in this case this should be the first element to receive focus
• when opening the modal, a CSS class modal-open should be added to the body and then removed when closing the modal
• pressing Escape should close the modal
• when closing the modal, focus should be moved back to the "Delete" button
• tabbing should be constrained within the modal
• the "Cancel" link without a href attribute is not focusable, should be a button with a type="button" attribute
• I'd consider to place the primary button on the left
• maybe event should be explicitly passed to deleteTheme
• when pressing OK to confirm the deletion, focus should be moved back to the most logical place (not sure where actually, maybe making the "Theme deleted" notice focusable)
• I'm probably missing something :)

There are several modal dialog used in the admin, each one with its own implementation, duplicated code sometimes, just a few modals have a sufficient level of accessibility. Sooner or later we should probably standardize all the modal dialogs an have a sort of mini "Modal API".

[swissspidy]

Replying to afercia:

I'd suggest to have a look at what was done for the wp-embed-share-dialog, see the aria role "dialog", the aria-label attribute used on the modal container, focus handling and constraining tabbing.

• when opening the modal, focus should be moved to the modal: probably making the container itself focusable since this modal doesn't contain just form elements but also text that needs to be made available to assistive technologies

Done in the latest patch.

• maybe add a Close button on the top right, in this case this should be the first element to receive focus

I'd rather like do that when settling for some kind of Modal API as you suggested.

• when opening the modal, a CSS class modal-open should be added to the body and then removed when closing the modal

This class is already present because of the theme overlay (which itself is a modal).

• pressing Escape should close the modal

Done in the latest patch.

• when closing the modal, focus should be moved back to the "Delete" button
• tabbing should be constrained within the modal

Both still to be done. Anyone wanna chime in?

• the "Cancel" link without a href attribute is not focusable, should be a button with a type="button" attribute
• I'd consider to place the primary button on the left
• maybe event should be explicitly passed to deleteTheme

Done in the latest patch.

• when pressing OK to confirm the deletion, focus should be moved back to the most logical place (not sure where actually, maybe making the "Theme deleted" notice focusable)

Still to be done.

• I'm probably missing something :)

There are several modal dialog used in the admin, each one with its own implementation, duplicated code sometimes, just a few modals have a sufficient level of accessibility. Sooner or later we should probably standardize all the modal dialogs an have a sort of mini "Modal API".

+1. If this ticket turns out to be more complicated than it seems, perhaps we should first start this standardization.

[obenland]

Themes are not the only instance where we use confirm(), it's used in ~24 other places in core.

I'm not sure the price we're paying with another API to circumvent this browser feature in all of the admin is worth the benefit. How often do users really delete more than one theme at a time, AND get annoyed enough by the AYS message, to prevent it from popping up? I'd like to think users can differentiate between a necessary AYS that is worth keeping around and a site just randomly opening dialogs.

[swissspidy]

After considering @obenland's wise words for a bit, this really sounds like a wontfix or maybelater. Having something custom for such dialogs would be nice to have, but that doesn't need to be tied to a release. It's really not worth the effort, it already took me a while to get the current patch together.

[afercia]

Replying to swissspidy:

it already took me a while to get the current patch together.

Yep, making modal dialogs accessible it's pretty time consuming :) Thinking WordPress should provide an easy way to do that, something like a simple function call to invoke a modal dialog and pass content. A standardized component. Just a thought, there's not even a plan about it, hopefully something to consider for the next future.
I'd vote for "maybelater" :)