Skip to content
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

Merged
merged 3 commits into from
Jul 12, 2024
Merged

Conversation

BabyElias
Copy link
Contributor

QA Accessibility Testing Guidance:

  • Keyboard Navigation -
    • Arrow keys/Tab key
      • Able to move between cells using up/down/left/right arrow keys
      • When end of table reached at any point(using arrow keys), the navigation loops within the table (does not move to any other component)
      • Able to move between cells using Tab key. When end of table reached, pressing tab key shifts focus to the next component.
    • Pressing enter on the table header :
      • For Local sorted table example (1st table), sorts the table & announces if sorted in ascending or descending order
      • For Backend 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.
  • The cells are highlighted when navigating through them.
  • Data in each cell is identified with its corresponding column header and row by the screen reader.

@MisRob
Copy link
Member

MisRob commented Jun 21, 2024

@radinamatic
Copy link
Member

Hey @BabyElias, excellent progress! 😍

Smooth keyboard navigation is great experience 👏🏽, but there are a few hickups when it comes to what is NVDA enunciating:

  • there is a confusing not selected screen reader output inside each of the table cells, and despite a lot of trying to actually select something, I came out empty handed 😕 There must be something in the way the table is constructed that gives the NVDA the wrong idea each cell is something like a checkbox...

  • more concerning is the fact that the actual content of the cell is not enunciated, as you can see in the Speech Viewer of the NVDA in this recording:

    KTable.Component.Example.mp4

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):
Table preview.zip

KTable.Component.Example.-.vanilla.HTML.mp4

@BabyElias
Copy link
Contributor Author

BabyElias commented Jul 9, 2024

@radinamatic, not sure but my output for NVDA screen reader on the same screen shows something different.

  • There is no 'not selected' attribute here. I wonder what might be causing this discrepancy since we don't have any aria-selected attribute. Maybe, this can also happen because we are using ⬍ this symbol and this is what gets read as 'not-selected' internally.
  • Also, the content of the cell is getting enunciated in my preview.
Name (Local)   column header  sorted ascending  row 1  Name (Local)   column 1
Age (Local) ⬍  column header  row 1  Age (Local) ⬍  column 2
City (Local) ⬍  column header  row 1  City (Local) ⬍  column 3
Joined (Local) ⬍  column header  row 1  Joined (Local) ⬍  column 4
Misc (Local)  column header  row 1  Misc (Local)  column 5
Alice Johnson  row 2  Name (Local)   column 1
30 years old  row 2  Age (Local) ⬍  column 2
Houston  row 2  City (Local) ⬍  column 3
2020-07-18T00:00:00Z  row 2  Joined (Local) ⬍  column 4

@radinamatic
Copy link
Member

As @MisRob has suggested, the difference was in the browser:

  • in my testing with NVDA updated to 2024.02, (and reset to factory defaults), in Firefox updated to 128, the results were same as reported above.
    2024-07-10_02-16-39

  • when I switched to test with Chrome, I got the same results that you reported:

    2024-07-10_03-26-55

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... 🤔

@BabyElias
Copy link
Contributor Author

Ahh, I see. Tried it on my own system too and had this issue with firefox. Let me see how can this be fixed.

@BabyElias
Copy link
Contributor Author

Hey @radinamatic @MisRob !
Just to put the note here,
The problem is with the NVDA Screen Reader itself. Apparently, when tabindex is specified with table, it leads to this 'Not Selected' behaviour in firefox. There's a bug already raised for this in the NVDA Screen Reader repo also,( where I did have a discussion with the developers too) and they will be seeing what can be done.
P.S : Tabindex is necessary for us, since we need it for the keyboard navigation to work in anyway.
Attaching link to the discussion/issue here.

@MisRob
Copy link
Member

MisRob commented Jul 10, 2024

Thanks for looking into that @BabyElias. Yes, you're right that we need to work with tabindex. So I think here it'd be best to keep an eye on the linked issue and see if they can fix it. I can't think of anything we could do regarding this problem.

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?

@MisRob
Copy link
Member

MisRob commented Jul 10, 2024

And thanks for having conversation with NVDA team @BabyElias

@BabyElias
Copy link
Contributor Author

