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

Try/windows high contrast support #4833

Closed
wants to merge 6 commits into from

Conversation

jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Feb 2, 2018

Add support for Windows High Contrast Mode, by changing focus styles from using box-shadow to using borders and outlines. Fixes #4297.

Visually there shouldn't be any difference between this branch and master. But because borders take up space and change metrics, a lot of gymnastics had to be done so buttons kept their same footprint as before. As such, it is not unlikely I missed some buttons/styles in the process of doing this. As such, this PR needs a fair bit of testing.

Things to look for include buttons or elements that are suddenly smaller than before, or indeed bigger than before.

Some screenshots:

screen shot 2018-02-02 at 13 43 12

screen shot 2018-02-02 at 13 43 17

screen shot 2018-02-02 at 13 43 27

screen shot 2018-02-02 at 13 43 31

screen shot 2018-02-02 at 13 43 39

Joen Asmussen added 2 commits February 2, 2018 13:04
This should help Windows High Contrast users.
This should get the remaining tweaks in.
@jasmussen jasmussen added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Testing Needs further testing to be confirmed. labels Feb 2, 2018
@jasmussen jasmussen self-assigned this Feb 2, 2018
@jasmussen
Copy link
Contributor Author

Pushed another tweak to these, to make the focus style dotted in menu items:

menu focus

This starts to address parts of #4871.

@mtias
Copy link
Member

mtias commented Feb 5, 2018

I like the treatment within menus as it seems clearer it's not an active / off state.

@afercia
Copy link
Contributor

afercia commented Feb 5, 2018

@jasmussen thanks for trying this. I've just tested on Windows High Contrast mode with IE 11 and Edge, and unfortunately the transparent border is always visible, so everything appears "bordered" both in a normal and focused state with no apparent difference. See also the original comment by the Simply Accessible team on #1562 (comment)

screenshot 31

This changes from using a transparent border to shifting paddings. Construction zone.
@jasmussen jasmussen added the [Status] In Progress Tracking issues with work in progress label Feb 5, 2018
@jasmussen jasmussen removed the [Status] In Progress Tracking issues with work in progress label Feb 6, 2018
@jasmussen
Copy link
Contributor Author

Okay, I believe I handled the issue by using paddings instead of transparent borders. Not quite as clean, but let's start by testing if this solves the issue.

@jasmussen
Copy link
Contributor Author

@afercia any thoughts? I would appreciate if you could test this again for me, see if it addresses the issue. Thank you.

@afercia
Copy link
Contributor

afercia commented Feb 8, 2018

@jasmussen seems a good improvement 🙂 with some edge cases.

Initial state: borders are correctly not visible (there are no transparent borders now, right?):

screenshot 33

Focus on the top bar controls works nicely (the tooltip arrow is a bit weird though):

screenshot 34

On other Gutenberg UI controls, focus is sometimes barely visible. For example, the focused sidebar panel can be distinguished just because it gets a dotted border (I guess it's the default outline?):

screenshot 35

Instead, on other controls focus is still not visible, and that's the case of controls that inherit the focus stye from the core CSS, which uses only box-shadow. For example:

  • Publish button: focus not visible
  • Some "links" like visibility "Public" or the publish date: focus not visible

Last thing I've noticed: the "Move to trash" button "jumps" on focus.

@jasmussen
Copy link
Contributor Author

jasmussen commented Feb 19, 2018

Thank you all for the feedback, especially the screenshots, Andrea.

I was on vacation last week, and before that I didn't merge this in because I never felt comfortable with the hacks I had to perform to get this to work. I had hoped I could offset some of the complexity with SCSS math and mixins, but it just remains too complex. So I've thought about alternate approaches during this week, and came up with quite a few. In particular, this approach I think would work for us:

https://codepen.io/joen/pen/mXXbOR

The benefit to that approach is that it allows us to continue using box-shadow for the visual styling of the button. We then leverage the fact that even transparent outlines show up in high contrast mode, allowing us to not only create extra thick focus outlines for that use case, but it maps out an approach that lets us tailor the visual styles for high contrast modes without affecting the buttom markup, yet lets us address your subsequent points in a better way.

Here's stock focus:

36366436-6b1d08cc-154e-11e8-8756-d96e3cdbb787

Here's high contrast focus:

36366441-6f946954-154e-11e8-9230-943955ffb69a

This seems like the best approach because it allows us extra thick and very visually distinct focus outlines, like the rest of the system has in this mode, all the while working in both Edge and Firefox.

As such I'm going to take a new stab at this one. Thanks again for the help.

@jasmussen
Copy link
Contributor Author

Closing this in favor of #5138.

@jasmussen jasmussen closed this Feb 19, 2018
@jasmussen jasmussen deleted the try/windows-high-contrast-support branch February 19, 2018 09:47
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 Testing Needs further testing to be confirmed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants