A11Y: Improve create account modal for screen readers (PR #14204)

Improves the create account modal for screen readers by doing the following:

  • Making the alert-error section into an aria-live region and making it always visible (configurable per-modal), otherwise the message is not read out. Made a change so the field-related error messages are always shown beneath the field.
  • Add aria-invalid and aria-describedby attributes to each field in the modal, so the screen reader will read out the error hint on error. This necessitated an Ember component extension to allow both the aria-* attributes to be bound and to render on {{input}}.
  • Moved the social login buttons to the right in the HTML structure so they are not read out first.
  • Added aria-label attributes to the login buttons so they can have different content for screen readers.

GitHub

can you try to remove jquery usage here while you are at it please ?

same here, would probably be easy to ditch jquery

can you format this properly while you are at it please ?

format

why queryAll and not query ?

I think I would prefer something like: sr_title or screen_reader_title

I think this absolute positioning here will cause issues: on mobile, on other locales (for example if the message exceeds the vertical space afforded by the 6em padding) or on other modals that don’t have the top padding.

Can you keep the :empty pseudoelement, but instead of hiding with display: none or visibility: hidden, maybe try something like this:

  #modal-alert {
    display: block;
    padding: 1em 3.5em 1em 1em;
    min-height: 35px;
    ...
    &:empty {
      min-height: 0px;
      padding: 0px;
      overflow: hidden;
    }
  }

I don’t have NVDA handy, but in a regular browser that seems to behave correctly.

@martin-brennan did you consider using this Using the alert role - Accessibility | MDN ?

This return is unecessary (unless you specifically need null vs undefined which would be unusual!)

This should be imported. Ember as a global will be removed.

No idea, was there in the test already, will change

Sure thanks for the suggestion, I didn’t really know how to do it well so will see if this works

@jjaffeux Ah yeah I think you are right with role="alert"; it is a live region with an implicit aria-live value of assertive and an aria-atomic value of true:

I will change this, and we should update our internal topic with guidance around using this.

Sure, will change this, and we should be consistent with this internally

@jjaffeux, @pmusaraj, @eviltrout can you please take a look again? aria-role=“alert” worked great and so did Penar’s suggestions to use height: 0; so now we have the show/hide of the error as before but now it works with the screen reader.

I grouped the fixes in logical commits, so hopefully should be easy to follow :+1:

having that many computed properties is a smell.

Can you think of any way to group some of them into one property we could use here ?

same comment about the number of properties, could be in another PR

I don’t want to annoy you too much with this but FTR, proper formating is:

{{input
  foo=bar
  baz=fooz
}}
``

@jjaffeux, @pmusaraj, @eviltrout can you please take a look again? aria-role=“alert” worked great and so did Penar’s suggestions to use height: 0; so now we have the show/hide of the error as before but now it works with the screen reader.

I grouped the fixes in logical commits, so hopefully should be easy to follow :+1:

looks good to me, thx for documenting this