@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.

@MisRob
Copy link
Member

MisRob commented Jul 10, 2024

@BabyElias Oh I see, I didn't get it from the issue

@MisRob
Copy link
Member

MisRob commented Jul 10, 2024

@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)

@MisRob
Copy link
Member

MisRob commented Jul 10, 2024

@radinamatic Apart from documenting as suggested above, I don't think we can do anything else at this point

@BabyElias
Copy link
Contributor Author

Sure @MisRob. I'll add it in the documentation when making the next PR.

@MisRob
Copy link
Member

MisRob commented Jul 10, 2024

Perfect :)! Let's wait for @radinamatic's final confirmation and I think we could merge since this was the only blocker, I believe.

@radinamatic
Copy link
Member

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 not selected might not be a blocker, not having the cell content announced at all for anyone who uses the Firefox browser to navigate the table is a much bigger issue 😞

Do we have any other option of making the sorting column work without adding the tabindex attribute to the whole table? Since the table is not editable, strictly speaking, sighted keyboard users do not need to navigate cell-by-cell, they only need to sort and be able to scroll the table in view, (if necessary, when it is too wide for their screen). In essence, if the fancy easy navigation we've implemented with the plain Tab and arrow keys (nice addition, but not strictly required for sighted keyboard users), but in exchange is actually impeding all the screen reader users of one of the major browsers we support (the only group of users who actually MUST traverse each cell, and who already have the old, traditional way of doing it), to be able to make use of the table content, sounds like a no-go to me.

@BabyElias
Copy link
Contributor Author

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!

@MisRob
Copy link
Member

MisRob commented Jul 11, 2024

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.

@radinamatic
Copy link
Member

I thought that keyboard is a must, but now reading your comment, it makes sense.

That's totally my bad, I should have been much clearer regarding the requirement priorities for this component, I'm sorry!

@MisRob
Copy link
Member

MisRob commented Jul 11, 2024

Oh no no, no problem. It's a world of compromise :) Good discussion.

@BabyElias
Copy link
Contributor Author

BabyElias commented Jul 11, 2024

@radinamatic @MisRob
Made a few changes to the aria-attributes of the table, and have been able to achieve this screen reader output in firefox using NVDA. The keyboard navigation remains intact as before. With Chrome, the screen reader output is as before, no changes were observed. Even though this announces 'selected' (only in firefox, not chrome. Tried to remove that but it does not work any other way), but focuses on the cell content as well. Perhaps it can be tested in the playground page as well again.

City (Local)⬆️  column header  selected  row 1  column 3
sorted ascending
Joined (Local)⬍  column header  selected  row 1  column 4
Misc (Local)  column header  selected  row 1  column 5
Samuel Green  selected  row 2  Name (Local)⬍  column 1
22 years old  selected  row 2  Age (Local)⬍  column 2
Chicago  selected  row 2  City (Local)⬆️  column 3
2023-03-10T00:00:00Z  selected  row 2  Joined (Local)⬍  column 4
N/A  selected  row 2  Misc (Local)  column 5
Alice Johnson  selected  row 3  Name (Local)⬍  column 1
30 years old  selected  row 3  Age (Local)⬍  column 2
Houston  selected  row 3  City (Local)⬆️  column 3

@radinamatic
Copy link
Member

@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! 😍
While traversing the table with the default Ctrl+Alt+arrow, I do hear not selected, but as we agreed, that is not a blocker, and we hope it gets fixed by the NVDA community.

2024-07-12_05-59-22

Just in case you were unaware of this free a11y tool, axeCore dev tools does report some underlying issues, and you might want to go over them for your own skills building purpose. However, given that we've confirmed the good accessible user experience with the actual screen reader testing, I do not consider those blocker in our use case.

2024-07-12_06-03-35

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 🤷🏽

@BabyElias
Copy link
Contributor Author

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"
Copy link
Member

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

Copy link
Member

@radinamatic radinamatic left a 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 🎉 🎉 🎉 :shipit:

@MisRob MisRob self-requested a review July 12, 2024 12:29
@MisRob MisRob merged commit 11f0604 into learningequality:gsoc-table Jul 12, 2024
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants