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

Accessible colors for DynamicColorIOS #31651

Closed
wants to merge 9 commits into from
Closed

Conversation

birkir
Copy link
Contributor

@birkir birkir commented Jun 3, 2021

Summary

Allow you to harvest the UIAccessibilityContrastHigh trait from iOS to show accessible colors when high contrast mode is enabled.

// usage

PlatformColorIOS({
  light: '#eeeeee',
  dark: '#333333',
  highContrastLight: '#ffffff',
  highContrastDark: '#000000',
});

// {
//   "dynamic": {
//     "light": "#eeeeee",
//     "dark": "#333333",
//     "highContrastLight": "#ffffff",
//     "highContrastDark": "#000000",
//   }
// }

This is how apple's own dynamic system colors work under the hood (https://developer.apple.com/design/human-interface-guidelines/ios/visual-design/color/#dynamic-system-colors)


The react native docs mention that more keys may become available in the future, which this PR is adding:

In the future, more keys might become available for different user preferences, like high contrast.

https://reactnative.dev/docs/dynamiccolorios

Changelog

[iOS] [Added] - High contrast dynamic color options for dark and light mode.

Test Plan

Added unit tests for normalizeColor to pass the high contrast colors downstream to RCTConvert

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 3, 2021
Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

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

Code analysis results:

  • eslint found some issues. Run yarn lint --fix to automatically fix problems.

Libraries/StyleSheet/__tests__/normalizeColor-test.js Outdated Show resolved Hide resolved
Libraries/StyleSheet/__tests__/normalizeColor-test.js Outdated Show resolved Hide resolved
@birkir
Copy link
Contributor Author

birkir commented Jun 3, 2021

Checks: Unrelated issue with test_windows test.

@analysis-bot
Copy link

analysis-bot commented Jun 3, 2021

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 9,242,738 +0
android hermes armeabi-v7a 8,752,081 +0
android hermes x86 9,704,750 +0
android hermes x86_64 9,670,048 +0
android jsc arm64-v8a 10,889,726 +0
android jsc armeabi-v7a 9,790,351 +0
android jsc x86 10,947,138 +0
android jsc x86_64 11,553,949 +0

Base commit: 88fe26d

@analysis-bot
Copy link

analysis-bot commented Jun 3, 2021

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 88fe26d

@p-sun
Copy link
Contributor

p-sun commented Jun 5, 2021

Looks good! Would you mind renaming it to highContrastDark and highContrastLight since it's more specific and clearer than accessible? Android and Windows also use the words high contrast.

@birkir
Copy link
Contributor Author

birkir commented Jun 5, 2021

Would you mind renaming it to highContrastDark and highContrastLight since it's more specific and clearer than accessible? Android and Windows also use the words high contrast.

Current naming comes from Apple's UIColor table in the iOS Human Guidelines.

The UIColor has an "dark" and "light" options, and "accessible" variant for each of those, but they react automatically to High Contrast and Invert Colors traits.

I think overall it's a good suggestion to scope keys by traits. Especially if the API will reach other platforms as well.

Only thing is that colour schemes won't be scoped (the dark and light keys).

But for now, and If no one disagrees, I'll go ahead and scope the accessible keys to highContrast!

Thanks for the feedback @p-sun

@p-sun
Copy link
Contributor

p-sun commented Jun 5, 2021

What does "colour schemes won't be scoped (the dark and light keys)" mean?

@birkir
Copy link
Contributor Author

birkir commented Jun 5, 2021

What does "colour schemes won't be scoped (the dark and light keys)" mean?

Traits are highly dynamic and can offer a matrix of different values for different states. For example, we can have a scenario where you are in a dark mode, with high contrast enabled, inverted colors, and a P3 display gamut.

Enabling all these keys in the current setup, won't allow you to match more than two of those scenarios, a color scheme, and only one trait.

{
  dark: 'red', // not scoped with "colorScheme" as "colorSchemeDark"
  highContrastDark: 'hotpink',
  invertedDark: 'blue',
  gamutP3Dark: '#ff0001'
}

Where what you would probably like to do is something like

[UIColor colorWithDynamicProvider:^UIColor *_Nonnull(UITraitCollection *_Nonnull collection) {
  if (trait.legibilityWeight == UILegibilityWeightBold) {
    return [UIColor whiteColor];
  }

  if (trait.userInterfaceStyle == UIUserInterfaceStyleDark) {
    if (trait.accessibilityContrast == UIAccessibilityContrastHigh) {
      return [UIColor blueColor];
    }
    return [UIColor redColor];
  }
}]

I just didn't think it was feasible to try to implement this and went with the simplified version that Apple offers, "accessible".

(this is just an example, inverted colors isn't a trait as of now)

@p-sun
Copy link
Contributor

p-sun commented Jun 5, 2021

I see! Naming it high contrast makes sense then since that's the only trait we're making adaptable here. People probably don't need more traits than that to change colors. Would you mind updating the docs too?

Android only has the "High Contrast Text" switch in the settings -- I believe this only changes text colors. So Android is only using the high contrast trait to adapt colors for accessibility.

@birkir
Copy link
Contributor Author

birkir commented Jun 5, 2021

@p-sun I updated the docs in a new pull request on react-native-website and I did the rename of /accessible/highContrast/for the keys.

https://deploy-preview-2640--react-native.netlify.app/docs/next/dynamiccolorios

The PR is ready for merge on my end ✅

@facebook-github-bot
Copy link
Contributor

@p-sun has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@p-sun merged this pull request in 4b9d9dd.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Jun 8, 2021
kelset pushed a commit that referenced this pull request Jun 16, 2021
Summary:
Allow you to harvest the `UIAccessibilityContrastHigh` trait from iOS to show accessible colors when high contrast mode is enabled.

```jsx
// usage

PlatformColorIOS({
  light: '#eeeeee',
  dark: '#333333',
  highContrastLight: '#ffffff',
  highContrastDark: '#000000',
});

// {
//   "dynamic": {
//     "light": "#eeeeee",
//     "dark": "#333333",
//     "highContrastLight": "#ffffff",
//     "highContrastDark": "#000000",
//   }
// }
```

This is how apple's own dynamic system colors work under the hood (https://developer.apple.com/design/human-interface-guidelines/ios/visual-design/color/#dynamic-system-colors)

 ---

The react native docs mention that more keys may become available in the future, which this PR is adding:

> In the future, more keys might become available for different user preferences, like high contrast.

https://reactnative.dev/docs/dynamiccolorios

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[iOS] [Added] - High contrast dynamic color options for dark and light mode.

Pull Request resolved: #31651

Test Plan: Added unit tests for `normalizeColor` to pass the high contrast colors downstream to RCTConvert

Reviewed By: lunaleaps

Differential Revision: D28922536

Pulled By: p-sun

fbshipit-source-id: f81417f003c3adefac50e994e62b9be14ffa91a1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Needs: React Native Team Attention
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants