kittygiraudel / a11y-dialog Goto Github PK

When the dialog is open the user cannot access the browser's address bar using tab navigation. The browser address bar is part of the default focus loop and some users may rely on that convention.

Will you be supporting the upcoming dialog element for HTML 5.2?

https://bitsofco.de/whats-new-in-html-5-2/
https://www.w3.org/TR/html52/interactive-elements.html#elementdef-dialog

Maybe?

this seems to be a common issue with dialog libraries. the way i worked around it for my own implementations is that i disabled scrolling for the content in the background and added a scrollbar for the dialog modal.

In the DOM API it is possible to put the ID of the modal I want to show/hide, in the JS API that is not possible. I would love to pass an ID to show() or hide() to address exactly the modal I want to show/hide.

I notice, if you don't include the button to open the dialog I get a couple of console errors in Safari.

In some cases we might not need an open button. The modal might be triggered by something else. For example an evil email subscribe popup.

I could of course use CSS to hide the button. But is there a better way to handle those kind of cases, when a button isn't required?

E.g. data-hide-modal="my-accessible-modal".

Reference: #11 (comment)

Pros:

• Takes care of the hiding part, which removes the need for some extra CSS
• Hidden from any user, not just users with assistive technology

More info here: https://www.paciellogroup.com/blog/2012/05/html5-accessibility-chops-hidden-and-aria-hidden/.

Hey,

One of the things I test when I have to implement a dialog is how it handles a large amount of content.

As of today, a11y-dialog dispatches the event target (event.target) as the trigger on events in the internal _fire method:

A11yDialog.prototype._fire = function (type, event) {
  var listeners = this._listeners[type] || [];
  var trigger = event ? event.target : void 0;
 
  listeners.forEach(function (listener) {
    listener(this.node, trigger);
  }.bind(this));
};

This works okay but in case the toggle has sub elements, such as embedded SVG for instance, the dispatched node might be something unexpected. Right now, implementors can use Element.prototype.closest to retrieve the toggle itself:

dialog.on('show', (dialog, trigger) => {
  trigger = trigger.closest('[data-a11y-dialog-show]');
  // …
});

Note: in case the JS API is used over the DOM API, the selector could be something else of course.

It might be better to dispatch the current target (event.currentTarget) which always refers to the element to which the event handler has been attached, as opposed to event.target which identifies the element on which the event occurred.

This would be a breaking change and would have to be part of 4.0.0.

In order to promote the modal script better, it would be nice to have some fancy styling. It is a bit dull right now.

Quick headsup; on my 27" screen the offset for hiding items offscreen is not low enough and I still see them on screen:

.screen-reader-offscreen {
    left: -999px; // should be even lower (e.g. -9999px)
}

The README should say what browsers are supported, or more likely what legacy browsers are not.

Since you’re using vanilla JS I suspect that means IE8 is out, but confirming that IE9 is supported (and maybe fixing the few JS things that IE9 doesn’t understand) would be nice.

Bower? npm? What else?

In IE11 (tested in 11.494) the following error is logged to the console:
Object doesn't support this action
triggered by line:
event = new CustomEvent(eventName);

The issue with CustomEvent is documented on MDN here:
https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent/CustomEvent

It seems that some version of IE do offer a window.CustomEvent property but that it's missing some methods.

I’m curious why you are writing your own implementation of tabbing:

https://github.com/edenspiekermann/a11y-dialog/blob/master/a11y-dialog.js#L112
https://github.com/edenspiekermann/a11y-dialog/blob/master/a11y-dialog.js#L263-L265
https://github.com/edenspiekermann/a11y-dialog/blob/master/a11y-dialog.js#L357-L374

If you are trapping focus:

https://github.com/edenspiekermann/a11y-dialog/blob/master/a11y-dialog.js#L111
https://github.com/edenspiekermann/a11y-dialog/blob/master/a11y-dialog.js#L275-L281

And expecting a hide-able DOM structure:

https://github.com/edenspiekermann/a11y-dialog#expected-dom-structure

Why not just let native tabbing work? Also, remember that keyboard navigation is not limited to tabbing (VoiceOver uses CMD+Option+Arrow), which is also covered by your DOM structure + trapping focus.

Could this script be simplified, or is this solving another problem?

data-hide-modal -> data-modal-hide
data-show-modal -> data-modal-show

Developers may need to set the display value of the modal to something other than block when showing it. jQuery fixes this in their show/hide methods by reading the calculated CSS values for the element (which is a bit costly) and storing that info in memory when hiding with display:none.

A simpler option might be to allow developers to disable the CSS display block/none mechanism, and only update the aria-hidden attribute. This enables developers to set their own display:

.modal { display: flex; }
.modal[aria-hidden="true"] { display: none; }

Or maybe an even simpler fix would be:

  Modal.prototype.show = function () {
    ...
    this.$overlay.style.display = '';
    this.$modal.style.display = '';
    ...
  };

