[EuiModal] Accessibilty and experience updates

[thompsongl]

Recent focus trap updates (#3631) raised the question of the default behavior of EuiModal as it relates to ownFocus and other features the new library affords (scroll locking and pointer-event disabling).

Let's discuss what (if any) changes to make, and what longstanding a11y issues we can resolve in the process.

• ownFocus by default
• #502 and #699 propose various changes
• EuiOverlayMask as part of EuiModal

[thompsongl]

@myasonik Do you have a more curated list of a11y changes/improvements?

[myasonik]

• I think EuiModal already has ownFocus on by default and we brought up adding ownFocus on by default for other components (such as Flyovers and Popovers).

• For basic markup, a dialog doesn't need too much:

  section role="dialog" aria-modal="true">
  <button aria-label="close modal">x</button>
  {content}
  /section>
  

  • A dialog always needs a name (aria-label or aria-labelledby). If there's a title in the modal, aria-labelledby should point to it. aria-describedby is optional.
  • If there's a title, it should default to an h1

• For functionality, we should have:

  • On esc, close the modal
  • the focus-lock stuff
  • On clicking outside the modal, close the modal (optional)
  • When the modal opens, focus needs to move into the modal. It can go on the wrapping container (in which case you add tabindex=-1 but it can also go to an internal form field if the modal is a form or ok/cancel for confirmation modal. (Always going to the wrapper is an ok default experience.) (It shouldn't default to the close button.)
  • When modal closes, focus on the button that opened the modal. If it no longer exists, user should provide where focus goes. (Ideally, this would be easily overwritable but heavily enforced because we're terrible at ensuring focus isn't lost.)

• Probably just a docs addition, it's sometimes recommended to add a note to buttons that they open a modal if the button text isn't clear about it:

<button>
  Add a thing
  <EuiScreenReaderOnly>Opens a modal</EuiScreenReaderOnly>
</button>

[thompsongl]

@cchaos I recall you mentioning wanting to add an overlay mask to EuiModal by default, also. Does that sound right?

[cchaos]

Yeah I think it makes sense to have it similar to flyout, where ownFocus comes with the overlay mask.

[github-actions[bot]]

👋 Hey there. This issue hasn't had any activity for 180 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

[nansing90]

@thompsongl Please let us know what is the proposed solution for this bug?

[thompsongl]

Looks like the only remaining recommended updates to EuiModal are attribute and ARIA additions:

For basic markup, a dialog doesn't need too much:

<section role="dialog" aria-modal="true">
	<button aria-label="close modal">x</button>
	{content}
</section>

A dialog always needs a name (aria-label or aria-labelledby). If there's a title in the modal, aria-labelledby should point to it. aria-describedby is optional. If there's a title, it should default to an h1

[thompsongl]

We could consider adding outsideClickCloses boolean prop like EuiFlyout has, also.

[cchaos]

We could consider adding outsideClickCloses boolean prop like EuiFlyout has, also.

We've specifically said no to this request every time it has come up based on our guidelines of usage (that every modal must have a distinct close button).

[1Copenut]

@daveyholler Let's review this in next backlog groom. I think it might be able to close.

[daveyholler]

Already complete