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

feat: linear gradient android #45433

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
7f84c01
Linear gradient
intergalacticspacehighway Jun 20, 2024
19a21d3
fix
intergalacticspacehighway Jul 15, 2024
f7ac2c7
revert ios changes
intergalacticspacehighway Jul 15, 2024
1b50d8a
nits
intergalacticspacehighway Jul 15, 2024
b75d4af
fix: snapshot test
intergalacticspacehighway Jul 15, 2024
d79e1d3
rename background to experimental_backgroundImage
intergalacticspacehighway Jul 19, 2024
d38772c
merge main
intergalacticspacehighway Jul 19, 2024
f382d98
fix snapshot tests
intergalacticspacehighway Jul 19, 2024
6ffd05d
fix: prettier format
intergalacticspacehighway Jul 19, 2024
1d6f728
gate experimental_backgroundImage to fabric only
intergalacticspacehighway Jul 20, 2024
1ee4743
fix: native static view config warning
intergalacticspacehighway Jul 20, 2024
9cf5c15
fix: prettier
intergalacticspacehighway Jul 20, 2024
badf935
fix: type
intergalacticspacehighway Jul 20, 2024
d4e1436
chore: improve examples
intergalacticspacehighway Jul 20, 2024
6c0fe54
Merge branch 'main' into feat/linear-gradient-android
intergalacticspacehighway Jul 27, 2024
3df8f0e
fix: case insensitive css linear gradient values
intergalacticspacehighway Jul 27, 2024
98c6cf6
improve testcases
intergalacticspacehighway Jul 27, 2024
0c4e20a
improve testcases
intergalacticspacehighway Jul 27, 2024
62aa6b3
rename type background image primitive to gradient value
intergalacticspacehighway Jul 28, 2024
cfa017f
convert css gradient to kotlin
intergalacticspacehighway Jul 28, 2024
7531702
rename cssgradient and move to uimanager style package
intergalacticspacehighway Jul 28, 2024
0cc8e89
merge main
intergalacticspacehighway Jul 28, 2024
8b43b07
fix types
intergalacticspacehighway Jul 28, 2024
b944b8f
move experimental_backgroundImage to view only
intergalacticspacehighway Jul 28, 2024
c29529f
baseviewprops handle enablePropIteratorSetter
intergalacticspacehighway Jul 28, 2024
085262c
move backgroundimage check to formsview
intergalacticspacehighway Jul 29, 2024
6592715
align style object closely to css spec
intergalacticspacehighway Jul 29, 2024
1cb92de
fix: ts type
intergalacticspacehighway Jul 29, 2024
2557da0
more test cases
intergalacticspacehighway Jul 29, 2024
f3c5aa3
case insensitive linear gradient fn test
intergalacticspacehighway Jul 29, 2024
28034d4
cleanup
intergalacticspacehighway Jul 29, 2024
2f1bf9e
draw gradient above background color
intergalacticspacehighway Jul 31, 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
Prev Previous commit
Next Next commit
rename background to experimental_backgroundImage
  • Loading branch information
intergalacticspacehighway committed Jul 19, 2024
commit d79e1d3346f68f2107a0dd3834a11f40e2f07a14
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import type {AnyAttributeType} from '../../Renderer/shims/ReactNativeTypes';