there’s also:

  Modal.prototype.show = function () {
    ...
    this.$overlay.style.removeProperty('display');
    this.$modal.style.removeProperty('display');
    ...
  };

but I’m not sure about browser support for any of those two last solutions.
Apparently style.removeProperty() is from a 15 year-old spec so it might have decent support.

Sorry that’s a bit long, thinking and researching out loud here. :)

Thanks for great accessible modal dialog script. I've been looking something like this for a while.

I understand perfectly why there is id="main" or passing the ID as a second parameter. However when I wanted to use this script in public WordPress Plugin where I can only take educated guess about ID names. In other words I don't know html markup.

I guess my question is that would second parameter also be false if I have no clue about the markup?

I have changed the ID to reflect the certain modal box, but nothing, Could you please assist with this?

For styling and logical purposes, it would be better to have the modal be a single container, not a container + overlay.

Typical structure can be:

<div class="modal" hidden>
  <div class="modal-overlay" data-modal-hide></div>
  <div class="modal-content">
    <!-- Your content -->
    <button type="button" class="modal-close" data-modal-hide
            aria-label="Close this modal">&times;</button>
  </div>
</div>

Keeping the overlay separate allows users to animate it separately from the main content (e.g. make the overlay appear first, content second). Users can remove the data-modal-hide from the overlay if don’t want clicks on the background to close the modal. Thus we’re making good use of the API.

If developers want different layouts, they can use their own structure and CSS, since the script would act only on the container element and on data-modal-* attributes.

On mobile devices, when you open the modal, it auto-focuses on the first interactable element. This is good for accessibility. But it also brings up the keyboard automatically on opening the modal, which pushes the modal up, thereby mostly or completely hiding the dialog title, even on modals with very little content (like the demo). This can be somewhat confusing, as in some circumstances you can end up being presented with a form without a title, and you have to scroll (or close the keyboard) to figure out what the dialog is about.

Potential solution:

Focus on the dialog title -- either by adding tabindex="0" in the html (this may be undesirable), or via javascript. If it is added via javascript, then maybe this option could be made part of the library?

IMO: Instead of focusing the first focusable element in the modal it would be better to focus the close button of the modal. So if a keyboard user accidently opened the modal doesn't need to tab through all elements to close it.

You should probably look at the element, as this HTML5 element will eventually give much better symantics for assistive devices (or simply the browser) to use.

For example, when "open" and "modal", supporting browsers will limit the focusable controls to those elements within the dialog (e.g. tabbing between elements, won't focus anything else on the page, so you don't need to do anything in JS to change the tabindex).

https://developer.mozilla.org/en/docs/Web/HTML/Element/dialog

In the example code (in README.md) related to events triggering on opening/closing a dialog:

