KittyGiraudel / a11y-dialog

A very lightweight and flexible accessible modal dialog script.
https://a11y-dialog.netlify.app
MIT License
2.42k stars 130 forks source link

Add custom event in escape handler #695

Closed joe-watkins closed 3 months ago

joe-watkins commented 3 months ago

Hello! ;)

This PR is for #694 It adds a custom event to the Escape key handler.

The name of this custom event is hardcoded at this stage and is called dialog:escape

Use Case: Dialog has a child component that needs the escape key for its own functionality. For example, an ARIA Combobox with an expanded Listbox where escape should collapse that expanded Listbox instead of closing the dialog.

If a11y-dialog users wanted to get in front of the default Escape key functionality they would listen for this new custom event dialog:escape and simply use preventDefault(); in that component's JavaScript.

Example use: ARIA APG Select Combobox init

Select.prototype.init = function () {
    /* ... */
    document.addEventListener('dialog:escape', (event) => {
        if (this.open) { // Check if the dropdown is open
            this.updateMenuState(false); // Close the dropdown
            event.preventDefault(); // Prevent default behavior stops escape from working in a11y-dialog
        }
    });
}

I look forward to seeing what you cook up for documentation and tests and hope others enjoy this addition. Thanks for supporting a11y-dialog too @KittyGiraudel

KittyGiraudel commented 3 months ago

I spent a bit of time on this:

  1. I massaged the code a little bit to use the fire method (which already creates and dispatches CustomEvent). One thing I’m not super sure about is whether it’s fine to make all dialog events bubble, but I think it should be safe?
  2. I added some end-to-end tests to make sure that this feature works as expected.

I think from a code perspective, it looks pretty good now. But it would be nice to have a review again @joe-watkins. :)

KittyGiraudel commented 3 months ago

Actually before moving on, I want to have a look whether we could simplify this whole thing by always checking whether the custom event was cancelled, and if it was, not do anything (show, hide, destroy…). This way, we could probably do something like:

Select.prototype.init = function () {
    /* ... */
    document.addEventListener('hide', (event) => {
        const closingWithEscape = event.detail?.key === 'Escape';
        if (this.open && closingWithEscape) { // Check if the dropdown is open
            this.updateMenuState(false); // Close the dropdown
            event.preventDefault(); // Prevent default behavior stops escape from working in a11y-dialog
        }
    });
}

That would make it better I think. But I need to see if I can make it work.

KittyGiraudel commented 3 months ago

Moved to #696. Wanna review it @joe-watkins? :)

KittyGiraudel commented 3 months ago

Closed in favor of #696.

joe-watkins commented 3 months ago

@KittyGiraudel Thanks so much for thinking this through more and coming up with a more sound solution :) I appreciate the effort!