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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DynamicColorMacOS] Downgrade to warning if appearance is nil #1885

Merged
merged 2 commits into from
Jul 21, 2023

Conversation

Saadnajmi
Copy link
Collaborator

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 馃憤
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 馃憤
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

When a user cuts/copies text from a <TextInput> that has it's text color styled using DynamicColorMacOS, we would get a Redbox saying that the current appearance could not be resolved. Looking into it, it seems that at this point in native code, Appkit returns an appearance of NSAppearanceNameSystem, which seems to correspond to the "Any" case of an asset catalog if you were specifying a color that way instead.

While I'm not sure why cutting/copying text gets us in this state, it seems this is a valid state to be in, where the OS doesn't have the current appearance and just reports back a generic one. Let's fall back to the light color (as you would with an asset catalog) with a warning instead of an error.

Changelog

[MACOS] [CHANGED] - Downgrade DynamicColorMacOS error to warning if appearance is nil

Test Plan

Testing in RNTester, we get a yellow box instead of a redbox.

@Saadnajmi Saadnajmi enabled auto-merge (squash) July 21, 2023 02:18
@Saadnajmi Saadnajmi merged commit 7ca6479 into microsoft:main Jul 21, 2023
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants