Button: Focus in high contrast mode fix

[lynamemi]

Pull request checklist

• Addresses an existing issue: Fixes Accessibility issue - buttons #3419
• Include a change request file using $ npm run change

Description of changes

Button focus wasn't working in high contrast mode. To fix this, I refactored getFocusStyle to take a highContrastStyle object that defines focus style in high contrast mode. CommandBarButton has a different style than the rest.

I debated about putting more logic inside getFocusStyle, but landed on passing a generic IRawStyle object for more flexibility.

In High Contrast White:

Focus areas to test

I tested that focus still works in non high contrast.

[betrue-final-final]

Much better.

[phkuo]

Is there a difference between this and just "IRawStyle?"?

[lynamemi]

Probably not different, but I am getting a tslint error if I don't have the extra bits when I rush rebuild:
missing-optional-annotation Argument following optional argument missing optional annotation: highContrastStyle: IRawStyle

[micahgodbolt]

I can see why it needs to be IRawStyle or undefined, but do you need to set the default to undefined? isn't that what it'll be if it's not set?

[lynamemi]

Alas, that's part of what triggers the error I noted above.

[lynamemi]

bump for this PR. we're getting more issues reported for this bug.

[micahgodbolt]

If ben is good with the high contrast, the I'm fine. If you can clean up that function just a bit great, but not stopping this PR.

[micahgodbolt]

I can see why it needs to be IRawStyle or undefined, but do you need to set the default to undefined? isn't that what it'll be if it's not set?

[lynamemi]

@phkuo Any more feedback?

[yaananth]

High contrast black theme the background of the selected button (which should be similar to focus) should be in "Cyan" and button text should be in "Black" colour.

In white theme background colour should be in "Purple" and button text colour should be in "White".

Are we doing that here? From description, it doesn't look like it.

.......(edited):
Is that not the case? Is there a standard somewhere for web? We can argue that focusing a button is in a way selecting the button, if it's just focus, border would suffice, but should we honor select styles as well? Do we have a definitive guidance here?

[lynamemi]

@yaananth, this PR is just for button focus in high contrast mode. I worked with our designer, @betrue-final-final. If it's another high contrast bug you're seeing, please file a separate issue.

[yaananth]

@lynamemi That's where the confusion is, how do we distinguish focus vs selection for a button in web world? Is there a defintitive guidance here?

consider VS:

focusing, adopts "select" styles.
If we follow that, then focus should render "Select" styles for buttons in web.

[betrue-final-final]

Focus is a border and there is no selected button state. It’s either rest or hover or pressed.

[yaananth]

Thanks @betrue-final-final as long as everyone across org is considering the same, sounds good!