Try new title separators focus style.

Yoast / yoast-components

Accessible React components by Yoast

GNU General Public License v3.0

21 stars 6 forks source link

Try new title separators focus style. #826

Closed afercia closed 5 years ago

afercia commented 5 years ago

Summary

This PR can be summarized in the following changelog entry:

Restore focus indication on the title separators user interface

This PR restores the focus style as done for the Settings page in https://github.com/Yoast/wordpress-seo/pull/12186. Also, updates the title separators "boxes" size to make them look as in the Settings page. Updates the standalone example to use the correct symbols.

Test instructions

Please follow test instructions on https://github.com/Yoast/wordpress-seo/pull/12186 If you want, you can also test the standalone example.

UI changes

[x] This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Fixes https://github.com/Yoast/wordpress-seo/issues/12144

Dieterrr commented 5 years ago

CR done, code looks good. Two questions though: @afercia What was the reason you implemented the dashed border? For me personally it leads to a confusing experience because it looks like something is broken. Is there a way to solve this differently visually? Also asking @hedgefield :) -The branch has merge conflicts.

afercia commented 5 years ago

I've opted for a dashed border because it was easy to implement and also allowed to simplify the CSS. I've tried a blue box-shadow (the WP standard focus style) but it really didn't look right and was almost unnoticeable. Glad to change it if there's a better design feedback! /Cc @hedgefield

afercia commented 5 years ago

Solved merge conflicts in the built CSS files.

hedgefield commented 5 years ago

@afercia Sure, I can see the default focus style isn't great. Try this one:

It adds a 2px blue box shadow with 0 blur. It's similar to the focus style on the Close Wizard button. Does that work for you?

You'll notice this one doesn't have rounded corners, could you remove those while you're at it? ;)

afercia commented 5 years ago

@hedgefield thanks, that's similar to the box-shadow and I'm not sure it's sufficient. Troublesome for persons with color perception impairments. The luminosity difference between purple and blue is very low. Yes, the perceived shape becomes thicker but it's not very noticeable.

hedgefield commented 5 years ago

Thickness alone is not enough I assume? Otherwise you could try to make the outline a little bigger, but probably this is an issue with our focus styles in other places too, so I'd recommend merging this with my proposed focus style improvements and then we take a look at focus styles globally after that.

afercia commented 5 years ago

OK, thanks! Done (also on https://github.com/Yoast/wordpress-seo/pull/12186).

abotteram commented 5 years ago

CR 👍