google / site-kit-wp

Site Kit is a one-stop solution for WordPress users to use everything Google has to offer to make them successful on the web.
https://sitekit.withgoogle.com
Apache License 2.0
1.23k stars 279 forks source link

Fix glitches when closing various modal dialogs #9137

Open techanvil opened 1 month ago

techanvil commented 1 month ago

Feature Description

There are a number of glitches evident that occur when closing various Site Kit modal dialogs.

Note that a dialog can be closed via the Cancel button, or clicking outside the dialog/pressing the Escape key.

When a dialog is closed, it should restore focus to the previously focused element. However, some dialogs are not restoring the focus when closing it via one or more of the above closing mechanisms. Additionally, the button that opens the dialog is being broken in some cases.

To illustrate, here's a mostly well-behaved dialog. As can be seen the Dashboard Sharing dialog restores focus to the button that opens it when closing via the Cancel button, when clicking outside the dialog, and when pressing Escape. Clicking the button to open it again works as expected. The only minor glitch evident here is the tooltip on the button is a bit inconsistent and only pops up on refocusing in the case where the dialog is closed via Escape.

https://github.com/user-attachments/assets/4f79a61b-da85-4823-86bb-126da1933849

Here's a less well-behaved dialog. The Consent Mode dialog does restore focus to the Enable consent mode switch when closing by clicking outside the dialog or pressing Escape. However, it doesn't restore focus when closing by pressing Cancel. Furthermore, it actually breaks the switch when closing by clicking outside the dialog or pressing Escape.

https://github.com/user-attachments/assets/4b045002-f94d-423f-901d-4a0258eabda3

It turns out that all of our dialogs have one or more glitches that need addressing.

Note that the Dashboard Sharing dialogs will be addressed via a separate issue, https://github.com/google/site-kit-wp/issues/9138, as their defects and implementation are a bit different to the other dialogs.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation Brief

FIx for button getting broken after clicking outside the modal to close it

Test Coverage

QA Brief

Changelog entry

zutigrm commented 4 weeks ago

I detected the reason for button being broken after clocking outside the Modal. But focus not getting back to the button that triggered it is pretty weird occurrence. What I tracked down is that once modal is closed, the focus is switched to the body, and only once you click anywhere on the page, focus returns to the original element that triggered the modal. Couldn't identify the reason focus in some modals goes back to the body first before returning to the button. Un-assigning myself in order not to spend too much time going down the rabbit hole, I included the first part of the fix in the IB, for the focus issue - leaving it someone else who might be more familiar with this ModalDialog component or have run into the reason for this focus shift before.