Skip to content

Use trait collection to resolve border colors - #32492

Closed
danilobuerger wants to merge 1 commit into
react:mainfrom
danilobuerger:patch-4
Closed

Use trait collection to resolve border colors#32492
danilobuerger wants to merge 1 commit into
react:mainfrom
danilobuerger:patch-4

Conversation

@danilobuerger

@danilobuergerdanilobuerger commented Oct 28, 2021

Copy link
Copy Markdown
Contributor

Summary

c974cbf changed the border colors to be of UIColor instead of CGColor. This allowed working with dark mode to switch the border colors automatically. However, in certain situation the system can't resolve the current trait collection (see https://stackoverflow.com/a/57177411/2525941). This commit resolves the colors with the current trait collection to ensure the right colors are selected. This matches with the behavior of how the background color is resolved (also in displayLayer:).

Changelog

[iOS] [Fixed] - Resolve border platform color based on current trait collection

Test Plan

Same test plan as #29728

@facebook-github-botfacebook-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 Oct 28, 2021
@facebook-github-botfacebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Oct 28, 2021
@pull-bot

Copy link
Copy Markdown

PR build artifact for 38d5f6b94114e9e9e0a58943d71de7f64c6cd335 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@analysis-bot

analysis-bot commented Oct 28, 2021

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
ios-universaln/a--

Base commit: f7a66e6
Branch: main

@analysis-bot

analysis-bot commented Oct 28, 2021

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a8,297,788-2,614
androidhermesarmeabi-v7a7,634,879-4,063
androidhermesx868,771,104-4,744
androidhermesx86_648,708,122-4,635
androidjscarm64-v8a9,783,593-1,242
androidjscarmeabi-v7a8,768,874-1,847
androidjscx869,749,504-1,956
androidjscx86_6410,345,405-2,222

Base commit: 384e1a0
Branch: main

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@cortinicocortinico left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For some reason CLANGFORMAT is flagging those lines:712, 730, 733

I'd say let's rebase this and verify if Circle CI is green. If is still green after the rebase and the internal job I can reformat it manually.

Also cc @sammy-SC for a iOS pass

react@c974cbf changed the border colors to be of UIColor instead of CGColor. This allowed working with dark mode to switch the border colors automatically. However, in certain situation the system can't resolve the current trait collection (see https://stackoverflow.com/a/57177411/2525941). This commit resolves the colors with the current trait collection to ensure the right colors are selected. This matches with the behavior of how the background color is resolved (also in displayLayer:).
@danilobuerger

Copy link
Copy Markdown
ContributorAuthor

Hi @cortinico thanks for the review. I rebased it and amended the lines you mentioned. It looks like there were some whitespaces that the linter didn't agree with.

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@react-native-bot

Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @danilobuerger in 9a35818.

When will my fix make it into a release? | Upcoming Releases

@react-native-botreact-native-bot added the Merged This PR has been merged. label Feb 7, 2022
@danilobuerger

Copy link
Copy Markdown
ContributorAuthor

@cortinico Thank you for taking care of my outstanding PRs. I really do appreciate it.

@danilobuerger
danilobuerger deleted the patch-4 branch February 7, 2022 18:41
@cortinico

Copy link
Copy Markdown
Contributor

@cortinico Thank you for taking care of my outstanding PRs. I really do appreciate it.

Sorry those fell through the cracks 👍 Glad we got to merge them though!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BugCLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.MergedThis PR has been merged.Platform: iOSiOS applications.Shared with MetaApplied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@danilobuerger@pull-bot@analysis-bot@facebook-github-bot@react-native-bot@cortinico