KittyGiraudel / a11y-dialog

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

[Questions] questions from a curious student #454

Closed iamskok closed 1 year ago

iamskok commented 1 year ago

First of all, thanks a lot for such a fantastic library @KittyGiraudel! I've been going over the code and played with VoiceOver this weekend and have a few questions I hope you will be able to answer:

  1. Why does VoiceOver not announces the aria-labelledby attribute in the demo example? It only announces a "close button" and aria-describedby even though .dialog-container is focused. After removing the close button, VoiceOver starts announcing both (the aria-labelledby and aria-describedby attributes). I don't quite understand this behavior.
  2. What is the reason for focusing on the dialog element vs. using the first focusable element (e.g., close button)?
  3. Why is dialog markup continuously injected in the DOM vs. being injected on show? Are there any benefits to this approach? I've checked headless, react-aria, reach-ui and they all inject the markup on the dialog trigger action.
  4. What does role="document" on the actual dialog exactly doing?
KittyGiraudel commented 1 year ago

Hey Vladimir,

Thank you for reaching out. Let me try to answer your questions.

Why does VoiceOver not announces the aria-labelledby attribute in the demo example? It only announces a "close button" and aria-describedby even though .dialog-container is focused. After removing the close button, VoiceOver starts announcing both (the aria-labelledby and aria-describedby attributes). I don't quite understand this behavior.

I just tried it and I can confirm this behavior. It is peculiar, and I must admit I am not entirely sure why VoiceOver does not announce the dialog name via the aria-labelledby association.

I checked everything on my side, as well as some other libraries and recommendations and as far as I can tell, the implementation is correct. It’s worth mentioning that the name mapping works properly, as showed in the accessibility panel (here in Firefox):

Screenshot 2023-01-15 at 10 14 29

Maybe @hdv knows?

What is the reason for focusing on the dialog element vs. using the first focusable element (e.g., close button)?

The library used to focus the first focusable element until version 7.2.0 (very recently!) at which point we switched to focusing the dialog container. This was raised in https://github.com/KittyGiraudel/a11y-dialog/issues/169, after a discussion on Twitter.

There are a few reasons why we decided to do that:

  1. Assistive technologies announce the dialog title immediately (I mean, besides VoiceOver apparently?), which is pretty good for context — most likely better than whatever is the first element.
  2. Consistency: it does not matter what content lives inside the dialog, it will always focus the dialog itself when open.
  3. Even though it‘s not recommended, it plays nicely with dialogs without a focusable element or with dynamically injected focusable elements.

    Why is dialog markup continuously injected in the DOM vs. being injected on show? Are there any benefits to this approach? I've checked headless, react-aria, reach-ui and they all inject the markup on the dialog trigger action.

I assume you meant “continuously present” instead of “continuously injected”?

I think it stems from the fact that all the implementations you have checked are React-based, where components are typically rendered when needed. Remember that a11y-dialog does not make assumption about the frontend stack. ;)

It’s possible to do the same with a11y-dialog as well: one could inject the dialog markup on interaction and flush it when closing the dialog. It’s just not exactly necessary.

What does role="document" on the actual dialog exactly do?

This is a helpful leftover from the original library from which a11y-dialog originated. I quote from the original doc:

The contents of the modal are wrapped in role="document". This is to aid NVDA users so they can more easily browse the contents of the modal. NVDA previously added support for fully browsing the contents of the modal, but it requires the user to switch browsing modes in NVDA. Using role="document" automatically puts the user in the mode where they can fully browse the contents.

iamskok commented 1 year ago

@KittyGiraudel thanks for your detailed answers! I did some more tests with the demo and it turns out that VO is working as expected on Safari (announcing both aria-labelledby and aria-describedby attributes), but is skipping aria-labelledby on both Firefox and Chrome.

It seems like the "fix" is to move the close button after the title and description elements. I was able to get consistent announcements of aria-labelledby and aria-describedby attributes on all three browsers. Here is the example.

The one annoying thing I can see with this approach is that it will become cumbersome to use it with React/Vue and when children/slot is used to pass HTML element for aria-describedby. We will be forced to set close button as the last element which will mess with the focus order (the first tab will not focus on the close button). Maybe there is some smart workaround for this?

// app.tsx

<Dialog
  toggle={toggleDialog}
  isShown={isDialogShown}
  role='dialog'
  labelledby='dialog-title'
  describedby='dialog-description'
  title='My awesome dialog title'
  closeLabel='Close dialog'
>
  <p id='dialog-description'>This is dialog description</p>

  <button>save</button>
  <button>cancel</button>
</Dialog>
// dialog.tsx

<div
  tabIndex={-1}
  id='dialog'
  ref={dialogContainer}
  className='dialog-container'
  aria-hidden={!isShown}
  aria-modal='true'
  role={role}
  aria-labelledby={labelledby}
  aria-describedby={describedby}
>
  <div className='dialog-overlay' onClick={toggle()} />
  <div className='dialog-content' role='document'>
    <h1
      className='dialog-header'
      id={labelledby}
    >
      {title}
    </h1>

    {children}

    <button
      className='dialog-close'
      onClick={toggle}
      aria-label={closeLabel}
    >{'\u00D7'}
    </button>
  </div>
</div>
KittyGiraudel commented 1 year ago

(the first tab will not focus on the close button)

This is not normative in any way, so that would be okay I think. There is particular expectation that the first tab should focus the close button.

How about we add a section about this in the known issues page?

iamskok commented 1 year ago

I actually realized that my previous statement about React children and Vue slots is not true. Placing the icon after the dialog name associated via aria-labelledby (not after the dialog description associated via aria-describedby as I said above) "fixes" the issue. So this approach should work without issue with Vue and React as well.

KittyGiraudel commented 1 year ago

Thank you for your help! ✨