Close button doesn't work when added after modal shown

[mockdeep]

We have a workflow where we fetch the modal content from the server when the modal trigger is clicked. We want to give the user some feedback, so we show the modal backdrop with a loader and then dynamically add the modal content when the response comes back from the server. What I'm noticing, though, is that the modal close doesn't work with this workflow. It appears that event handlers are registered on any existing "hide" elements when the modal is first shown, but if the close icon is added after the modal is shown then it doesn't work.

We can work around this on our end by tracking clicks to the close button ourselves, but I wonder if there is an event delegation based approach that might work in a11y-dialog in order to support elements being added later.

[KittyGiraudel]

Hello there. 👋

Thank you for opening an issue. You’re absolutely correct with your description of the situation. Right now, the data-a11y-dialog-* are really just a convenience API. You can do without them — which you have to as you rightfully pointed out. If you have a suggestion to improve this without bloating the library, I’m all ears. :)

[mockdeep]

@KittyGiraudel yeah, I'm fine doing our current approach. In terms of event delegation, looking at these lines I can imagine instead putting the event listener on this.$el instead. E.g.:

this.$el.addEventListener('click', (event) => {
  if (event.target.dataset.a11yDialogHide !== undefined) { this._hide(); }
});

The awkward thing is, if you, say, have an icon inside your close button, it won't work if the icon is clicked:

<button aria-label="close" data-a11y-dialog-hide="true" type="button">
<i class="fa fa-close"></i>
</button>

You'd need to find out if any ancestors have it:

this.$el.addEventListener('click', (event) => {
  if (event.target.closest('[data-a11y-dialog-hide]')) { this._hide(); }
});

So not sure if that's worthwhile or has any weird edge cases I'm not thinking of. It does result in fewer event handlers being registered and less to clean up afterwards, though I doubt that's impactful enough for most people to worry about.

[KittyGiraudel]

I actually like it. I’ll think about it some more. :)

[KittyGiraudel]

Will be addressed in v8 via #387.

[KittyGiraudel]

This is now fixed in v8, currently released under the next tag. To try it before it gets published officially:

npm install a11y-dialog@next