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.

Sign up for GitHub

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

Jump to bottom

Button accessibility improvements #2899

Merged
merged 6 commits into from
Sep 6, 2022
Merged

Button accessibility improvements #2899

merged 6 commits into from
Sep 6, 2022

Conversation

marcoambrosini
Copy link
Contributor

@marcoambrosini marcoambrosini commented Jul 28, 2022 •
edited
Loading

Bunch of simplification and accessibility fixes for the Button component.

  • Removed unnecessary JS logic for keyboard navigation (apparently adding !important always activates the focus-visible when navigating with the keyboard)
  • Removed the lightening of backgrounds as focus feedback. There's already a huge outline, so best avoid reducing contrast
  • Removed opacity change for tertyary-no-background buttons. These are buttons whose use we should limit for very specific use-case such as within an input field. The only hover feedback will be the cursor change to pointer, while for keyboard navigation we have the beefy outline here too.
Screen.Recording.2022-07-28.at.12.26.12.mov

@marcoambrosini marcoambrosini added 3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities labels Jul 28, 2022
@marcoambrosini marcoambrosini self-assigned this Jul 28, 2022
@marcoambrosini marcoambrosini force-pushed the button-accessibility-improvements branch from 8ccf0c1 to cfc9a66 Compare July 28, 2022 10:24
@marcoambrosini marcoambrosini force-pushed the button-accessibility-improvements branch from cfc9a66 to d0fabaa Compare July 29, 2022 05:56
JuliaKirschenheuter
JuliaKirschenheuter approved these changes Jul 29, 2022
View reviewed changes
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.

tested on mail - works! Thank you!

@nickvergessen
Copy link
Contributor

nickvergessen commented Jul 29, 2022 •
edited
Loading

Error: The "AppSidebar.vue-subtitle_null-starred_false-compact_true-header_none-secondary_none-editable_true" image is different. Threshold limit exceeded!

Lots of issues on CI

@marcoambrosini marcoambrosini force-pushed the button-accessibility-improvements branch 2 times, most recently from 04b28bf to c274f01 Compare August 4, 2022 06:51
@nickvergessen
Copy link
Contributor

Removed unnecessary JS logic for keyboard navigation (apparently adding !important always activates the focus-visible when navigating with the keyboard)

Should be fixed now that the server doesn't simply touch everything anymore:
nextcloud/server#33497

@marcoambrosini marcoambrosini force-pushed the button-accessibility-improvements branch from c274f01 to e44571b Compare August 23, 2022 08:44
@marcoambrosini
Copy link
Contributor Author

Should be fixed now that the server doesn't simply touch everything anymore:

Still, all that is not needed anymore.

@marcoambrosini marcoambrosini force-pushed the button-accessibility-improvements branch from e44571b to 3265136 Compare August 24, 2022 16:59
@marcoambrosini
Copy link
Contributor Author

Let's get this in?

nickvergessen
nickvergessen reviewed Aug 26, 2022
View reviewed changes
webpack.dev.js Outdated Show resolved Hide resolved
skjnldsv
skjnldsv approved these changes Sep 6, 2022
View reviewed changes
Copy link
Contributor

@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.

Isn't the black border a tad too strong? :)
Otherwise fine by me

@skjnldsv
Copy link
Contributor

skjnldsv commented Sep 6, 2022

Rebasing...

@skjnldsv skjnldsv force-pushed the button-accessibility-improvements branch from 26e62bf to 2e0c711 Compare September 6, 2022 12:34
@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 Sep 6, 2022
@marcoambrosini @skjnldsv
Remove webpack dev config file
36e82ed 
Signed-off-by: Marco Ambrosini <[email protected]>
@skjnldsv skjnldsv force-pushed the button-accessibility-improvements branch from 2e0c711 to 36e82ed Compare September 6, 2022 12:46
@skjnldsv skjnldsv enabled auto-merge September 6, 2022 12:46
@skjnldsv skjnldsv merged commit 3253d19 into master Sep 6, 2022
@skjnldsv skjnldsv deleted the button-accessibility-improvements branch September 6, 2022 12:51
@nfebe nfebe mentioned this pull request Jun 5, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nickvergessen nickvergessen nickvergessen left review comments

@JuliaKirschenheuter JuliaKirschenheuter JuliaKirschenheuter approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

@PVince81 PVince81 Awaiting requested review from PVince81

@julien-nc julien-nc Awaiting requested review from julien-nc

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan

@danxuliu danxuliu Awaiting requested review from danxuliu

@raimund-schluessler raimund-schluessler Awaiting requested review from raimund-schluessler

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

Assignees

@marcoambrosini marcoambrosini

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
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@marcoambrosini @nickvergessen @skjnldsv @juliusknorr @JuliaKirschenheuter