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! 😕

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!

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!

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

Open

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

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 26, 2025

intergalacticspacehighway Mar 29, 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

		"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

Are you sure you want to change the base?

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

intergalacticspacehighway commented Mar 29, 2025 • edited Loading

intergalacticspacehighway commented Mar 25, 2025 •

edited

Loading

jorge-cab Mar 26, 2025 •

edited

Loading

intergalacticspacehighway commented Mar 29, 2025 •

edited

Loading