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

[Fabric] ShadowProps handling were put behind a feature flag, but now no ComponentViews implement that flag #12577

Closed
jonthysell opened this issue Jan 9, 2024 · 1 comment · Fixed by #12623
Assignees
Labels
Area: Borders and Brushes Area: Fabric Support Facebook Fabric bug New Architecture Broad category for issues that apply to the RN "new" architecture of Turbo Modules + Fabric
Milestone

Comments

@jonthysell
Copy link
Contributor

Problem Description

Looking at the RN docs, most of the core components support both border and shadow props.

The change in PR #12287 added CompositionComponentViewFeatures flags to control whether the CompositionBase ComponentView processes border/shadow props for the subclassed ComponentView. The change also set all components to use CompositionComponentViewFeatures::Default, which is Borders only, which was fine, because each individual ComponentView sublclass processed shadows themselves.

However PR #12474 removed all these manual calls to process shadow props in the subclasses, without updating their feature flags to include the shadow props.

Steps To Reproduce

This was seen through static analysis.

Expected Results

No response

CLI version

npx react-native -v

Environment

npx react-native info

Target Platform Version

None

Target Device(s)

Desktop

Visual Studio Version

None

Build Configuration

None

Snack, code example, screenshot, or link to a repository

No response

@jonthysell jonthysell added bug Area: Fabric Support Facebook Fabric labels Jan 9, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) label Jan 9, 2024
@chrisglein chrisglein added this to the Next milestone Jan 11, 2024
@chrisglein chrisglein added Area: Borders and Brushes and removed Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) labels Jan 11, 2024
@chrisglein
Copy link
Member

Worth seeing how tests could have caught this. It's possible that snapshots didn't include the shadows, which would explain it. But we want this to break a test.

@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Borders and Brushes Area: Fabric Support Facebook Fabric bug New Architecture Broad category for issues that apply to the RN "new" architecture of Turbo Modules + Fabric
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants