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

Improve Aria Labels on DataView #772

Closed
14 of 15 tasks
MRichards99 opened this issue Jul 27, 2021 · 34 comments · Fixed by #813
Closed
14 of 15 tasks

Improve Aria Labels on DataView #772

MRichards99 opened this issue Jul 27, 2021 · 34 comments · Fixed by #813
Assignees
Labels
datagateway-dataview Issues corresponding to the dataview component. This component supersedes datagateway-table enhancement New feature or request good first issue Good for newcomers

Comments

@MRichards99
Copy link
Contributor

MRichards99 commented Jul 27, 2021

Description:
There's quite a few aria-label related improvements that could be made on DataView, so I'll collate them all into this issue. I'm guessing I'll spot things on Download and on Search so I'll create separate issues for those components when I get to them.

I won't include anything related to the view toggle switch, as that's being redesigned in #718 and I've spoken to @GoelBiju about having an aria label which is sensible for the final design of that.

Acceptance criteria:

  • Does whether the use of container in "container table search button" make sense to users? Decide and fix if not
  • Add gaps to table view information (check if we want to fix this or not)
  • Remove duplication on text and date filter components
  • Resolve unneeded gap caused by "..." on include example text
  • Make cog aria label more sensible
  • Fix aria labels outputting on screen readers on the first row of data (row 2 in code)
  • Remove duplication on pagination navigation element
  • Make screen reader output on max results drop down less confusing
  • Remove "button button" when tabbing through card view sort by columns. This could be automatically be fixed through the solution of Reduce Tab Presses on Card View Sorting Options #763
  • On date filters, add an appropriate aria label to make the calendar button functionality clear
  • The page view button has an aria label of 'page view button', which, when read by a screen reader, will say 'page view button button'. Remove the word 'button' in the aria label.
  • The dates are read as 'start date date filter from edit from... (yyyy-MM-dd)', which sounds jarring. The 'start date date filter from' section is the aria label followed by the placeholder text 'from... (yyyy-MM-dd)'. We can't find a way of not making the placeholder text be read aloud so this is hard to fix right now
  • doi is pronounced as doy
  • investigate inconsistencies with tab selecting from dropdown lists. Currently, it seems like tabbing to a dropdown requires you to press space to open the dropdown and then either space or enter to start interacting with it before requiring enter to be pressed to select an option
  • Focusing on data in the table using tabs sometimes allows for the screen reader to read out "grid grouping" or read the whole data in the table
@MRichards99 MRichards99 added enhancement New feature or request good first issue Good for newcomers datagateway-dataview Issues corresponding to the dataview component. This component supersedes datagateway-table labels Jul 27, 2021
@MRichards99
Copy link
Contributor Author

image

When focused on and using a screen reader, the search and cart buttons are outputted as 'container table search button' and 'container table cart button' (defined by aria labels). Does the word container make sense to users? As a developer, I understand container means either table or card view, but is that clear to a user?

@MRichards99
Copy link
Contributor Author

MRichards99 commented Jul 29, 2021

When tabbing around the page, the first focus on table view is when you get to the select all rows checkbox. When this happens, quite a bit of information is given to the user all at the same time, at quick pace:
"table with 50 rows and 10 columns row 1 select all rows column 1 clickable select all rows check box not checked"

This is useful information but needs to be separated up a bit. In the first instance, full stops should be used at the end of the aria labels to give a bit of gap between each piece of information. Another suggestion I found is to use aria-labelledby, so there's a defined relationship between different components on the page. I don't know enough about aria to know what effect that'll help/it'll improve this - https://stackoverflow.com/a/55273574/8752408

@MRichards99
Copy link
Contributor Author

Tabbing to a text filter component (screenshot below), the following text is read to the user: "Title Filter by Title Column 3 Title button"

image

There's a bit of duplication there with the filter label which could be reduced. The "Title button" could be made clearer - sort by title button perhaps?

@MRichards99
Copy link
Contributor Author

When focusing on the text field, a screen reader dictates: "Include edit blank"
image