import processAspectRatio from '../../StyleSheet/processAspectRatio';
import processBackground from '../../StyleSheet/processBackground';
import processBackgroundImage from '../../StyleSheet/processBackgroundImage';
import processColor from '../../StyleSheet/processColor';
import processFilter from '../../StyleSheet/processFilter';
import processFontVariant from '../../StyleSheet/processFontVariant';
Expand Down Expand Up @@ -129,7 +129,7 @@ const ReactNativeStyleAttributes: {[string]: AnyAttributeType, ...} = {
/**
* Linear Gradient
*/
background: {process: processBackground},
experimental_backgroundImage: {process: processBackgroundImage},

/**
* View
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,9 @@ const directEventTypes = {
const validAttributesForNonEventProps = {
// @ReactProps from BaseViewManager
backgroundColor: {process: require('../StyleSheet/processColor').default},
background: {process: require('../StyleSheet/processBackground').default},
experimental_backgroundImage: {
process: require('../StyleSheet/processBackgroundImage').default,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Related to comment below about keeping this off of BaseViewManager, if we start with just <View>, could we keep local to that view manager and viewconfig?

transform: true,
transformOrigin: true,
experimental_filter: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export type FilterPrimitive =
| {sepia: number | string}
| {'drop-shadow': DropShadowPrimitive | string};

export type BackgroundPrimitive = {
export type BackgroundImagePrimitive = {
type: 'linearGradient';
start: {x: number; y: number};
end: {x: number; y: number};
Expand Down Expand Up @@ -262,7 +262,6 @@ export interface TransformsStyle {
export interface ViewStyle extends FlexStyle, ShadowStyleIOS, TransformsStyle {
backfaceVisibility?: 'visible' | 'hidden' | undefined;
backgroundColor?: ColorValue | undefined;
background?: Array<BackgroundPrimitive> | string | undefined;
borderBlockColor?: ColorValue | undefined;
borderBlockEndColor?: ColorValue | undefined;
borderBlockStartColor?: ColorValue | undefined;
Expand Down
7 changes: 5 additions & 2 deletions packages/react-native/Libraries/StyleSheet/StyleSheetTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
'use strict';

import type AnimatedNode from '../Animated/nodes/AnimatedNode';
import type {BackgroundPrimitive} from '../StyleSheet/processBackground';
import type {BackgroundImagePrimitive} from '../StyleSheet/processBackgroundImage';
import type {
____DangerouslyImpreciseStyle_InternalOverrides,
____ImageStyle_InternalOverrides,
Expand Down Expand Up @@ -731,8 +731,11 @@ export type ____MixBlendMode_Internal =
| 'saturation'
| 'color'
| 'luminosity';

type ____BackgroundStyle_Internal = $ReadOnly<{
background?: $ReadOnlyArray<BackgroundPrimitive> | string,
experimental_backgroundImage?:
| $ReadOnlyArray<BackgroundImagePrimitive>
| string,
}>;

export type ____ViewStyle_InternalCore = $ReadOnly<{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@

'use strict';

import processBackground from '../processBackground';
import processBackgroundImage from '../processBackgroundImage';

const processColor = require('../processColor').default;

describe('processBackground', () => {
describe('processBackgroundImage', () => {
it('should process a simple linear gradient string', () => {
const input = 'linear-gradient(to right, red, blue)';
const result = processBackground(input);
const result = processBackgroundImage(input);
expect(result).toEqual([
{
type: 'linearGradient',
Expand All @@ -33,7 +33,7 @@ describe('processBackground', () => {

it('should process a linear gradient with angle', () => {
const input = 'linear-gradient(45deg, red, blue)';
const result = processBackground(input);
const result = processBackgroundImage(input);
expect(result[0].type).toBe('linearGradient');
expect(result[0].start.x).toBeCloseTo(0.146447, 5);
expect(result[0].start.y).toBeCloseTo(0.853553, 5);
Expand All @@ -48,7 +48,7 @@ describe('processBackground', () => {
it('should process multiple linear gradients', () => {
const input =
'linear-gradient(to right, red, blue), linear-gradient(to bottom, green, yellow)';
const result = processBackground(input);
const result = processBackgroundImage(input);
expect(result).toHaveLength(2);
expect(result[0].start).toEqual({x: 0, y: 0.5});
expect(result[0].end).toEqual({x: 1, y: 0.5});
Expand All @@ -58,7 +58,7 @@ describe('processBackground', () => {

it('should process a linear gradient with multiple color stops', () => {
const input = 'linear-gradient(to right, red 0%, green 50%, blue 100%)';
const result = processBackground(input);
const result = processBackgroundImage(input);
expect(result[0].colorStops).toEqual([
{color: processColor('red'), position: 0},
{color: processColor('green'), position: 0.5},
Expand All @@ -69,7 +69,7 @@ describe('processBackground', () => {
it('should process a linear gradient with rgba colors', () => {
const input =
'linear-gradient(to right, rgba(255,0,0,0.5), rgba(0,0,255,0.8))';
const result = processBackground(input);
const result = processBackgroundImage(input);
expect(result[0].colorStops).toEqual([
{color: processColor('rgba(255,0,0,0.5)'), position: 0},
{color: processColor('rgba(0,0,255,0.8)'), position: 1},
Expand All @@ -78,7 +78,7 @@ describe('processBackground', () => {

it('should process a linear gradient with hsl colors', () => {
const input = `linear-gradient(hsl(330, 100%, 45.1%), hsl(0, 100%, 50%))`;
const result = processBackground(input);
const result = processBackgroundImage(input);
expect(result[0].colorStops).toEqual([
{color: processColor('hsl(330, 100%, 45.1%)'), position: 0},
{color: processColor('hsl(0, 100%, 50%)'), position: 1},
Expand All @@ -87,7 +87,7 @@ describe('processBackground', () => {

it('should process a linear gradient without direction', () => {
const input = 'linear-gradient(#e66465, #9198e5)';
const result = processBackground(input);
const result = processBackgroundImage(input);
expect(result[0].colorStops).toEqual([
{color: processColor('#e66465'), position: 0},
{color: processColor('#9198e5'), position: 1},
Expand All @@ -98,7 +98,7 @@ describe('processBackground', () => {
const input = `linear-gradient(to right ,
rgba(255,0,0,0.5), rgba(0,0,255,0.8)),
linear-gradient(to bottom , rgba(255,0,0,0.9) , rgba(0,0,255,0.2) )`;
const result = processBackground(input);
const result = processBackgroundImage(input);
expect(result).toHaveLength(2);
expect(result[0].start).toEqual({x: 0, y: 0.5});
expect(result[0].end).toEqual({x: 1, y: 0.5});
Expand Down Expand Up @@ -126,7 +126,7 @@ describe('processBackground', () => {
],
},
];
const result = processBackground(input);
const result = processBackgroundImage(input);
expect(result).toEqual([
{
type: 'linearGradient',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const TO_BOTTOM_START_END_POINTS = {
end: {x: 0.5, y: 1},
};

export type BackgroundPrimitive = {
export type BackgroundImagePrimitive = {
type: 'linearGradient',
start: {x: number, y: number},
end: {x: number, y: number},
Expand All @@ -36,14 +36,14 @@ export type BackgroundPrimitive = {
}>,
};

export default function processBackground(
background: $ReadOnlyArray<BackgroundPrimitive> | string,
): $ReadOnlyArray<BackgroundPrimitive> {
if (typeof background === 'string') {
const parsedBackground = parseCSSLinearGradient(background);
return parsedBackground;
} else if (Array.isArray(background)) {
const parsedBackground = background.map(bg => {
export default function processBackgroundImage(
backgroundImage: $ReadOnlyArray<BackgroundImagePrimitive> | string,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add handling for nullish input? Recently hit us somewhere else.

): $ReadOnlyArray<BackgroundImagePrimitive> {
Copy link
Contributor

Choose a reason for hiding this comment

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

The output has some more guarantees that the input doesn't. E.g. output of processor ColorValue should become ProcessedColorValue, right?

if (typeof backgroundImage === 'string') {
const parsedBackgroundImage = parseCSSLinearGradient(backgroundImage);
return parsedBackgroundImage;
} else if (Array.isArray(backgroundImage)) {
const parsedBackgroundImage = backgroundImage.map(bg => {
return {
type: bg.type,
start: bg.start,
Expand All @@ -57,15 +57,15 @@ export default function processBackground(
}),
};
});
return parsedBackground;
return parsedBackgroundImage;
}

return [];
}

function parseCSSLinearGradient(
cssString: string,
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this handle case insensitivity correctly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. It didn't, fixed and added testcases.

): $ReadOnlyArray<BackgroundPrimitive> {
): $ReadOnlyArray<BackgroundImagePrimitive> {
const gradients = [];
let match;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7719,7 +7719,9 @@ export type ____MixBlendMode_Internal =
| \\"color\\"
| \\"luminosity\\";
type ____BackgroundStyle_Internal = $ReadOnly<{
background?: $ReadOnlyArray<BackgroundPrimitive> | string,
experimental_backgroundImage?:
| $ReadOnlyArray<BackgroundImagePrimitive>
| string,
}>;
export type ____ViewStyle_InternalCore = $ReadOnly<{
...$Exact<____LayoutStyle_Internal>,
Expand Down Expand Up @@ -8007,8 +8009,8 @@ declare module.exports: processAspectRatio;
"
`;

exports[`public API should not change unintentionally Libraries/StyleSheet/processBackground.js 1`] = `
"export type BackgroundPrimitive = {
exports[`public API should not change unintentionally Libraries/StyleSheet/processBackgroundImage.js 1`] = `
"export type BackgroundImagePrimitive = {
type: \\"linearGradient\\",
start: { x: number, y: number },
end: { x: number, y: number },
Expand All @@ -8017,9 +8019,9 @@ exports[`public API should not change unintentionally Libraries/StyleSheet/proce
position: number,
}>,
};
declare export default function processBackground(
background: $ReadOnlyArray<BackgroundPrimitive> | string
): $ReadOnlyArray<BackgroundPrimitive>;
declare export default function processBackgroundImage(
backgroundImage: $ReadOnlyArray<BackgroundImagePrimitive> | string
): $ReadOnlyArray<BackgroundImagePrimitive>;
"
`;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,9 @@ public void setTransform(@NonNull T view, @Nullable ReadableArray matrix) {
}

@Override
@ReactProp(name = ViewProps.BACKGROUND)
public void setBackground(@NonNull T view, @Nullable ReadableArray gradients) {
logUnsupportedPropertyWarning(ViewProps.BACKGROUND);
@ReactProp(name = ViewProps.BACKGROUND_IMAGE)
public void setBackgroundImage(@NonNull T view, @Nullable ReadableArray backgroundImage) {
logUnsupportedPropertyWarning(ViewProps.BACKGROUND_IMAGE);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ public void setProperty(T view, String propName, @Nullable Object value) {
case ViewProps.TRANSFORM:
mViewManager.setTransform(view, (ReadableArray) value);
break;
case ViewProps.BACKGROUND:
mViewManager.setBackground(view, (ReadableArray) value);
case ViewProps.BACKGROUND_IMAGE:
mViewManager.setBackgroundImage(view, (ReadableArray) value);
break;
case ViewProps.TRANSFORM_ORIGIN:
mViewManager.setTransformOrigin(view, (ReadableArray) value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public interface BaseViewManagerInterface<T extends View> {

void setTransform(T view, @Nullable ReadableArray matrix);

void setBackground(T view, @Nullable ReadableArray matrix);
void setBackgroundImage(T view, @Nullable ReadableArray backgroundImage);

void setTransformOrigin(T view, @Nullable ReadableArray transformOrigin);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public object ViewProps {
// Props that affect more than just layout
public const val ENABLED: String = "enabled"
public const val BACKGROUND_COLOR: String = "backgroundColor"
public const val BACKGROUND: String = "background"
public const val BACKGROUND_IMAGE: String = "experimental_backgroundImage"
public const val FOREGROUND_COLOR: String = "foregroundColor"
public const val COLOR: String = "color"
public const val FONT_SIZE: String = "fontSize"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,12 @@ public void setTVPreferredFocus(ReactViewGroup view, boolean hasTVPreferredFocus
}
}

@ReactProp(name = ViewProps.BACKGROUND)
public void setBackground(ReactViewGroup view, @Nullable ReadableArray background) {
if (background != null && background.size() > 0) {
CSSGradient[] cssGradients = new CSSGradient[background.size()];
for (int i = 0; i < background.size(); i++) {
ReadableMap gradientMap = background.getMap(i);
@ReactProp(name = ViewProps.BACKGROUND_IMAGE)
public void setBackgroundImage(ReactViewGroup view, @Nullable ReadableArray backgroundImage) {
Copy link
Contributor

Choose a reason for hiding this comment

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

After this lands, this change will intersect a bit with another one, where I have been refactoring to be able to eventually apply background styles as part of BaseViewManager. bbcca54

This also enables the infra for CompositeBackgroundDrawable, for any future cases where we want to use individual Drawables as units of layering (like, if we wanted to mix Fresco provided bitmap image drawables, between gradients, or something like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CompositeBackgroundDrawable would be cool!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is kind of similar to iOS view layers. Exciting!

if (backgroundImage != null && backgroundImage.size() > 0) {
CSSGradient[] cssGradients = new CSSGradient[backgroundImage.size()];
for (int i = 0; i < backgroundImage.size(); i++) {
ReadableMap gradientMap = backgroundImage.getMap(i);
cssGradients[i] = new CSSGradient(gradientMap);
}
view.setGradients(cssGradients);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,13 @@ BaseViewProps::BaseViewProps(
"experimental_filter",
sourceProps.filter,
{})),
background(
CoreFeatures::enablePropIteratorSetter ? sourceProps.background
backgroundImage(
CoreFeatures::enablePropIteratorSetter ? sourceProps.backgroundImage
Copy link
Contributor

Choose a reason for hiding this comment

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

Please also add to the area around RAW_SET_PROP_SWITCH_CASE_BASIC

enablePropIteratorSetter is still enabled in a few places internally, and if enabled, it goes through that path, in setProp, instead of the convertRawProp here.

: convertRawProp(
context,
rawProps,
"background",
sourceProps.background,
"experimental_backgroundImage",
sourceProps.backgroundImage,
{})),
mixBlendMode(
CoreFeatures::enablePropIteratorSetter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
#include <react/renderer/graphics/Color.h>
#include <react/renderer/graphics/Filter.h>
#include <react/renderer/graphics/Transform.h>
#include <react/renderer/graphics/Background.h>
#include <react/renderer/graphics/BackgroundImage.h>

#include <optional>

Expand Down Expand Up @@ -64,7 +64,7 @@ class BaseViewProps : public YogaStylableProps, public AccessibilityProps {
std::vector<FilterPrimitive> filter{};

// Linear gradient
std::vector<BackgroundPrimitive> background{};
std::vector<BackgroundImagePrimitive> backgroundImage{};

// MixBlendMode
std::string mixBlendMode;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ void ViewShadowNode::initialize() noexcept {
viewProps.importantForAccessibility != ImportantForAccessibility::Auto ||
viewProps.removeClippedSubviews || viewProps.cursor != Cursor::Auto ||
!viewProps.filter.empty() || !viewProps.mixBlendMode.empty() ||
!viewProps.background.empty() ||
!viewProps.backgroundImage.empty() ||
Copy link
Contributor

@NickGerleman NickGerleman Jul 23, 2024

Choose a reason for hiding this comment

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

This is not the only prop guilty of this, but I think this one should go in formsView, instead of formsStackingContext. I.e. we don't want views with a background image to be flattened, but they also shouldn't form their own stacking context for z-ordering (at least on browsers https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_positioned_layout/Understanding_z-index/Stacking_context)

HostPlatformViewTraitsInitializer::formsStackingContext(viewProps);

bool formsView = formsStackingContext ||
Expand Down
Loading