Studio: Bulk delete preview sites action not working

[gavande1]

Related issues

• Fixes STU-549

Proposed Changes

I proposed a fix for the bulk delete preview sites action not working by using aria-modal instead of inert attribute. That attribute was added as part of #1423 PR to fix the accessibility warning. Using aria-modal fixes those issues and it also fixes the issues with Bulk delete preview sites action.

Read more about aria-modal attribute: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Attributes/aria-modal

Testing Instructions

• Check out this PR
• Delete rm node_modules folder and then run npm install && npm start
• Make sure you have a couple of preview sites
• Navigate to a different tab than Overview and go back to the Overview tab, for example, navigate from Overview -> Sync and then from Sync -> Overview
• Go to the Preferences screen
• Navigate to the Usage tab
• Click the ︙(three dots) menu
• Check that "Delete all preview sites" is clickable.

Also, follow the test instructions from PR #1423 for any regression.

Pre-merge Checklist

• Have you checked for TypeScript, React or other console errors?

[katinthehatsite]

I did not review this yet but wanted to mention that I added inert in this patch to the modal: https://github.com/Automattic/studio/pull/1423/files to solve the accessibility warnings it was throwing 🙈
We might walk to double-check what is the best approach here.

[gavande1]

Interesting. I wouldn’t have guessed about this change. Thanks, @katinthehatsite, for sharing the context. I used aria-modal instead of inert to eliminate the console errors, which also resolves the issue with the delete action button.

[epeicher]

Thanks Rahul for the fix! This was a great sleuthing, I wonder how did you identified the attribute that was causing the issue? 🕵️

I have tested it, and I can now click on the Delete all preview sites button in different scenarios. I have also tested the related task and I don't see any warnings so this LGTM!

Delete all preview sites	Add new site (no warnings)

[gavande1]

@epeicher Thanks for the review.

how did you identified the attribute that was causing the issue?

Thanks for asking. Here are the steps I followed:

• Initially, I confirmed that all props passed to the component were correct and as expected.
• There was nothing unusual in the component logic or prop values.
• This led me to suspect that the issue might be related to the DropdownMenu not working properly inside the Modal.
• I reviewed Dropdown in Modal Doesn't Work WordPress/gutenberg#19453, which described a similar problem.
• That issue gave me the idea to inject a Popover.Slot, hinting that the root cause might be related to how the DOM was being managed.
• I inspected the DOM closely and noticed that an inert attribute was being applied to the popover container. So removed it inline and it worked.
• Based on this, I tried appending a custom popover component in the modal scope, and it worked perfectly.

[gavande1]

Hey team, I would prefer to receive more reviews before merging this, as I would like to include it in the current release.

[bcotrim]

Changes LGTM 👍
Did not notice any regression, no console errors and delete all preview sites action worked.
Nice catch!

[ivan-ottinger]

Great investigation! I can confirm as well the change fixes the issue and I did not observe any regressions.

[wojtekn]

How do those changes influence compatibility with VoiceOver?

[wojtekn]

I retested it and it works fine now. 🥳

[gavande1]

How do those changes influence compatibility with VoiceOver?

Based on my understanding so far, aria-modal should improve screen reader capability. The aria-modal attribute is used to indicate that a modal dialog is a modal, meaning it traps focus and that users should not interact with the rest of the page while it is open. This improves accessibility by helping screen readers understand the modal context.

Previously, aria-hidden="true" and the inert attribute were applied to the popover container outside the modal to prevent interaction with it while the modal is open. Probably, this is the reason why Chrome complained about having aria-hidden="true" to the modal container while having an interactive element within.