Repository navigation
Add conic gradients to View backgroundImage - #58143
Stringsaeed wants to merge 19 commits into
Conversation
|
Warning JavaScript API change detected This PR commits an update to
This change was flagged as: |
866b153 to
178d3e5
Compare
|
Hey @intergalacticspacehighway, who help landing this PR? |
| if (gradient.position.top.has_value()) { | ||
| centerPoint.y = gradient.position.top->resolve(static_cast<float>(size.height)); | ||
| } else if (gradient.position.bottom.has_value()) { | ||
| centerPoint.y = size.height - gradient.position.bottom->resolve(static_cast<float>(size.height)); | ||
| } |
There was a problem hiding this comment.
if we pass conic-gradient(from 0deg at right 10px bottom 20px, ...) the JS emits bottom 20, right 10 but BackgroundImagePropsConversions.cpp pre-sets position.top = 50% and position.left = 50% so bottom and right here would not override those values.
There was a problem hiding this comment.
Fixed in d3bc12e9f91. Explicit bottom now clears the default top, and explicit right clears the default left, while unspecified axes remain centered. Added regression coverage for string/object syntax across native CSS parsing and C++ prop-setter flag combinations, and verified the positioning in RNTester on both platforms.
178d3e5 to
8a24d6f
Compare
|
Do we need to separate the PR into smaller 2 PRs? one for C++ foundation. @intergalacticspacehighway what do you think? |
intergalacticspacehighway
left a comment
There was a problem hiding this comment.
Looks good to me. CCing @jorge-cab for the final decision.
160d2b6 to
c4dd2b2
Compare
|
Hi @cipolleschi @cortinico — would either of you have a chance to take a look at this PR? I’d especially appreciate feedback on the Fabric/native implementation and whether the overall API/approach fits where Thanks! |
That's a 3K LOC PR :/ plus this comes from out of the blue without an issue where we could have discussed this beforehand. |
That's actually a good point, I should've opened an issue to discuss. I'm happy to open one now so we can align on the direction first. I can also split this PR into smaller gh stacked PRs if that would make it easier to review. A good portion of code is tests/RNTester/API snapshots, but I agree the actual change is still fairly broad across JS, C++, iOS, and Android. I thought we should decrease css gap as ongoing project in react-native, but maybe I have wrong context. Would you prefer that I open the issue first and pause this PR until we agree on the approach? |
In general those changes start with a GitHub issue opened on this repo: I'd love to know @intergalacticspacehighway opinion on this change as a whole also before we invest more time on it. |
|
Guys, this PR is cool addition to current gradients capabilities in RN and can be very helpful with diagrams etc |
|
@cortinico The PR follows the existing backgroundImage/gradient patterns and platform APIs, so it fits in well. Looks good to me 👍 One small nit @Stringsaeed: Object-syntax linear/radial stops now accept |
|
Let's start by rebasing this one as we can't merge it as is (there are several conflicts) |
73a6905 to
5d4d2e7
Compare
|
@cortinico Updated ✅ @intergalacticspacehighway now we keeping the old behavior back |
Summary
Adds
conic-gradient()support toView’sbackgroundImagestyle on iOS and Android.Supports:
from <angle>.at <position>, including edge offsets, percentages, fractional values, and negative offsets.Implementation
ConicGradientmodel, and native prop conversion.CAGradientLayeron iOS andSweepGradienton Android.pxstrings in linear/radial object color stops and transition hints. Numeric and percentage object positions remain supported.RNTester includes 16 corner cases with string/object syntax switching, dimension swapping, and hide/show controls.
Scope
This PR does not add:
repeating-conic-gradient().in hsl.Changelog:
[General] [Added] - Support conic gradients in View backgroundImage.
Test Plan
Automated checks
yarn --cwd packages/react-native-codegen build: passed.yarn build-types,yarn build-types --validate, andyarn test-generated-typescript: passed.yarn flow-check: passed with 0 errors.ESLint and Prettier for changed JavaScript files, C++ formatting for changed files, and the repository Kotlin formatting check: passed.
C++ API snapshots: regenerated all 12 variants with
yarn cxx-api-buildusing Doxygen 1.16.1 and Python 3.12.14. Generation ran in a clean source copy to exclude ignored native build headers.yarn cxx-api-validate --snapshot-dir /Users/saeed/projects/react-native/scripts/cxx-api/api-snapshots --output-dir /private/tmp/pr58143-cxx-current-validationpassed all 12 snapshots.Fantom: all 796 tests passed across these six suites:
yarn fantom 'View-conicGradient-itest|processConicGradient-itest|processBackgroundImage-itest|processBackgroundPosition-itest|/View-itest|View-nativeCSSParsing-itest' --runInBand
Coverage includes both native CSS parsing modes and both C++ prop-setter modes.
Android: all 19 tests passed, including 12 conic-gradient tests and 7 color-stop tests:
./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest
--tests com.facebook.react.uimanager.style.ConicGradientTest
--tests com.facebook.react.uimanager.style.ColorStopTest
-Preact.internal.useHermesNightly=false -Pkotlin.incremental=false
--console=plain --no-parallel
C++ CSS parser: all 39
CSSBackgroundImageTesttests passed, including five conic cases, using a standalone C++20 GoogleTest runner compiled from the current test source.Validation limits
apiDumporapiCheck; both task invocations reported task-not-found. The Android API snapshot is unchanged.-Wno-error=deprecated-literal-operatorfor a third-party JSON header. No source or build-script workaround is included.Verified
Ran it locally: yes
The new tests reject
positions: ['20px']in both linear and radial object syntax, for colored stops and transition hints. They were run with the pre-fix parser and then the fixed parser:Existing tests continue to cover numeric and percentage object stops, CSS pixel stops, and conic angular stops.
Earlier native rendering verification
RNTester was built and run on an iOS simulator and Android emulator. All 16 corner cases were checked, along with resizing, hiding, and restoring the gradient without remounting the view.
All 11 iOS pixel tests passed through a focused XCTest runner linked against the production renderer and gradient utilities. Four cases failed before the rectangular-rendering fix. This was not a run of the complete RNTester XCTest suite.
The screenshots below record that earlier verification.
Visual verification
Tested in RNTester on iOS and Android. The “Conic Gradient Corner Cases” example includes 16 selectable cases, string/object syntax switching, resizing, and hide/show controls.
Geometry and rotation
The red wedge should retain its 45° angle across aspect ratios. The 90° and −270° rotations should look identical.
Center positioning
The white ring marks the expected center where it falls within the view. Negative offsets place the center above and left of the view, producing a solid-blue visible area.
Color stops, tiling, and composition
Repeated rectangular tiles should retain independent, undistorted centers. The transition hint controls the blend midpoint, and transparent portions of the conic layer reveal the underlying linear gradient.
Invalid input and zero-size views
Invalid gradients should leave only the gray fallback background. A zero-size view should paint nothing and must not crash.
Updates without remounting
Swapping dimensions should recalculate the gradient geometry. Hiding should reveal the fallback background, and restoring should redraw the gradient correctly.