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

Augment keyboard accessibility of AppSidebar #2715

Merged
merged 13 commits into from
Jul 12, 2022
Merged

Conversation

Pytal
Copy link
Contributor

@Pytal Pytal commented Jun 1, 2022

To Do

  • Close button
  • Figure
  • Star
  • Edit title

@Pytal Pytal added enhancement New feature or request 2. developing Work in progress accessibility Making sure we design for the widest range of people possible, including those who have disabilities labels Jun 1, 2022
@Pytal Pytal added this to the 6.0.0 milestone Jun 1, 2022
@Pytal Pytal self-assigned this Jun 1, 2022
@Pytal Pytal force-pushed the enh/a11y-keyboard-appsidebar branch from 6475d3a to c5dd519 Compare June 2, 2022 20:44
@Pytal
Copy link
Contributor Author

Pytal commented Jun 2, 2022

@raimund-schluessler any idea on resolving the failing tests?

@raimund-schluessler
Copy link
Contributor

The visual regression tests fail, because the appearance of the AppSidebar changed. You need to download the snapshot artifacts from the cypress run, replace the snapshots in cypress/snapshots/base with the actual ones from the cypress run, check that you like the snapshots, i.e. your changes lead to the desired result, commit the new snapshots and see the tests pass.

However, there seems to be an issue with the tertiary_actions now, see the "Editable title after click with custom tertiary action" example in the docs, where the checkbox is squished now, because the tertiary__actions style is not applied anymore:
Screenshot 2022-06-02 at 23-18-14 Nextcloud Vue Style Guide

@Pytal Pytal added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 2, 2022
@Pytal Pytal requested review from raimund-schluessler, a team, mejo- and JuliaKirschenheuter and removed request for a team June 2, 2022 23:41
@Pytal Pytal marked this pull request as ready for review June 2, 2022 23:41
Copy link
Contributor

@raimund-schluessler raimund-schluessler left a comment

Choose a reason for hiding this comment

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

@Pytal could you please remove the PNG files that where not there before (the one with the numbers in the file name)?

@Pytal Pytal force-pushed the enh/a11y-keyboard-appsidebar branch 3 times, most recently from e9b841d to 2e8f919 Compare June 3, 2022 20:03
@Pytal Pytal requested review from artonge and CarlSchwan June 8, 2022 00:39
@raimund-schluessler
Copy link
Contributor

@Pytal The color or opacity of the star is off, which looks weird if it is overlayed with the folder icon, see e.g.

AppSidebar vue-subtitle_true-starred_true-compact_true-header_image-secondary_none-editable_true-base

and https://github.com/nextcloud/nextcloud-vue/pull/2715/files#diff-b888e1950c55db271730b2350fbe7aa975167f52b84930ee515b0f86387d81ee for the before/after comparison

Also, the height of the header now changes when you start the edit mode, making all content jump:
SidebarAfter

This was not the case before:
SidebarBefore

@Pytal
Copy link
Contributor Author

Pytal commented Jun 10, 2022

  • Regarding title and subtitle positioning and spacing: That should be fixed as it’s a regression – both are too far down and too far apart, ideally should be same as before as that was ideal in relation to the filetype icon.

The title may either be editable or not so to prevent layout shifts due to the 44px Button component when editing the minimum height for the title must be 44px, I'll see about tweaking the positioning

  • On the favoriting: This would probably be for a follow up pull request, but the actual accessibility fix would be what we did in the Mail app:

Will be done here for now and the new component may be added separately

@Pytal Pytal force-pushed the enh/a11y-keyboard-appsidebar branch 2 times, most recently from f8a12e2 to a323d57 Compare June 10, 2022 21:35
@Pytal
Copy link
Contributor Author

Pytal commented Jun 10, 2022

The star and star outline icons without color look like

Before After

Without the color the star would blend in with the image it overlays if they are similarly colored, @jancborchardt @nimishavijay design suggestions?

@Pytal
Copy link
Contributor Author

Pytal commented Jun 27, 2022

@jancborchardt how should we keep the star visible with the new grayscale filled and outlined star icons seen in the screenshots above?

@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jun 28, 2022
@skjnldsv skjnldsv removed their request for review June 28, 2022 09:53
@Pytal Pytal force-pushed the enh/a11y-keyboard-appsidebar branch from a323d57 to 2d80062 Compare July 12, 2022 19:29
Pytal added 11 commits July 12, 2022 19:33
Signed-off-by: Christopher Ng <chrng8@gmail.com>
Signed-off-by: Christopher Ng <chrng8@gmail.com>
Signed-off-by: Christopher Ng <chrng8@gmail.com>
Signed-off-by: Christopher Ng <chrng8@gmail.com>
Signed-off-by: Christopher Ng <chrng8@gmail.com>
Signed-off-by: Christopher Ng <chrng8@gmail.com>
Signed-off-by: Christopher Ng <chrng8@gmail.com>
Signed-off-by: Christopher Ng <chrng8@gmail.com>
Signed-off-by: Christopher Ng <chrng8@gmail.com>
Signed-off-by: Christopher Ng <chrng8@gmail.com>
Signed-off-by: Christopher Ng <chrng8@gmail.com>
@Pytal Pytal force-pushed the enh/a11y-keyboard-appsidebar branch from 2d80062 to 67344b4 Compare July 12, 2022 19:33
Pytal added 2 commits July 12, 2022 19:38
Signed-off-by: Christopher Ng <chrng8@gmail.com>
Signed-off-by: Christopher Ng <chrng8@gmail.com>
@Pytal Pytal force-pushed the enh/a11y-keyboard-appsidebar branch from 67344b4 to c4dfe89 Compare July 12, 2022 19:40
@Pytal
Copy link
Contributor Author

Pytal commented Jul 12, 2022

Proceeding as per @jancborchardt's decision in https://cloud.nextcloud.com/call/gqff69i8 to use a background for the star icons

Before After
image image
image image

@Pytal Pytal added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Jul 12, 2022
@Pytal Pytal merged commit 8648ba3 into master Jul 12, 2022
@Pytal Pytal deleted the enh/a11y-keyboard-appsidebar branch July 12, 2022 19:54
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 Making sure we design for the widest range of people possible, including those who have disabilities design Design, UX, interface and interaction design enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants