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

[BITV] 9.1.3.1e/8.1 - The user table represents a grid element and should be implemented as such. (1) #36921

Closed
2 tasks done
AndyScherzinger opened this issue Feb 28, 2023 · 15 comments · Fixed by #37870 or #39050
Closed
2 tasks done
Assignees
Labels
4. to release Ready to be released and/or waiting for tests to finish accessibility feature: users and groups
Milestone

Comments

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Feb 28, 2023

⚠️⚠️⚠️ Regression after table implementation: #38424

At the moment, the data is implemented in an unstructured manner using "div" elements. More information on grid implementation can be found at the following URLs: https://sarahmhigley.com/writing/grids-part1/, https://sarahmhigley.com/writing/grids-part2/ or https://www.w3.org/WAI/ARIA/apg/patterns/grid/.

If upstream additions can't be made

4b98244143224a248f230b7ff75c24c8

Details

https://report.bitvtest.de/default-en/d63601ac-cb34-4645-8256-66bec78964a0.html#checkpoint-3f0ff0bc0e-v8-n1

@michaelnissenbaum
Copy link

@JuliaKirschenheuter I would say we have a situation where the solution lies somewhere in the middle. But I would say we can also implement it as a normal table instead of a grid.

@AndyScherzinger AndyScherzinger changed the title [BITV] 9.1.3.1e/8.1 - The user table represents a grid element and should be implemented as such. At the moment, the data is implemented in an unstructured manner using "div" elements. More information on grid implementation can be found at the following URLs: https://sarahmhigley.com/writing/grids-part1/, https://sarahmhigley.com/writing/grids-part2/ or https://www.w3.org/WAI/ARIA/apg/patterns/grid/. (1) [BITV] 9.1.3.1e/8.1 - The user table represents a grid element and should be implemented as such. (1) Apr 21, 2023
@JuliaKirschenheuter JuliaKirschenheuter added 2. developing Work in progress and removed 1. to develop Accepted and waiting to be taken care of labels Apr 21, 2023
@AndyScherzinger AndyScherzinger added this to the Nextcloud 27 milestone Apr 24, 2023
@JuliaKirschenheuter JuliaKirschenheuter added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels May 12, 2023
@JuliaKirschenheuter JuliaKirschenheuter added 1. to develop Accepted and waiting to be taken care of and removed 4. to release Ready to be released and/or waiting for tests to finish labels May 25, 2023
@JuliaKirschenheuter
Copy link
Contributor

#38424

@JuliaKirschenheuter JuliaKirschenheuter removed their assignment May 25, 2023
@Pytal Pytal self-assigned this Jun 8, 2023
@Pytal Pytal added 2. developing Work in progress and removed 1. to develop Accepted and waiting to be taken care of labels Jun 9, 2023
@Pytal Pytal mentioned this issue Jun 29, 2023
5 tasks
@Pytal Pytal added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 6, 2023
@Pytal Pytal removed this from the Nextcloud 27.0.1 milestone Jul 10, 2023
@Pytal Pytal added this to the Nextcloud 28 milestone Jul 10, 2023
@JuliaKirschenheuter JuliaKirschenheuter self-assigned this Jul 12, 2023
@Pytal Pytal added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jul 13, 2023
@Pytal
Copy link
Member

Pytal commented Jul 17, 2023

Please verify the table semantics on https://try.nextcloud.com/ltd/a11y/index.php/settings/users @michaelnissenbaum and let us know if adjustments should be made :)

@Pytal Pytal linked a pull request Jul 18, 2023 that will close this issue
5 tasks
@michaelnissenbaum
Copy link

Hi @JuliaKirschenheuter. You're on the right track, BUT in the case of tables, it is important not to mix ARIA roles and HTML elements, which you have done here. Either the table should be implemented entirely with ARIA roles or solely with HTML elements, without mixing them. You can find a good description at https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/Table_Role.
CleanShot 2023-07-19 at 12 48 37@2x

@Pytal
Copy link
Member

Pytal commented Jul 20, 2023

The usage of both HTML elements and roles in the markup here is a technical limitation and did not seem optimal at first but given that browser engines interpret these semantics correctly i.e.

image

(from Firefox devtools)

and as a result provides the correct information to screenreaders, is this an acceptable solution @michaelnissenbaum?

@michaelnissenbaum
Copy link

Hi @JuliaKirschenheuter. Unfortunately, with such a mix of HTML elements and ARIA roles, it cannot be guaranteed that all screen readers will correctly render the table across all browsers. I would strongly advise against using such a mixed approach in this case.

@JuliaKirschenheuter
Copy link
Contributor

JuliaKirschenheuter commented Aug 11, 2023

Thanks a lot @michaelnissenbaum for this advice!
@michaelnissenbaum unfortunately we can't create a whole HTML table in that case because of technical limitations ;( Would it be ok if we would use only ARIA roles without HTML elements?

@Pytal
Copy link
Member

Pytal commented Aug 16, 2023

Ultimately it's best to use HTML elements instead of hacking vue-virtual-scroller so I'll do it the in-house way à la https://github.com/nextcloud/server/blob/feat/f2v/files/apps/files/src/components/VirtualList.vue

@Pytal
Copy link
Member

Pytal commented Aug 16, 2023

Any things we should take into account with VirtualList 👆 @skjnldsv? Shall we copy-n-adapt for users or abstract it into a reusable table component?

@skjnldsv
Copy link
Member

Well, I don't know about the latest a11y hacks, but we're fully using official semantic for a table.
I don't have much time to dive into why we need rowgroups or such, there is probably some more changes needed (like scope attributes), but we should indeed take the latest VirtualList.vue component in #39808 yes :)

https://a11y-101.com/development/tables

@AndyScherzinger
Copy link
Member Author

Thanks a lot @michaelnissenbaum for this advice!
@michaelnissenbaum unfortunately we can't create a whole HTML table in that case because of technical limitations ;( Would it be ok if we would use only ARIA roles without HTML elements?

@michaelnissenbaum any comment on this and subsequent comments?

@michaelnissenbaum
Copy link

From my perspective, there's nothing preventing us from implementing the table entirely with ARIA. You can find the description here: https://www.w3.org/WAI/ARIA/apg/patterns/table/examples/table/.

@Pytal
Copy link
Member

Pytal commented Sep 13, 2023

Let's see if the maintainer takes a look soon

If not, plan ARIA it is

@Pytal Pytal added 3. to review Waiting for reviews and removed 4. to release Ready to be released and/or waiting for tests to finish labels Sep 13, 2023
@skjnldsv
Copy link
Member

@AndyScherzinger
Copy link
Member Author

@Pytal can this item be considered done for the moment or are there any open items? If So can you please add them to the top issue description? Thanks!

@Pytal Pytal added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 11, 2023
@Pytal Pytal closed this as completed Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish accessibility feature: users and groups
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants