feat: radial gradient android changes #50269

intergalacticspacehighway · 2025-03-25T21:38:00Z

Summary:

Adds android changes for radial gradient. Previous PR - #50209

Changelog:

[ANDROID] [ADDED] - Radial gradient

Test Plan:

Added tests in processBackgroundImage-test.js in feat: radial gradient js prop passing #50268
Merge this and this and check examples in RadialGradientExample.js

facebook-github-bot · 2025-03-26T15:15:22Z

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

jorge-cab

1 bug, crashing on radius of 0 and the rest code styling stuff

jorge-cab · 2025-03-26T17:12:41Z

...react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/RadialGradient.kt

+    val shader = AndroidRadialGradient(
+      centerX,
+      centerY,
+      radiusX,


I think this will crash when passing a radius of 0?

Something like radial-gradient(0px 0px at center, red, blue)

Negative radius seems fine though it should fail like on web, I would add test for both cases

jorge-cab · 2025-03-26T17:41:04Z

...react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/RadialGradient.kt

+  private enum class Shape {
+    CIRCLE,
+    ELLIPSE;
+
+    companion object {
+      fun fromString(value: String): Shape {
+        return when (value.lowercase()) {
+          "circle" -> CIRCLE
+          "ellipse" -> ELLIPSE
+          else -> ELLIPSE
+        }
+      }
+    }
+  }


I think you can do:

Suggested change

private enum class Shape {

CIRCLE,

ELLIPSE;

companion object {

fun fromString(value: String): Shape {

return when (value.lowercase()) {

"circle" -> CIRCLE

"ellipse" -> ELLIPSE

else -> ELLIPSE

}

}

}

}

private enum class Shape(val value: String) {

CIRCLE("circle"),

ELLIPSE("ellipse");

companion object {

fun fromString(value: String): Shape =

enumValues<Shape>().find { it.value == value.lowercase() } ?: ELLIPSE

}

}

Since the fromString() method feels a bit arbitrary like that

jorge-cab · 2025-03-26T17:50:19Z

...react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/RadialGradient.kt

+  private enum class SizeKeyword {
+    CLOSEST_SIDE,
+    FARTHEST_SIDE,
+    CLOSEST_CORNER,
+    FARTHEST_CORNER;
+
+    companion object {
+      fun fromString(value: String?): SizeKeyword {
+        return when (value?.lowercase()) {
+          "closest-side" -> CLOSEST_SIDE
+          "farthest-side" -> FARTHEST_SIDE
+          "closest-corner" -> CLOSEST_CORNER
+          else -> FARTHEST_CORNER
+        }
+      }
+    }
+  }


Suggested change

private enum class SizeKeyword {

CLOSEST_SIDE,

FARTHEST_SIDE,

CLOSEST_CORNER,

FARTHEST_CORNER;

companion object {

fun fromString(value: String?): SizeKeyword {

return when (value?.lowercase()) {

"closest-side" -> CLOSEST_SIDE

"farthest-side" -> FARTHEST_SIDE

"closest-corner" -> CLOSEST_CORNER

else -> FARTHEST_CORNER

}

}

}

}

private enum class SizeKeyword {

CLOSEST_SIDE("closest-side"),

FARTHEST_SIDE("farthest-side"),

CLOSEST_CORNER("closest-corner"),

FARTHEST_CORNER("farthest-corner");

companion object {

fun fromString(value: String?): SizeKeyword =

enumValues<SizeKeyword>().find { it.value == value.lowercase() } ?: FARTHEST_CORNER

}

}

jorge-cab · 2025-03-26T18:02:21Z

...react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/RadialGradient.kt

+  private val shape: Shape = run {
+    val shapeString = gradientMap.getString("shape") ?: throw IllegalArgumentException("Radial gradient must have shape")
+    Shape.fromString(shapeString)
+  }


Suggested change

private val shape: Shape = run {

val shapeString = gradientMap.getString("shape") ?: throw IllegalArgumentException("Radial gradient must have shape")

Shape.fromString(shapeString)

}

private val shape: Shape = gradientMap.getString("shape")?.let { Shape.fromString(it) }

?: throw IllegalArgumentException("Radial gradient must have shape")

jorge-cab · 2025-03-26T18:09:04Z

...react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/LinearGradient.kt

-        }
-
-        else -> throw IllegalArgumentException("Invalid direction type: $type")
+  private val direction: Direction = run {


We are parsing AND initializing here. I think we should do a parseDirection function to split the logic a bit.

A way that would be nice to layer these, is if we have some ctor taking the concrete data types, and then some function that extracts those out of ReadableMap, and constructs from it. That will also make these types generic in the future to changes in how we might store the intermediates.

jorge-cab · 2025-03-26T18:09:22Z

...react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/RadialGradient.kt

+  }
+  private val isCircle: Boolean = shape == Shape.CIRCLE
+
+  private val position: Position = run {


For all run blocks. Maybe instead we can have a parsePosition(), parseSize and parseColorStops instead of doing the logic on initialization so we are not both initializing and parsing at the same time? @NickGerleman What do you think?

NickGerleman · 2025-03-26T20:16:44Z

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/Gradient.kt

-          linearGradient.getShader(bounds.width().toFloat(), bounds.height().toFloat())
-    }
-  }
+public interface Gradient {


Is it intentional we moved this from internal to public?

Suggested change

public interface Gradient {

internal interface Gradient {

NickGerleman · 2025-03-26T20:19:34Z

...native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/BackgroundImageLayer.kt

+          "linearGradient" -> LinearGradient(gradientMap, context)
+          "radialGradient" -> RadialGradient(gradientMap, context)


Left a comment on the JS PR, but we might want to change these for consistency

Suggested change

"linearGradient" -> LinearGradient(gradientMap, context)

"radialGradient" -> RadialGradient(gradientMap, context)

"linear-gradient" -> LinearGradient(gradientMap, context)

"radial-gradient" -> RadialGradient(gradientMap, context)

NickGerleman · 2025-03-26T20:21:30Z

...native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/BackgroundImageLayer.kt

        null
      }
+    } else {
+      null


nit: this is a little tricky to read. Consider

Early return null on null (or, only accept non-null? Seems like caller would need to check that anyways)

Explicitly check data types and return null instead of swallowing exception. That could help avoid over-swallowing logic errors too

NickGerleman · 2025-03-26T20:22:42Z

...native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/BackgroundImageLayer.kt

@@ -14,16 +14,22 @@ import com.facebook.react.bridge.ReadableMap

 public class BackgroundImageLayer(gradientMap: ReadableMap?, context: Context) {


nit: can we make this internal?

we're using this in BackgroundStyleApplicator so need to keep it public! 😕

I'm not quite following. E.g. BackgroundStyleApplicator as the public API relies on private internals like OutsetBoxShadowDrawable

So the issue is that it is being exposed as an argument to a public function. So when making it internal it results in below.

NickGerleman · 2025-03-26T20:24:29Z

...native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/BackgroundImageLayer.kt


-  public fun getShader(bounds: Rect): Shader? = gradient?.getShader(bounds)
+  public fun getShader(bounds: Rect): Shader? =
+    gradient?.getShader(bounds.width().toFloat(), bounds.height().toFloat())


Backing up a bit, wondering what the possible null state means here? I.e. why can we have a no-op BackgroundImageLayer? Wondering how this will also eventually fit into the work @jorge-cab will be taking on more in the future about image support for background-image.

If parsing any gradient fails we return null failing silently and do not draw the gradient like web. So this won't return a shader in that case!

Instead of allowing constructing a BackgroundImageLayer which is in an invalid state, could we pull parsing out of the constructor? E.g. so that some static parse function returns null if passed invalid data, but then, if we can correctly parse a BackgroundImageLayer, and returns one, it must be valid.

Added parse function here. i had to make the constructor private because of access modifier issues (can't keep the constructor public since Gradient interface is internal). So BackgroundImageLayer will stay public because of this but it's constructor will stay private.

NickGerleman · 2025-03-26T20:31:40Z

...ages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/ColorStop.kt

+
+public data class ProcessedColorStop(var color: Int? = null, val position: Float? = null)
+
+public object ColorStopUtils {


I know this is pulled out of old code, but now this seems pretty unit testable. Could we maybe add some tests for these? BorderRadiusStyleTest is a test for something in the same structure that could be matched I think.

NickGerleman · 2025-03-26T20:51:13Z

...ages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/ColorStop.kt

+import com.facebook.react.uimanager.PixelUtil
+import kotlin.math.ln
+
+public data class ColorStop(var color: Int? = null, val position: LengthPercentage? = null)


nit: we should prefer class over data class where we don't need equality or hashing bc they can lead to binary bloat. Some folks have been looking to remove data class usages from RN to save a bit on binary.

NickGerleman · 2025-03-26T20:52:22Z

...ages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/ColorStop.kt

+
+public data class ColorStop(var color: Int? = null, val position: LengthPercentage? = null)
+
+public data class ProcessedColorStop(var color: Int? = null, val position: Float? = null)


Why do we allow each part to be nullable?

Could we document what these types mean, and how they are used?

NickGerleman · 2025-03-26T20:57:09Z

...react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/LinearGradient.kt

-        }
-
-        else -> throw IllegalArgumentException("Invalid direction type: $type")
+  private val direction: Direction = run {


A way that would be nice to layer these, is if we have some ctor taking the concrete data types, and then some function that extracts those out of ReadableMap, and constructs from it. That will also make these types generic in the future to changes in how we might store the intermediates.

NickGerleman · 2025-03-26T20:59:38Z

...react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/style/RadialGradient.kt

+        return when (value.lowercase()) {
+          "circle" -> CIRCLE
+          "ellipse" -> ELLIPSE
+          else -> ELLIPSE


Should we ever get here?

True, JS processor handles all the defaults. Removed.

intergalacticspacehighway · 2025-03-29T09:47:16Z

@jorge-cab @NickGerleman Thanks for all the inputs and catching a bug. I have addressed all the comments (also pushed related changes to iOS and JS PRs). Let me know if anything else is needed!

NickGerleman · 2025-03-31T20:22:02Z

Left a couple quick comments, but otherwise mostly LGTM

facebook-github-bot · 2025-04-02T20:55:42Z

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

facebook-github-bot · 2025-04-07T16:53:57Z

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

facebook-github-bot · 2025-04-07T17:32:34Z

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

Summary: Adds JS changes for radial gradient. Previous PR - #50209 ## Changelog: [GENERAL] [ADDED] - Radial gradient <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests Pull Request resolved: #50268 Test Plan: - Added tests in `processBackgroundImage-test.js` in #50268 - Merge this, #50266 and #50269 and check examples in `RadialGradientExample.js` Reviewed By: NickGerleman Differential Revision: D71898524 Pulled By: jorge-cab fbshipit-source-id: 39bdf0f647ba19839b6e6deb09b30a1c337e6457

facebook-github-bot · 2025-04-17T23:43:14Z

@jorge-cab merged this pull request in a240994.

Summary: Adds android changes for radial gradient. Previous PR - facebook#50209 ## Changelog: [ANDROID] [ADDED] - Radial gradient <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests Pull Request resolved: facebook#50269 Test Plan: - Added tests in processBackgroundImage-test.js in facebook#50268 - Merge this and [this](facebook#50268) and check examples in `RadialGradientExample.js` Reviewed By: NickGerleman Differential Revision: D71898546 Pulled By: jorge-cab fbshipit-source-id: 2872bcf16a492c7bc829be10eedb0f6c7dad32c7

Summary: Adds JS changes for radial gradient. Previous PR - facebook#50209 ## Changelog: [GENERAL] [ADDED] - Radial gradient <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests Pull Request resolved: facebook#50268 Test Plan: - Added tests in `processBackgroundImage-test.js` in facebook#50268 - Merge this, facebook#50266 and facebook#50269 and check examples in `RadialGradientExample.js` Reviewed By: NickGerleman Differential Revision: D71898524 Pulled By: jorge-cab fbshipit-source-id: 39bdf0f647ba19839b6e6deb09b30a1c337e6457

radial gradient android changes

0d286c8

facebook-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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Mar 25, 2025

revert js example

bc75900

This was referenced Mar 25, 2025

feat: radial gradient js prop passing #50268

Closed

feat: radial gradient #50209

Closed

jorge-cab reviewed Mar 26, 2025

View reviewed changes

NickGerleman reviewed Mar 26, 2025

View reviewed changes

intergalacticspacehighway added 7 commits March 27, 2025 23:21

handle 0 radius user input on android

84d8883

code style changes

2c17c44

rename type from camelCase to kebab-case

e28db5a

refactor radial and linear gradients to use parse function

d978a9b

fix linear gradient direction

cb62d34

add color stop utils testcases

74adb21

add comments in color stop type

538ef70

add position null check in color stop test

b197aae

add parse function to BackgroundImageLayer

e175879

facebook-github-bot closed this in a240994 Apr 17, 2025

facebook-github-bot added the Merged This PR has been merged. label Apr 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: radial gradient android changes #50269

feat: radial gradient android changes #50269

intergalacticspacehighway commented Mar 25, 2025 •

edited

Loading

facebook-github-bot commented Mar 26, 2025

jorge-cab left a comment

jorge-cab Mar 26, 2025 •

edited

Loading

jorge-cab Mar 26, 2025

jorge-cab Mar 26, 2025

jorge-cab Mar 26, 2025

jorge-cab Mar 26, 2025

NickGerleman Mar 26, 2025

jorge-cab Mar 26, 2025

NickGerleman Mar 26, 2025

NickGerleman Mar 26, 2025

NickGerleman Mar 26, 2025

NickGerleman Mar 26, 2025

intergalacticspacehighway Mar 29, 2025

NickGerleman Mar 31, 2025

intergalacticspacehighway Apr 1, 2025

NickGerleman Mar 26, 2025

intergalacticspacehighway Mar 29, 2025

NickGerleman Mar 31, 2025

intergalacticspacehighway Apr 1, 2025

NickGerleman Mar 26, 2025

NickGerleman Mar 26, 2025

NickGerleman Mar 26, 2025

NickGerleman Mar 26, 2025

NickGerleman Mar 26, 2025

intergalacticspacehighway Mar 29, 2025

intergalacticspacehighway commented Mar 29, 2025 •

edited

Loading

NickGerleman commented Mar 31, 2025

facebook-github-bot commented Apr 2, 2025

facebook-github-bot commented Apr 7, 2025

facebook-github-bot commented Apr 7, 2025

facebook-github-bot commented Apr 17, 2025

		"linearGradient" -> LinearGradient(gradientMap, context)
		"radialGradient" -> RadialGradient(gradientMap, context)

		@@ -14,16 +14,22 @@ import com.facebook.react.bridge.ReadableMap

		public class BackgroundImageLayer(gradientMap: ReadableMap?, context: Context) {


		public data class ProcessedColorStop(var color: Int? = null, val position: Float? = null)

		public object ColorStopUtils {


		public data class ColorStop(var color: Int? = null, val position: LengthPercentage? = null)

		public data class ProcessedColorStop(var color: Int? = null, val position: Float? = null)

feat: radial gradient android changes #50269

feat: radial gradient android changes #50269

Conversation

intergalacticspacehighway commented Mar 25, 2025 • edited Loading

Summary:

Changelog:

Test Plan:

facebook-github-bot commented Mar 26, 2025

jorge-cab left a comment

Choose a reason for hiding this comment

jorge-cab Mar 26, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

intergalacticspacehighway commented Mar 29, 2025 • edited Loading

NickGerleman commented Mar 31, 2025

facebook-github-bot commented Apr 2, 2025

facebook-github-bot commented Apr 7, 2025

facebook-github-bot commented Apr 7, 2025

facebook-github-bot commented Apr 17, 2025

intergalacticspacehighway commented Mar 25, 2025 •

edited

Loading

jorge-cab Mar 26, 2025 •

edited

Loading

intergalacticspacehighway commented Mar 29, 2025 •

edited

Loading