Due to the '...' in the include, there's a delay between "include" and "edit blank" which could confuse some users.

@MRichards99
Copy link
Contributor Author

When focusing on the cog, the screen reader dictates "clickable menu button sub menu". This doesn't describe the functionality of it. Using dev tools, I can see that element has aria-labelledby="select-filter-type". Maybe changing this to a regular aria-label might fix this issue (select filter type adequately describes the functionality).

image

@MRichards99
Copy link
Contributor Author

For reference, the issues I've just raise also added in comments also impact date filters.

@MRichards99
Copy link
Contributor Author

When tabbing through the page, on the first row of data, none of the aria-label register with the screen reader i.e. I'm not being told anything and just sit here in silence. They work on the second row onwards (row 3 in code). I can shift tab back to the first data row and then those will work too. I'm not quite sure why this is, needs further investigation.

image

@MRichards99
Copy link
Contributor Author

On card view, tabbing to the pagination navigation nets you the following from the screen reader: "pagination navigation navigation ........". Not a major thing, but the duplicate navigation shouldn't happen.

image

@MRichards99
Copy link
Contributor Author

Focusing on the max results drop down gives the following from the screen reader - "clickable max results 10 menu button sub menu". This isn't the clearest - maybe dropping the "sub menu" might make more sense?

image

@MRichards99
Copy link
Contributor Author

When tabbing through the sort by columns, the second tab nets "button button". This shouldn't output on a screen reader, the first tab gives useful information. This is linked to and the cause of #763

image

@MRichards99
Copy link
Contributor Author

Note that DOI is pronounced "doy". That could just be my screen reader and probably not much that can be done on DataGateway's side, but something to note nonetheless.

@GoelBiju
Copy link
Contributor

GoelBiju commented Aug 9, 2021

When tabbing through the sort by columns, the second tab nets "button button". This shouldn't output on a screen reader, the first tab gives useful information. This is linked to and the cause of #763

image

This has been resolved by #795.

@GoelBiju
Copy link
Contributor

image

When focused on and using a screen reader, the search and cart buttons are outputted as 'container table search button' and 'container table cart button' (defined by aria labels). Does the word container make sense to users? As a developer, I understand container means either table or card view, but is that clear to a user?

I have changed the naming now to remove container from the beginning of these labels. Having container does not make sense to a user.

@MRichards99
Copy link
Contributor Author

