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

Restore the block hover and focus styles in Unified Toolbar mode. #11737

Merged
merged 2 commits into from
Nov 19, 2018

Conversation

afercia
Copy link
Contributor

@afercia afercia commented Nov 11, 2018

Description

The Unified Toolbar mode should not remove outlines. This was discussed and agreed by the accessibility team and it's our recommendation as a team.

For example, I may find Unified Toolbar mode useful for my workflow but, as a user with vision impairments, still want a clear indication of hover and focus on the blocks. Instead, I can't use Unified Toolbar because it removes focus indication.

Quoting from the related issue #10559

Regardless of the accessibility issues with focus, I think that these controls should only change the specific items they describe, and not also impact other behaviors - that's confusing, and makes it more difficult to configure the design the way you want.

This PR is an effort to propose a positive action. It partially reverts #9394. An option for a lighter UI when writing is already available with "Spotlight Mode" and as a team we feel that's the proper place for UI modifications that are clearly extraneous to a "Unified Toolbar" option.

Fixes #10559

@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Nov 11, 2018
@afercia afercia requested a review from tofumatt November 11, 2018 17:27
@youknowriad youknowriad added the Needs Design Feedback Needs general design feedback. label Nov 12, 2018
@tofumatt
Copy link
Member

Could you add some screenshots to this pull request to make it clearer what the before/after of these changes should look like?

@afercia
Copy link
Contributor Author

afercia commented Nov 12, 2018

Could you add some screenshots to this pull request to make it clearer what the before/after of these changes should look like?

It just restores the default styles used in the default mode. Anyways, here's the screenshots:

focus before (caret is in the second block):

focus before

focus after:

focus after

hover before:

hover before

hover after:

hover after

2 considerations:

Copy link
Member

@tofumatt tofumatt left a comment

Choose a reason for hiding this comment

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

On hover there's a more visible blue border. I wonder why the hover style is more prominent than the focus style.

This is intentional because the really intense hover style on focus is a bit distracting.

I am for this change because it makes things consistent, and something like spotlight mode should be what does away with/controls these hover/focus states, not the toolbar positioning.

I also see the benefit in some kind of higher-contrast option that enables more intense focus colours, but I think that should be separate.

cc'ing some others for their feedback because while I like this, I don't think I have final design say 😉

@afercia
Copy link
Contributor Author

afercia commented Nov 13, 2018

Agree to wait a bit for final feedback.

This is intentional because the really intense hover style on focus is a bit distracting.

I'd like to remind everyone that for non-text contrast (user interface components) the contrast requirement is at least 3:1.

@mtias mtias force-pushed the update/unified-toolbar-restore-blocks-hover-focus-style branch from e0151da to ae49092 Compare November 19, 2018 18:47
@mtias
Copy link
Member

mtias commented Nov 19, 2018

Pushed a change with the suggestion in #10559. I think the selected outline contrast could be increased slightly.

@mtias mtias added this to the 4.5 milestone Nov 19, 2018
Copy link
Member

@karmatosed karmatosed left a comment

Choose a reason for hiding this comment

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

Design wise approving.

@antpb
Copy link
Contributor

antpb commented Nov 19, 2018

This looks good in my testing. With the final design approval we are good to go in merging this. Woohoo Thanks all!

Fixes #10559

@antpb antpb merged commit 9297fc9 into master Nov 19, 2018
@mtias mtias deleted the update/unified-toolbar-restore-blocks-hover-focus-style branch November 19, 2018 21:27
@mtias
Copy link
Member

mtias commented Nov 19, 2018

Let's also continue the conversation around hover states across the two modes separately. This improves the selection state which seemed the most important. We could also look at increasing the contrast of the selected state too.

@afercia
Copy link
Contributor Author

afercia commented Nov 19, 2018

Thanks. @mtias on the related issue, some new contributors at WordCamp Milano asked you some feedback. It would be nice from you to reply to encourage participation of new contributors, as this is one of the main goals of an open source project. Thank you.

@afercia
Copy link
Contributor Author

afercia commented Nov 19, 2018

Also, seems the latest commit didn't update a few snapshots? The build failed /Cc @mtias @antpb .

@mtias
Copy link
Member

mtias commented Nov 19, 2018

@afercia I missed that ping, thanks.

@Usamaliaquat123
Copy link

@karmatosed nice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants