Skip to content

[Fabric] Add support for PlatformColor #7801

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

Merged
5 commits merged into from
May 21, 2021

Conversation

acoates-ms
Copy link
Contributor

@acoates-ms acoates-ms commented May 19, 2021

This overrides the implementation of the native fabric Color object to add support for PlatformColor.

The Color object is already platform specific with a different implementation on ios than on android.

Microsoft Reviewers: Open in CodeFlow

@acoates-ms acoates-ms added the Area: Fabric Support Facebook Fabric label May 19, 2021
@acoates-ms acoates-ms requested a review from a team as a code owner May 19, 2021 16:50
@acoates-ms acoates-ms added the AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity) label May 21, 2021
@ghost
Copy link

ghost commented May 21, 2021

Hello @acoates-ms!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 2bb6811 into microsoft:master May 21, 2021
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request May 23, 2021
Summary:
`isColorMeaningful` is the only place in xplat code that currently uses `colorComponentsFromColor`, which assumes that a color is an RGBA value.  When implementing `PlatformColor` for windows, where colors might be complex patterns or effects, I'd like to keep the details of `SharedColor` isolated within `SharedColor`.  This change moves `isColorMeaningful` into `color.cpp`, where each platform can provide an implementation that takes into account its platform specific color capabilities.

See microsoft/react-native-windows#7801 for an example of window's SharedColor which can be either an RGBA value, or a name of a native color/brush.

## 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
-->

[Internal] [Changed] - Move isColorMeaningful to platform specific code

Pull Request resolved: #31557

Test Plan: This shouldn't change any of the code, its just moving the existing function - normal CI/automation should be plenty of validation.

Reviewed By: JoshuaGross, sammy-SC

Differential Revision: D28557698

Pulled By: mdvacca

fbshipit-source-id: 2a94850fe9c5037598107e1307f4153cee6491fb
@acoates-ms acoates-ms deleted the fabricplatformcolor branch July 23, 2021 16:07
@jonthysell jonthysell added the New Architecture Broad category for issues that apply to the RN "new" architecture of Turbo Modules + Fabric label Mar 7, 2024
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Fabric Support Facebook Fabric AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity) New Architecture Broad category for issues that apply to the RN "new" architecture of Turbo Modules + Fabric
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

4 participants