dialog.addEventListener('dialog:show', function (e) { // e.target.id });

The event listener is being attached to the A11yDialog js object, instead of the DOM element that contains the dialog, which in this case, given the code above, would be dialogEl. This may confuse some people who copy-paste example code and expect it work (yeah, that is totally not how I found it! :)

This is not an issue per se but I wonder about the position of the "close" button in the markup.
I believe sighted keyboard users would expect to reach the button after tabbing out of the heading—since that's the visual flow.

What about having 2 buttons? One after the heading and one last in the dialog? With the same styling/positioning of course. Does that make sense?

Hey bro,

We are trying to use your awesome dialog in our future project but we have some troubles using it due to the reference to window element.

We are writing ES6 code which is then transpiled into AMD modules loaded through requireJS.

So in one of our modules, we have something like this:

import A11yDialog from 'a11y-dialog';

function foo() {
   let dialog = new A11yDialog(...);
}

export default foo;

It's not working because the a11y-dialog doesn't export anything, it binds a class to the window object.

To make our code work, we have to do something like this:

import NeverUsedObject from 'a11y-dialog';

function foo() {
   let dialog = new window.A11yDialog(...); // or just new A11yDialog(...)
}

export default foo;

But a code like this seems weird so we would like to have something more logic: the class imported from the a11y-dialog script is the class we instance to create a new dialog.

I'm not used to NPM publishing techniques, especially when it mix ES6 and ES5 stuff but I'm wondering if something can be done to avoid binding the A11yDialog class to window but to be able to return it instead.

Cya.

When the modal is open and i tap the close button.

Expected: The dialog closes
Actual: Dialog remains open, close button has focus styling

Edit:

Recreated in:

• Chrome 58 on android 7, One+ 3T
• Firefox 52 on android, One+ 3T
• Chrome 55 on android, Samsung Galaxy S7 edge
• Safari 10.3 on ios, iPhone 6+

Thanks for your work on this. It's a nice and small modal implementation. I'm wondering what your stance is on opening a modal on too small viewports. The obvious answer is of course to not open a modal. Can this modal implementation be improved by enabling scrollbars in that case or similar solutions?

It looks like the 2.5.1 release introduced the following exception:

main.js:77 Uncaught TypeError: this.hasAttribute is not a function

This occurs in the example code, in the a11y-dialog constructor (main.js, line 77). I'm not sure what the intention was here, but this in this is an A11yDialog instance (which is really just a plain JS object), not an Element.

The default id used to retrieve the global container is main, and while making sense, might be too generic for some scenarios. It would be preferable to use something unique such as layout-wrapper or such.

Edit: A11yDialog has been chosen. Please check #22 (comment) to give your opinion!

Modal seems a bit too generic for a global variable. It would be a good idea to find a better name. However it will be a breaking change (v2).

Ideas:

• AccessibleModalDialog (a bit long?)
• AMD (probably not a good idea)
• ESPIDialog (not a fan of branding everything)
• AMDialog
• A11yDialog
• Any other suggestion?

It would be really useful if opening/closing a dialog could trigger a relevant event.
Will happily submit a PR if that would speed things along.

I have a setup where I open a modal and then want to step from the currently open modal to the next one. From the second modal I want to go to either the previous modal, or the next one, and so on.

The pseudo-code of the buttons inside the modal that enable this behavior:

Modal 1

<button data-a11y-dialog-hide="modal-1" data-a11y-dialog-show="dialog-2" >next modal</button>

Modal 2

<button data-a11y-dialog-hide="dialog-2" data-a11y-dialog-show="dialog-1">prev modal</button>
<button data-a11y-dialog-hide="dialog-2" data-a11y-dialog-show="dialog-3" >next modal</button>

… and so on.

Now, when I click on the "next" button in the first modal, everything works as expected, the modal 1 hides and modal 2 shows up. In fact, the "next" button of all modals works just fine.

But when I click on the "prev" button e.g. in the second modal, to go back to the first modal, I get the following error in Chrome, Opera, and Safari (aka. Blink and WebKit browsers):

Uncaught RangeError: Maximum call stack size exceeded.

Test scenario: http://jsbin.com/bumimohexi/1/edit?html,js,output

What is causing this and how can I avoid it?

The behaviour when opening and closing the dialog is not very intuitive or meaningful and could be improved by animation.

A couple of styling hooks on the dialog would be great to allow devs to add their own animation to the opening and closing of the dialog.

Hooks like .is-open .is-opening .is-closed .is-closing would be awesome.

I noticed something interesting while running some unit tests tonight: the keydown handler added by the JavaScript API to the document object is never removed. You get a new handler attached each time you create a new a11y-dialog instance, so in a large single page application that could be quite a few handlers. It seems like there needs to be some sort of cleanup method that consumers should call.

If a site has multiple dialogs I think we only really need one instance of the overlay div. Currently it's part of the dialog which means we end up with multiple copies of the same code. Maybe this could be refactored to make it DRY.

Is it possible to update the URL when modal is opened to make it linkable or is that not good practice?

I'm using a11ydialog to open up a small form for the user to provide input. One of those elements is a select list using the select2 (https://select2.github.io/) library for auto-completion of remote data. The way select2 seems to work is the markup that acts as the auto-complete drop-down is created as an immediate child of the body tag. This means that is is a sibling to the a11ydialog markup.

I can work around any styling issues so that the select2 auto-complete markup is visible, however I noticed that the maintainFocus function is not allowing the user to type text into the select2 widget.

Is there a way to optionally allow markup outside of the a11ydialog markup to have focus? Or is there any other recommendations?

Something like https://github.com/drublic/css-modal/blob/master/modal.js#L516-L538 would be nice.

Use case: I want to display a form in the dialog. When the user accidentally clicks outside the dialog or on the close button, the dialog closes without warning. I could check in the hide event listener if the user has already entered some data and confirm if they really want to close.

It would be great if I then could just return false in the event listener to cancel the action. Maybe a beforeHide event would also be valuable.

The same can also be applied to the show event, though I guess it won't be too helpful.

I know that the recommendation is to have this css:

[aria-hidden="true"] {
  display: none;
}

However that solution leaves much to be desired in terms of in-out transitions using CSS.

Since the modal code already implements the ability to disable focus on elements, as used on the modals siblings, would it be possible to do the same thing for the modal content when closing the modal?

This would enable the ability to use other means of hiding the visual side of the dialog when closing it, which includes transitions in and out.

What happens:

The user taps the tab key several times to be able to activate the modal dialog.

The (frustrated) user cannot use the tab key so he reaches the mosue to click on the modal dialog link.

The dialog opens (expected)

The user tries to dismiss the dialog by trying to tab through the links and focus on the (x) button and tap (spacebar) on it.

The user cannot do it, and the user is even more frustrated.

Expectation:

Make the user happy by implementing tab indexes to the demo page, and also to the dialog.