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.

Sign up for GitHub

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

fix: Linear gradient border styles with BackgroundStyleApplicator #46084

Closed
wants to merge 5 commits into from
Closed

fix: Linear gradient border styles with BackgroundStyleApplicator #46084

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6234525
fix: gradient with background style applicator
intergalacticspacehighway Aug 18, 2024
a91a41a
Merge branch 'main' into fix/gradient-background-style-applicator
intergalacticspacehighway Aug 23, 2024
b60e990
clear gradient on null
intergalacticspacehighway Aug 23, 2024
dcfd810
background image layer interface
intergalacticspacehighway Aug 24, 2024
4bb262a
use list instead of array
intergalacticspacehighway Aug 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions ...tive/ReactAndroid/src/main/java/com/facebook/react/uimanager/BackgroundStyleApplicator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import com.facebook.react.uimanager.style.BorderRadiusProp
import com.facebook.react.uimanager.style.BorderRadiusStyle
import com.facebook.react.uimanager.style.BorderStyle
import com.facebook.react.uimanager.style.BoxShadow
import com.facebook.react.uimanager.style.Gradient
import com.facebook.react.uimanager.style.LogicalEdge

/**
Expand All @@ -49,6 +50,13 @@ public object BackgroundStyleApplicator {
ensureCSSBackground(view).color = color ?: Color.TRANSPARENT
}

@JvmStatic
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. We might eventually support background images based on URI, in which case we’d need to change this public API. Can we design in a way that will be future proof to that? Like maybe having a type for background image layer, that can be gradient type now, and uri later?
  2. Can we accept by list instead of array for consistency?

public fun setBackgroundImage(view: View, gradients: Array<Gradient>?): Unit {
if (!gradients.isNullOrEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If input is null or empty we should clear any existing gradients

ensureCSSBackground(view).setBackgroundImage(gradients);
}
}

@JvmStatic
@ColorInt
public fun getBackgroundColor(view: View): Int? = getCSSBackground(view)?.color
Expand Down
2 changes: 1 addition & 1 deletion ...actAndroid/src/main/java/com/facebook/react/uimanager/drawable/CSSBackgroundDrawable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ public void setColor(int color) {
invalidateSelf();
}

public void setGradients(Gradient[] gradients) {
public void setBackgroundImage(Gradient[] gradients) {
mGradients = gradients;
invalidateSelf();
}
Expand Down
8 changes: 6 additions & 2 deletions ...react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,12 @@ public void setBackgroundColor(int color) {
}

@UnstableReactNativeAPI
/*package*/ void setGradients(@Nullable Gradient[] gradient) {
getOrCreateReactViewBackground().setGradients(gradient);
/*package*/ void setBackgroundImage(@Nullable Gradient[] gradient) {
if (ReactNativeFeatureFlags.enableBackgroundStyleApplicator()) {
BackgroundStyleApplicator.setBackgroundImage(this, gradient);
} else {
getOrCreateReactViewBackground().setBackgroundImage(gradient);
}
}

@Deprecated(since = "0.76.0", forRemoval = true)
Expand Down
4 changes: 2 additions & 2 deletions ...act-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,9 @@ public void setBackgroundImage(ReactViewGroup view, @Nullable ReadableArray back
ReadableMap gradientMap = backgroundImage.getMap(i);
gradients[i] = new Gradient(gradientMap);
}
view.setGradients(gradients);
view.setBackgroundImage(gradients);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The greater context behind BackgroundStyleApplicator is that it works on any view (e.g. Text instead of just View). Once the flag has been enabled a bit longer, my plan has been to start moving all the setters to BaseViewMansger.

This is why the current view managers use BackgroundStyleApplicator directly when the gate is on, instead of calling through any specific underlying views.

} else {
view.setGradients(null);
view.setBackgroundImage(null);
}
}
}
Expand Down
Loading