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(files): move actions for selected file out from table header #41972

Merged
merged 3 commits into from
Jan 10, 2024

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Dec 1, 2023

Summary

Rendering actions button inside the table column header still counts as the table header. As the result:

  • actions buttons are used as column name which is not correct
  • other columns has no name

In this PR actions are moved out from the table and table header is always rendered.

TODO

  • FilesListTableHeaderActions is now a <div>, not a table header cell <th>
  • VirtualList has a new slot #header-overlay to display content with absolute position above the header
    • It has the same styles as a table row and full width
  • FilesListVirtual passes <FilesListTableHeaderActions> to the new #header-overlay slot
  • FilesListTableHeader doesn't render <FilesListTabelHeaderActions> and always render table header (
    • Table header should always be rendered, otherwise screen reader looses column names
    • When actions are shown, they overlays the header, and buttons are not visible
  • Remove styles override, unneeded after fix(NcButton): ellipsis text on small width nextcloud-libraries/nextcloud-vue#3936

Screen Reader NVDA

Column Before After
Name Add to favorite Move or copy Download Actions column2 clickable link Templates Name column 2 clickable link Templates
Size column 4 clickable 0 KB Size column 4 clickable 0 KB

Screenshots - no visual changes

Before After
image image
image image
image image

Checklist

@ShGKme ShGKme added this to the Nextcloud 29 milestone Dec 1, 2023
@ShGKme ShGKme self-assigned this Dec 1, 2023
@ShGKme ShGKme marked this pull request as draft December 1, 2023 14:41
@ShGKme ShGKme force-pushed the fix/41834/files--a11y-actions-on-selected branch from 448d533 to 55e6940 Compare January 8, 2024 15:10
@ShGKme ShGKme requested review from skjnldsv, susnux, JuliaKirschenheuter and Pytal and removed request for skjnldsv January 8, 2024 15:22
@ShGKme ShGKme added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 8, 2024
@ShGKme ShGKme marked this pull request as ready for review January 8, 2024 15:22
@ShGKme ShGKme force-pushed the fix/41834/files--a11y-actions-on-selected branch from 55e6940 to f30379e Compare January 8, 2024 15:29
@ShGKme ShGKme added the bug label Jan 8, 2024
@ShGKme ShGKme force-pushed the fix/41834/files--a11y-actions-on-selected branch from f30379e to 64924c1 Compare January 8, 2024 19:18
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

Great work!!

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jan 10, 2024
Having actions in the table header is no valid for a11y and counts as a column name.

Signed-off-by: Grigorii K. Shartsev <[email protected]>
Signed-off-by: Grigorii K. Shartsev <[email protected]>
@ShGKme ShGKme force-pushed the fix/41834/files--a11y-actions-on-selected branch from 64924c1 to 0f96a98 Compare January 10, 2024 16:57
@ShGKme
Copy link
Contributor Author

ShGKme commented Jan 10, 2024

Rebased onto master

@ShGKme ShGKme enabled auto-merge January 10, 2024 16:57
@ShGKme ShGKme merged commit 72786e9 into master Jan 10, 2024
41 checks passed
@ShGKme ShGKme deleted the fix/41834/files--a11y-actions-on-selected branch January 10, 2024 17:49
@ShGKme
Copy link
Contributor Author

ShGKme commented Jan 10, 2024

/backport b1a9017 to stable28

@JuliaKirschenheuter
Copy link
Contributor

JuliaKirschenheuter commented Jan 12, 2024

🥳🥳 great!

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 bug
Projects
None yet
5 participants