-
Notifications
You must be signed in to change notification settings - Fork 83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Accessibility Testing #669
Conversation
Hey @BabyElias, excellent progress! 😍 Smooth keyboard navigation is great experience 👏🏽, but there are a few hickups when it comes to what is NVDA enunciating:
As a comparison, check the output in the NVDA Speech Viewer in this recording, with the same data table, just the plain vanilla HTML with no sorting or other interactive feature (attached): KTable.Component.Example.-.vanilla.HTML.mp4 |
@radinamatic, not sure but my output for NVDA screen reader on the same screen shows something different.
|
As @MisRob has suggested, the difference was in the browser:
At this point, what needs additional investigation is the behavior on Firefox, and implement the fix which will make NVDA output on Firefox the same as it is on Chrome... 🤔 |
Ahh, I see. Tried it on my own system too and had this issue with firefox. Let me see how can this be fixed. |
Hey @radinamatic @MisRob ! |
Thanks for looking into that @BabyElias. Yes, you're right that we need to work with I assume that the other issue, the content of the cell is not enunciated, is not related to this, or do you think it may be? |
And thanks for having conversation with NVDA team @BabyElias |
@MisRob, the other issue too is related to this one. Apparently, because of the tabIndex, it does not shift focus to the content of the cell and just perceives it as a layout box. This causes the "not selected" as well as the enunciation problem. |
@BabyElias Oh I see, I didn't get it from the issue |
@BabyElias Could we please mention this in the table documentation page with the link to the issue? (for now just a note, we can format it nice later when we get to working on docs for the table) |
@radinamatic Apart from documenting as suggested above, I don't think we can do anything else at this point |
Sure @MisRob. I'll add it in the documentation when making the next PR. |
Perfect :)! Let's wait for @radinamatic's final confirmation and I think we could merge since this was the only blocker, I believe. |
Thank you @BabyElias for researching and engaging with the NVDA contributors on the source repo, maybe this will help to raise the priority of that issue that has been reported years ago. However, even if the extraneous Do we have any other option of making the sorting column work without adding the |
Makes sense @radinamatic. I'll see what can be done in terms of finding a middle ground that helps the non-sighted users as well as supports the keyboard navigation. Thank you for your insights! |
Thanks @radinamatic, I appreciate this perspective. I thought that keyboard is a must, but now reading your comment, it makes sense. Let's see if we can do something. |
That's totally my bad, I should have been much clearer regarding the requirement priorities for this component, I'm sorry! |
Oh no no, no problem. It's a world of compromise :) Good discussion. |
@radinamatic @MisRob
|
@BabyElias Whatever magic you've put into this, you nailed it! 👏🏽 👏🏽 👏🏽 When navigating across the table in Firefox, using the Tab & arrow keys, I don't hear anything weird (not) selected , and I do hear the content of the cell, just as I should! 😍 Just in case you were unaware of this free a11y tool, If we were in the sector that was aiming at complete formal compliance to standards and regulations, I might have a different stance, but as we always give precedence to the user experience (and it is as we expect it to be), I don't think you need to invest much more effort in fixing these. Just as formal compliance according to some automated testing technology does not guarantee a good accessible UX, the accessible user experience may still have some aspects of the code that a checker will interpret as a non-compliant 🤷🏽 |
Thank you for sharing this @radinamatic ! That seems like quite a useful tool, surely going to explore it soon in the process :) |
@@ -13,6 +13,8 @@ | |||
tabindex="0" | |||
:aria-sort="sortable && header.dataType !== DATA_TYPE_OTHERS ? getAriaSort(index) : null" | |||
:class="{ sortable: sortable && header.dataType !== DATA_TYPE_OTHERS }" | |||
role="columnheader" | |||
aria-colindex="index + 1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I think :ariaColindex="index +1"
was intended
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accessible sortable table is a go!!! Thank you @BabyElias 🎉 🎉 🎉
QA Accessibility Testing Guidance:
Local sorted table
example (1st table), sorts the table & announces if sorted in ascending or descending orderBackend sorted table
example (2nd table), announces 'Data Loading, Please Wait' while the data is loading, then announces 'Data loaded successfully' when sorting completed. This will not announce the sorting order(asc/desc) as of now, since it's a trial loading state.