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

Using data-a11y-dialog-show with web components #712

Closed
ella-etch opened this issue Sep 4, 2024 · 4 comments · Fixed by #713
Closed

Using data-a11y-dialog-show with web components #712

ella-etch opened this issue Sep 4, 2024 · 4 comments · Fixed by #713

Comments

@ella-etch
Copy link

Hi!

I’ve just upgraded to v8.1.0 in our web component based design system and it’s fixed a problem we were having with shadow DOM and focus trapping, so thanks 😄

However, we now have an issue when trying to use data-a11y-dialog-show on a button custom element to trigger showing the modal (which is also a custom element). It looks like the change from event.target to event.composedPath()[0] here means that we're now getting back the slot the button is in, rather than the button itself, so .show() isn’t being called.

I noticed there was a previous commit that accounted for a slot being returned and fell back to using event.target - I think reverting to that would fix the issue?

@KittyGiraudel
Copy link
Owner

Good morning Ella, and thank you for opening an issue! ✨

I was thinking about it on my way home last night, and I think the problem is that Element.prototype.closest does not cross shadow boundaries. So if our click target ends up being resolved as a <slot> (as you rightfully pointed out), then using .closest(..) to find the data-a11y-dialog-show ancestor doesn’t work because it stops at the shadow root.

If I am right about this, then there are two ways to address it:

  1. Find the shadow root first (by using event.target if event.composedPath()[0] yields a <slot> as we’ve suggested in the former commit), so that .closest(..) operates exclusively in the light DOM.
  2. Use a version of Element.prototype.closest that crosses shadow boundaries, so that it doesn’t just stop when reaching the first shadow root.

I think the 2nd version is better because it would work with however many nested web components, which is probably what we want (think a button inside of a card inside of a dialog or something).

I need to spend a bit of time coming up with a reproducible test case first so that I can try fixing it. :)

@KittyGiraudel
Copy link
Owner

Addressed in #713. Do you want to venture a review, @ella-etch? :)

@ella-etch
Copy link
Author

Hey @KittyGiraudel, thanks for doing this so quickly!

I've had a look through the code and also tested it out on our project - it all looks great, so I've left an approving review 💯

@KittyGiraudel
Copy link
Owner

Solved in 8.1.1. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants