Add autocomplete screen reader support #48
Conversation
sample-app/src/sass/App.scss
Outdated
| @@ -22,3 +22,18 @@ | |||
| .result-description { | |||
| padding-left: 2em; | |||
| } | |||
|
|
|||
| .sr-only { | |||
| position: absolute; | |||
There was a problem hiding this comment.
What's the reasoning behind position: absolute here? Setting that can make managing the DOM more difficult because it pulls the element out of the normal flow of the document, and I want to make sure it's use is necessary here
There was a problem hiding this comment.
I got this from the sr-only styling in the SDK. Wouldn't we want screen-reader only elements to be outside the normal flow?
There was a problem hiding this comment.
Ya, that makes sense to me assuming the element is hidden some way. It looks like clip: rect(0,0,0,0) may be doing the hiding. I'm wondering if display: none would work instead
There was a problem hiding this comment.
display: none hides something from the screen reader as well as visually, so it shouldn't be used here
There was a problem hiding this comment.
Got it. There's also visibility: hidden which doesn't remove the element from the layout which might work and be a little clearer than clip
There was a problem hiding this comment.
visibility: hidden also hides it from screen readers, just like display: none
|
|
||
| const prevAutcompleteCountText = autocompleteCountRef.current.innerHTML; | ||
|
|
||
| if (shouldDisplayDropdown) { |
There was a problem hiding this comment.
should this go in a useEffect? generally speaking there shouldn't be side effects in a render function, since the function itself may be called multiple times, and I believe multiple renders can be batched into a single DOM update. I think that right now, if the component rerenders for some reason that isn't the user typing in different text, that the screen reader will do a fresh read even though the input text has not changed? This might be an impossible scenario right now but is just an example.
There was a problem hiding this comment.
I was preventing duplicate announcements with the check in updateAutcompleteCountText that only sets the innerHTML if the number of options changes. so, I believe the only time the screen reader would announce something is if the number of options changes or if the user input changes. but, if it is still better to put it in a useEffect, then I can try moving lines 84-90 into the useEffect that is a little later in the code
|
This is already looking good! think we just need to iron out the ScreenReader interface |
Add a
ScreenReadercomponent that is used to provide screen reader instructions and announcements. Use this component inInputDropdownto give autocomplete instructions and screen reader announcements for number of autocomplete options found.J=SLAP-1402
TEST=manual
Check that screen reader instructions and announcements are correct in the sample app. An announcement is made for each input change and on clicking the input bar if there is input present.