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

fix(a11y): Files table roles #39385

Closed
wants to merge 2 commits into from
Closed

fix(a11y): Files table roles #39385

wants to merge 2 commits into from

Conversation

Pytal
Copy link
Member

@Pytal Pytal commented Jul 13, 2023

Summary

Before After
image image

Checklist

Pytal added 2 commits July 13, 2023 16:24
Signed-off-by: Christopher Ng <chrng8@gmail.com>
Signed-off-by: Christopher Ng <chrng8@gmail.com>
@Pytal Pytal added this to the Nextcloud 28 milestone Jul 13, 2023
@Pytal Pytal requested review from susnux, artonge and skjnldsv July 13, 2023 23:32
@Pytal Pytal self-assigned this Jul 13, 2023
@Pytal
Copy link
Member Author

Pytal commented Jul 13, 2023

/backport to stable27

Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To which ticket this PR belongs to?

@Pytal
Copy link
Member Author

Pytal commented Jul 20, 2023

To which ticket this PR belongs to?

Files2Vue a11y so would call this a followup to #36534

@JuliaKirschenheuter JuliaKirschenheuter self-requested a review July 24, 2023 09:08
@@ -23,7 +23,7 @@
<NcAppContent v-show="!currentView?.legacy"
:class="{'app-content--hidden': currentView?.legacy}"
data-cy-files-content>
<div class="files-list__header">
<div class="files-list__crumbs">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please no, more things than breadcrumbs are/will-be in here

@@ -143,8 +143,10 @@ export default Vue.extend({
mounted() {
// Make the root recycle scroller a table for proper semantics
const slots = this.$el.querySelectorAll('.vue-recycle-scroller__slot')
slots[0].setAttribute('role', 'thead')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the thead not needed anymore suddenly?
The first table tr row is exactly a header, no?

https://developer.mozilla.org/en-US/docs/Learn/HTML/Tables/Advanced#the_scope_attribute
image

@Pytal
Copy link
Member Author

Pytal commented Aug 16, 2023

Superseded by #39808

@Pytal Pytal closed this Aug 16, 2023
@Pytal Pytal deleted the fix/a11y-files-list branch August 16, 2023 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants