Skip to content

Conversation

@rurikoaraki
Copy link
Collaborator

Platforms Impacted

  • iOS
  • macOS
  • win32 (Office)
  • windows
  • android

Description of changes

Client reported some issues with the Button in HC.

It looks like we're using colors for plain text on the Button when we should be using ButtonText instead. This works for primary buttons but for other ones we're using the wrong color.

The reason this happens is because the alias tokens don't differentiate between usage in a canvas vs. usage in a button, so most alias tokens point to canvas/text instead of button/button text. Web fixes this by overriding the colors in HC specifically, so we'll do the same. Seems like a failure of the alias token system, but that's a separate conversation.

Changes:

  1. Add a utility function to check if we're in HC. This function operates differently depending on the platform. On mobile it no-ops, on macOS it retrieves a stored value, on windows it uses AppTheme and on win32 it reads a property on the Theme object.
  2. Replace current checks for high contrast with this new one
  3. Update colors on Button, Compound Button and ToggleButton

Verification

Before After
image image image image image image

Pull request checklist

This PR has considered (when applicable):

  • Automated Tests
  • Documentation and examples
  • Keyboard Accessibility
  • Voiceover
  • Internationalization and Right-to-left Layouts

@rurikoaraki rurikoaraki added Area: Button Issues related to the Fluent UI React Native Button component Area: Accessibility labels Mar 3, 2023
@rurikoaraki rurikoaraki requested a review from a team as a code owner March 3, 2023 00:14
@rurikoaraki rurikoaraki merged commit 6c33cfe into microsoft:main Mar 7, 2023
@rurikoaraki rurikoaraki deleted the hc_fixes_button branch March 7, 2023 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Accessibility Area: Button Issues related to the Fluent UI React Native Button component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants