Review accessibility of the modal

[thibaudcolas]

Specific things I'm not sure of or I think are wrong:

• Why does it stopPropagation on click on the wrapper around the content?
• Is it really a good practice to set an aria-label on the modal content?
• Why is the close button a div? Could we use a button instead?
• There should be a screen-reader only textual label in the close button, and the "X" should be aria-role="presentation"
• The modal__overlay should probably be aria-hidden.

[samanthaksanders]

Some initial comments:

• Guess there are several ways to handle the close button. Add the aria-label to trigger and apply aria-hidden on span around "X". Or have two spans, one with accessibly hidden class and actual words Close modal window and the "X" with role="presentation" or aria-hidden. Several ways to do, not sure what's best. Just need to pick one and apply it :)
• Close button needs an aria-controls attribute connected to an id applied to the content dialogue
• The modal__overlay doesn't actually have any content so I predict it wouldn't get interpreted as anything anyway. But we could always add aria-hiddento be safe
• No specific reason why the demo close button is a div, this is perhaps legacy. It has a role="button" and tabindex set appropriately so it's not technically wrong. Can just be avoided by using a button 😄
• In general, I have concerns with the way the actual modal content gets added and deleted. Not sure how the accessibility api gets updated if we add content to the DOM using js. May want to consider rendering the content by default and simply show/hiding it a bit more? I guess you react experts will maybe know more about this than me so it's just food for thought.