Document-level click event listening breaks inside web components

Question

Document-level click event listening breaks inside web components

thibaudcolas opened this issue a year ago · 15 comments

I believe we’ve identified a bug in v8.0.0 when a dialog is within a web component, due to the changes in #387 to support #367. When processed at the document level, the click on a data-a11y-dialog-hide element will be reported with the web component as the event target, rather than the data-a11y-dialog-hide element or one of its descendants.

Here is a minimal demo to reproduce the issue: a11y-dialog v8.0.0 custom elements event delegation – CodePen. In the import map, I added the URL to the v7.4.0 release to confirm that it does work.

I can see three possible ways to fix this, though my understanding of event delegation with shadow DOM isn’t very good so there could well be better options.

Allow providing a "root" to the A11yDialog constructor to configure where click event listening happens. The default could be the document, while web components implementations could provide the shadowRoot.
Make this "listen to click events everywhere all the time" behavior optional, opt-in or opt-out.
Switch from event.target to something like event.composedPath()[0], otherwise keeping the logic the same.

From my perspective the most compelling is option 2, or any alternative that also limits the amount of unnecessary click event processing. Three Element.closest calls per click on the page might not seem like a big deal but I would assume it’s very common to have A11yDialog instantiated multiple times (once per component that can trigger its own modal), on pages where there are other interactive elements. In our case I can foresee situations where we’d get close to 1000 unnecessary Element.closest calls per minute while users interact with our app due to click interactions on pages with multiple instances of A11yDialog.

Answer 1 · 2023-09-30T12:27:30.000Z

Hello Thibaud,

Thank you very much for opening an issue, and my apologies you faced some issue when using a11y-dialog. I have been trying to reproduce the problem with the CodePen you shared, but I get this.shadowRoot is null:

Could you help me reproduce it? Then I’ll try to address it. :)

Answer 2 · 2023-10-03T13:38:36.000Z

Hi @KittyGiraudel, that error was caused by the use of the non-standard shadowrootmode on the <template> element that's only available on Chromium browsers.

I've refactored the codepen to use the standard approach of explicitly calling attachShadow():

The reproducible bug in v8 is here: https://codepen.io/laymonage/pen/VwqGEBR
With v7, where the bug doesn't occur: https://codepen.io/laymonage/pen/NWeLOER

Hope this helps. Thanks!

Answer 3 · 2023-10-03T15:54:53.000Z

Alright, I opened #589. I would love some review on it. :)

Answer 4 · 2023-10-10T10:48:26.000Z

When using Safari 17 on MacOS, if I open the accessibility checker, and then try to open the Wagtail user bar menu, the menu will animate open but then disappear before it can be accessed. Closing the accessibility checker enables opening the user bar menu as expected. This was just after setting up the bakery demo in Docker, Wagtail v5.1.2

Answer 5 · 2023-10-10T11:35:29.000Z

Thanks @digicase but that sounds like a Wagtail issue and not an a11y-dialog issue, so it's best to post it in Wagtail's issue tracker e.g. wagtail/wagtail#10924.

And thank you very much @KittyGiraudel for the fix, I can't review it atm but I've asked my friend to review it if he has time. Hopefully he'll get to it soon!

Answer 6 · 2023-10-10T11:37:22.000Z

Sorry about that, I must have arrived here via a link from Wagtail issues and didn't realise I was no longer there.

Answer 7 · 2024-05-16T05:28:31.000Z

Alright, I opened #589. I would love some review on it. :)

LGTM! We are coming across the same issue using a11y dialog in web components when there are multiple dialogs

Answer 8 · 2024-05-16T07:45:03.000Z

I managed to get around this by setting a11y dialog to look at my host element in my modal component. That way the aria-modal attribute is attached at the light dom level and the event bubbling works.

Answer 9 · 2024-06-18T08:26:11.000Z

Ran into the same issue… we could benefit from the fix. Do you see any soon merge attempt of #589?

Answer 10 · 2024-06-18T08:31:25.000Z

Hey Sven! If you can confirm that the fix is working fine, I would be happy to release it. I never had any confirmation from anyone that this was indeed a decent fix, so I didn’t dare publishing it. :D

Answer 11 · 2024-06-19T12:36:44.000Z

I’ll try to test the fix, but maybe this needs some time… this error was reported to me where a team packed our components into a WC... I have to set up a test case.

Answer 12 · 2024-07-06T08:42:40.000Z

Gentle poke here. If anyone who experiences this issue wants to try out the fix from #589 (perhaps by manually modifying the code in node_modules or something like that) and confirm it works fine for your case, I’ll happily release it. :)

Answer 13 · 2024-07-24T07:22:23.000Z

Sorry, I’m sad, that I couldn’t test this out yet. :(
As I said, we are using the a11y-dialog in our design system and provide components. A team packed our component into a WC and had this kind of error. I didn’t had the time to reproduce the setup on my own. The team is using another modal dialog now :/

If someone is faster and smarter to set up a test case, I’ll appreciate it.

Answer 14 · 2024-08-18T09:53:22.000Z

Merged, but not yet released.

Answer 15 · 2024-08-18T13:07:48.000Z

Done in version 8.1.0. 🎉 I know it took forever, but I’m pretty confident with the fix now. I’ve also added more tests, so this is quite good. I’m looking forward to hearing your thoughts when y’all have tried it. :)