Skip to content

[Android] Update ColorPropConverterto support color function values - #43031

Closed
ryanlntn wants to merge 1 commit into
react:mainfrom
ryanlntn:feat/wide-gamut-color-prop-converter
Closed

[Android] Update ColorPropConverterto support color function values#43031
ryanlntn wants to merge 1 commit into
react:mainfrom
ryanlntn:feat/wide-gamut-color-prop-converter

Conversation

@ryanlntn

Copy link
Copy Markdown
Contributor

Summary:

This adds support for color function values to ColorPropConverter per the wide gamut color RFC. It updates the color conversion code so that it returns a Color instance before ultimately being converted to an Integer in preparation for returning long values as needed.

Changelog:

[ANDROID] [ADDED] - Update ColorPropConverter to support color function values

Test Plan:

Colors should work exactly the same as before.

Follow test steps from #42831 to test support for color() function syntax.

While colors specified with color() function syntax will not yet render in DisplayP3 color space they will not be misrecognized as resource path colors but will instead fallback to their sRGB color space values.

@facebook-github-botfacebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Infinite Red Partner: Infinite Red Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Feb 14, 2024
@ryanlntn
ryanlntnforce-pushed the feat/wide-gamut-color-prop-converter branch from ec5415e to ef1add6CompareFebruary 14, 2024 20:28
@analysis-bot

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a17,828,324+3
androidhermesarmeabi-v7an/a--
androidhermesx86n/a--
androidhermesx86_64n/a--
androidjscarm64-v8a21,186,792+0
androidjscarmeabi-v7an/a--
androidjscx86n/a--
androidjscx86_64n/a--

Base commit: 7b00a92
Branch: main

@ryanlntn
ryanlntnforce-pushed the feat/wide-gamut-color-prop-converter branch from e486395 to 53300f7CompareMarch 15, 2024 10:33
@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

@facebook-github-botfacebook-github-bot added the Merged This PR has been merged. label Apr 23, 2024
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@cipolleschi merged this pull request in fa7dbd5.

@github-actions

Copy link
Copy Markdown

This pull request was successfully merged by @ryanlntn in fa7dbd5.

When will my fix make it into a release? | How to file a pick request?

facebook-github-bot pushed a commit that referenced this pull request Apr 23, 2024
…44225)
Summary:
Pull Request resolved: #44225
Back out [#43031](#43031) as it makes some internal test fails.
## Changelog:
[Android][Fixed] - Revert #43031
Reviewed By: arushikesarwani94
Differential Revision: D56489260
fbshipit-source-id: bd843e6eb60fce0a1d5d8da9f3c5d2a36473ecfb
kosmydel pushed a commit to kosmydel/react-native that referenced this pull request May 6, 2024
Summary:
This adds support for color function values to ColorPropConverter per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). It updates the color conversion code so that it returns a Color instance before ultimately being converted to an Integer in preparation for returning long values as needed.
bypass-github-export-checks
## Changelog:
[ANDROID] [ADDED] - Update ColorPropConverter to support color function values
Pull Request resolved: react#43031
Test Plan:
Colors should work exactly the same as before.
Follow test steps from react#42831 to test support for color() function syntax.
While colors specified with color() function syntax will not yet render in DisplayP3 color space they will not be misrecognized as resource path colors but will instead fallback to their sRGB color space values.
Reviewed By: cortinico
Differential Revision: D55749058
Pulled By: cipolleschi
fbshipit-source-id: 37659d22c1db4b1a27a9a4f88c9beb703517b01f
kosmydel pushed a commit to kosmydel/react-native that referenced this pull request May 6, 2024
…eact#44225)
Summary:
Pull Request resolved: react#44225
Back out [react#43031](react#43031) as it makes some internal test fails.
## Changelog:
[Android][Fixed] - Revert react#43031
Reviewed By: arushikesarwani94
Differential Revision: D56489260
fbshipit-source-id: bd843e6eb60fce0a1d5d8da9f3c5d2a36473ecfb
facebook-github-bot pushed a commit that referenced this pull request May 20, 2024
Summary:
Pull Request resolved: #44237
This adds support for color function values to ColorPropConverter per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). It updates the color conversion code so that it returns a Color instance before ultimately being converted to an Integer in preparation for returning long values as needed.
## Changelog:
[ANDROID] [ADDED] - Update ColorPropConverter to support color function values
Pull Request resolved: #43031
Test Plan:
Colors should work exactly the same as before.
Follow test steps from #42831 to test support for color() function syntax.
While colors specified with color() function syntax will not yet render in DisplayP3 color space they will not be misrecognized as resource path colors but will instead fallback to their sRGB color space values.
---
After the failure with the tests, I reapplied the changes and test some Jest e2e tests that were failing yesterday:
{F1495277376}
Reviewed By: cortinico
Differential Revision: D56517579
Pulled By: cipolleschi
fbshipit-source-id: ae9b5bc2afe9eb9760dd91afb090385daf7102b8
kosmydel pushed a commit to kosmydel/react-native that referenced this pull request Jun 11, 2024
Summary:
This adds support for color function values to ColorPropConverter per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). It updates the color conversion code so that it returns a Color instance before ultimately being converted to an Integer in preparation for returning long values as needed.
bypass-github-export-checks
## Changelog:
[ANDROID] [ADDED] - Update ColorPropConverter to support color function values
Pull Request resolved: react#43031
Test Plan:
Colors should work exactly the same as before.
Follow test steps from react#42831 to test support for color() function syntax.
While colors specified with color() function syntax will not yet render in DisplayP3 color space they will not be misrecognized as resource path colors but will instead fallback to their sRGB color space values.
Reviewed By: cortinico
Differential Revision: D55749058
Pulled By: cipolleschi
fbshipit-source-id: 37659d22c1db4b1a27a9a4f88c9beb703517b01f
kosmydel pushed a commit to kosmydel/react-native that referenced this pull request Jun 11, 2024
…eact#44225)
Summary:
Pull Request resolved: react#44225
Back out [react#43031](react#43031) as it makes some internal test fails.
## Changelog:
[Android][Fixed] - Revert react#43031
Reviewed By: arushikesarwani94
Differential Revision: D56489260
fbshipit-source-id: bd843e6eb60fce0a1d5d8da9f3c5d2a36473ecfb
kosmydel pushed a commit to kosmydel/react-native that referenced this pull request Jun 11, 2024
Summary:
Pull Request resolved: react#44237
This adds support for color function values to ColorPropConverter per the wide gamut color [RFC](react-native-community/discussions-and-proposals#738). It updates the color conversion code so that it returns a Color instance before ultimately being converted to an Integer in preparation for returning long values as needed.
## Changelog:
[ANDROID] [ADDED] - Update ColorPropConverter to support color function values
Pull Request resolved: react#43031
Test Plan:
Colors should work exactly the same as before.
Follow test steps from react#42831 to test support for color() function syntax.
While colors specified with color() function syntax will not yet render in DisplayP3 color space they will not be misrecognized as resource path colors but will instead fallback to their sRGB color space values.
---
After the failure with the tests, I reapplied the changes and test some Jest e2e tests that were failing yesterday:
{F1495277376}
Reviewed By: cortinico
Differential Revision: D56517579
Pulled By: cipolleschi
fbshipit-source-id: ae9b5bc2afe9eb9760dd91afb090385daf7102b8
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA 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.p: Infinite RedPartner: Infinite RedPartnerShared 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.

3 participants

@ryanlntn@analysis-bot@facebook-github-bot