Just want to link this issue to the original reason this was created (#738)

@MRichards99
Copy link
Contributor Author

I've just noticed something else with the date filters. This issue contains lots of work for the filters on table view so it seems appropriate to add this here rather than create a new issue.

When using a screen reader and focused on the button that opens the calendar view thing (not sure what it's proper name is), the screen reader just outputs "button". It looks like there's no aria label present, so one should be added to make the button's functionality clear
image

@sam-glendenning
Copy link
Contributor

Hi @AkhilMag13 I'm assigning you to this since Goel started but didn't finish it. This should give you something else to do for this sprint. Best to work through the checklist in the issue description and tick things off as you go. Let me know of any issues!

@AkhilMag13
Copy link
Contributor

When focusing on the text field, a screen reader dictates: "Include edit blank"
image

Due to the '...' in the include, there's a delay between "include" and "edit blank" which could confuse some users.

I have fixed this by removing the '...' from the include and now the screen reader does not have the delay. Sam and I have also tried to include an aria-label to fix this problem without removing the ellipses, but that has not worked out yet. Any additional solutions are welcome.

sam-glendenning pushed a commit that referenced this issue Aug 25, 2021
Instead of clickable menu button sub menu, it now says clickable select filter type menu button sub menu. This is a marginal improvement, could possibly be improved further later on
@sam-glendenning
Copy link
Contributor

sam-glendenning commented Aug 25, 2021

When focusing on the cog, the screen reader dictates "clickable menu button sub menu". This doesn't describe the functionality of it.

I've now changed this in ee69081 to say "clickable select filter type menu button sub menu". This is a marginal improvement, with the potential to be made better later on.

I had another go at this in 9b6dab0. It now says "clickable include or exclude menu button sub menu". It's hard, if not impossible, to take out the menu button sub menu part

sam-glendenning pushed a commit that referenced this issue Aug 25, 2021
This prevents the word "navigation" being repeated
@sam-glendenning
Copy link
Contributor

On card view, tabbing to the pagination navigation nets you the following from the screen reader: "pagination navigation navigation ........". Not a major thing, but the duplicate navigation shouldn't happen.

This is now fixed by 80758a2

sam-glendenning pushed a commit that referenced this issue Aug 25, 2021
…erly #772

This also changes their value from the dataKey of the object to the label of the object. This is because of a jarring effect that can occur when a field is obtained through a complex relationship, e.g. an instrument's investigation. So this would lead to the aria label being 'sort by instrument.investigation'. To make this consistent with what the user sees on the screen, it now says 'sort by ${label-value}'
@AkhilMag13
Copy link
Contributor

Focusing on the max results drop down gives the following from the screen reader - "clickable max results 10 menu button sub menu". This isn't the clearest - maybe dropping the "sub menu" might make more sense?

image

This is an ongoing problem with regards to NVDA as the screen reader is reading the component's name along with the labels provided. This resulted in another problem being found where the dropdown options are not accessible using keyboard shortcuts and the options are also being read as blank.

@AkhilMag13
Copy link
Contributor

The "Display as Table/Cards" button was being read as "Page-view button" on the screen reader, which offers very little description of the functionality of the button.
image
I have fixed this issue 6af5cda by modifying the aria label to contain the ternary statement, based on the text of the button. Now it is read as "Page-view Display as table button" and "Page-view Display as cards button" respectively.

@GoelBiju
Copy link
Contributor

GoelBiju commented Sep 1, 2021

The unit tests on all plugins also need updating due to changes to the date filter.

The unit tests have been fixed now and all that needs correcting are end-to-end tests. These can be addressed once PR #813 has been reviewed.

AkhilMag13 added a commit that referenced this issue Sep 2, 2021
This was to fix the text overlapping the datepicker icon.
@GoelBiju
Copy link
Contributor

GoelBiju commented Sep 2, 2021

A small item that we may want is the ability to focus on column headers that do not have sorting/filtering. An example of this is "Dataset Count" which at the moment is just an icon and text. You cannot currently tab to the column header and will just skip over it when you try to.

The only issue at the moment is how the tabbing should be configured, using tabIndex produces a variety of results with the NVDA screen reader which is sometimes accurate but most of the time not with duplicate information (as @sam-glendenning has mentioned).

sam-glendenning pushed a commit that referenced this issue Sep 2, 2021
Reads out the column name through screen reader but still having problem with table information being read on every tab to a new column
sam-glendenning pushed a commit that referenced this issue Sep 2, 2021
@sam-glendenning sam-glendenning self-assigned this Sep 3, 2021
@GoelBiju
Copy link
Contributor

GoelBiju commented Sep 3, 2021

A small item that we may want is the ability to focus on column headers that do not have sorting/filtering. An example of this is "Dataset Count" which at the moment is just an icon and text. You cannot currently tab to the column header and will just skip over it when you try to.

The only issue at the moment is how the tabbing should be configured, using tabIndex produces a variety of results with the NVDA screen reader which is sometimes accurate but most of the time not with duplicate information (as @sam-glendenning has mentioned).

I have created this as a separate issue (#823) in order to not hold this issue up.

AkhilMag13 added a commit that referenced this issue Sep 3, 2021
…772

To make it easier to uniquely identify the from and to datepickers in unit tests
sam-glendenning pushed a commit that referenced this issue Sep 3, 2021
These take into account the ids added onto date filters
sam-glendenning pushed a commit that referenced this issue Sep 3, 2021
This takes into account the id added to the date filters
sam-glendenning pushed a commit that referenced this issue Sep 3, 2021
GoelBiju added a commit that referenced this issue Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
datagateway-dataview Issues corresponding to the dataview component. This component supersedes datagateway-table enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants