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

feat: linear gradient px and transition hint syntax support #48410

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

feat: linear gradient px and transition hint syntax support #48410

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f65c873
linear gradient transition hint syntax
intergalacticspacehighway Dec 30, 2024
8bbcc0a
nit perf
intergalacticspacehighway Jan 2, 2025
6913cdb
add more tests in processbackgroundimage
intergalacticspacehighway Jan 3, 2025
3c443c3
Linear gradient with px (#3)
intergalacticspacehighway Jan 5, 2025
a21b3e6
update snapshot
intergalacticspacehighway Jan 5, 2025
a1048a2
use LengthPercentage for position
intergalacticspacehighway Jan 9, 2025
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
2 changes: 1 addition & 1 deletion packages/react-native/Libraries/StyleSheet/StyleSheetTypes.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ export type GradientValue = {
// Angle or direction enums
direction?: string | undefined;
colorStops: ReadonlyArray<{
color: ColorValue;
color: ColorValue | null;
positions?: ReadonlyArray<string[]> | undefined;
}>;
};
Expand Down
30 changes: 30 additions & 0 deletions packages/react-native/Libraries/StyleSheet/__tests__/processBackgroundImage-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -697,4 +697,34 @@ describe('processBackgroundImage', () => {
});
}
});

it('should process color transition hint in object style', () => {
const input = [
{
type: 'linearGradient',
direction: 'To Bottom',
colorStops: [{color: 'red'}, {positions: ['20%']}, {color: 'blue'}],
},
];
const result = processBackgroundImage(input);
expect(result[0].type).toBe('linearGradient');
expect(result[0].direction).toEqual({type: 'angle', value: 180});
expect(result[0].colorStops).toEqual([
{color: processColor('red'), position: 0},
{color: null, position: 0.2},
{color: processColor('blue'), position: 1},
]);
});

it('should process color transition hint', () => {
const input = 'linear-gradient(red, 40%, blue)';
const result = processBackgroundImage(input);
expect(result[0].type).toBe('linearGradient');
expect(result[0].direction).toEqual({type: 'angle', value: 180});
expect(result[0].colorStops).toEqual([
{color: processColor('red'), position: 0},
{color: null, position: 0.4},
{color: processColor('blue'), position: 1},
]);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add some more? Hints that are invalid (> 100%, non percents, multiple in a row, etc). Some longer ones with more stops and more hints just to stress test things a bit, etc. The code has a lot of branching logic due to the complexity of the syntax we are allowing, so I think we should err on the side of excessive testing to raise our confidence level here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, added.

});
149 changes: 104 additions & 45 deletions packages/react-native/Libraries/StyleSheet/processBackgroundImage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@ type LinearGradientDirection =
| {type: 'angle', value: number}
| {type: 'keyword', value: string};

type ColorStopColor = ProcessedColorValue | null;

type ParsedGradientValue = {
type: 'linearGradient',
direction: LinearGradientDirection,
colorStops: $ReadOnlyArray<{
color: ProcessedColorValue,
color: ColorStopColor,
Copy link
Contributor

Choose a reason for hiding this comment

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

I would comment in what scenarios we can have a null color, or otherwise change the typing to make that super clear like

$ReadOnlyArray<{
  colorStop | transitionHint
}>

I think they are similar enough to just roll with the comment but it should be clear that that syntax is supported

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a comment. Thanks!

position: number,
}>,
};
Expand All @@ -49,33 +51,52 @@ export default function processBackgroundImage(
} else if (Array.isArray(backgroundImage)) {
for (const bgImage of backgroundImage) {
const processedColorStops: Array<{
color: ProcessedColorValue,
color: ColorStopColor,
position: number | null,
}> = [];
for (let index = 0; index < bgImage.colorStops.length; index++) {
const colorStop = bgImage.colorStops[index];
const processedColor = processColor(colorStop.color);
if (processedColor == null) {
// If a color is invalid, return an empty array and do not apply gradient. Same as web.
return [];
}
if (colorStop.positions != null && colorStop.positions.length > 0) {
for (const position of colorStop.positions) {
if (position.endsWith('%')) {
processedColorStops.push({
color: processedColor,
position: parseFloat(position) / 100,
});
} else {
// If a position is invalid, return an empty array and do not apply gradient. Same as web.
return [];
}
const positions = colorStop.positions;
// Color transition hint syntax (red, 20%, blue)
if (
colorStop.color == null &&
Array.isArray(positions) &&
positions.length === 1
) {
const position = positions[0];
if (typeof position === 'string' && position.endsWith('%')) {
processedColorStops.push({
color: null,
position: parseFloat(position) / 100,
});
} else {
// If a position is invalid, return an empty array and do not apply gradient. Same as web.
return [];
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can change this to share some code better. Do transition hints have to be percentages? Regardless we are doing a lot of the same things, can you try to get rid of the top level if/else here and share some of this logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do transition hints have to be percentages?

They are actually length-percentage in the spec. But i only added % support for now in color stops. Linear gradient support mixing % and px units which require us to know the dimension, for this we'll have to move color stop parsing to native side. I wanted to avoid that change until the new css parser.

#48410 (comment)

can you try to get rid of the top level if/else here and share some of this logic?

i kinda prefer the explicit checks, also each branch has a tiny comment on top, but i will take another look!

}
} else {
processedColorStops.push({
color: processedColor,
position: null,
});
const processedColor = processColor(colorStop.color);
if (processedColor == null) {
// If a color is invalid, return an empty array and do not apply gradient. Same as web.
return [];
}
if (positions != null && positions.length > 0) {
for (const position of positions) {
if (position.endsWith('%')) {
processedColorStops.push({
color: processedColor,
position: parseFloat(position) / 100,
});
} else {
// If a position is invalid, return an empty array and do not apply gradient. Same as web.
return [];
}
}
} else {
processedColorStops.push({
color: processedColor,
position: null,
});
}
}
}

Expand Down Expand Up @@ -169,47 +190,85 @@ function parseCSSLinearGradient(
// If first part is not an angle/direction or a color stop, return an empty array and do not apply any gradient. Same as web.
return [];
}
colorStopRegex.lastIndex = 0;

const colorStopsString = parts.join(',');
const colorStops = [];
const fullColorStopsStr = parts.join(',');
let colorStopMatch;
while ((colorStopMatch = colorStopRegex.exec(fullColorStopsStr))) {
const [, color, position1, position2] = colorStopMatch;
const processedColor = processColor(color.trim().toLowerCase());
if (processedColor == null) {
// If a color is invalid, return an empty array and do not apply any gradient. Same as web.
// split by comma, but not if it's inside a parentheses. e.g. red, rgba(0, 0, 0, 0.5), green => ["red", "rgba(0, 0, 0, 0.5)", "green"]
const stops = colorStopsString.split(/,(?![^(]*\))/);
for (const stop of stops) {
const trimmedStop = stop.trim().toLowerCase();
// Match function like pattern or single words
const colorStopParts = trimmedStop.match(/\S+\([^)]*\)|\S+/g);
if (colorStopParts == null) {
// If a color stop is invalid, return an empty array and do not apply any gradient. Same as web.
return [];
}

if (typeof position1 !== 'undefined') {
if (position1.endsWith('%')) {
// Case 1: [color, position, position]
if (colorStopParts.length === 3) {
const color = colorStopParts[0];
const position1 = colorStopParts[1];
const position2 = colorStopParts[2];
const processedColor = processColor(color);
if (processedColor == null) {
// If a color is invalid, return an empty array and do not apply any gradient. Same as web.
return [];
}
if (position1.endsWith('%') && position2.endsWith('%')) {
colorStops.push({
color: processedColor,
position: parseFloat(position1) / 100,
});
colorStops.push({
color: processedColor,
position: parseFloat(position2) / 100,
});
} else {
// If a position is invalid, return an empty array and do not apply any gradient. Same as web.
return [];
}
} else {
colorStops.push({
color: processedColor,
position: null,
});
}

if (typeof position2 !== 'undefined') {
if (position2.endsWith('%')) {
// Case 2: [color, position]
else if (colorStopParts.length === 2) {
const color = colorStopParts[0];
const position = colorStopParts[1];
const processedColor = processColor(color);
if (processedColor == null) {
// If a color is invalid, return an empty array and do not apply any gradient. Same as web.
return [];
}
if (position.endsWith('%')) {
colorStops.push({
color: processedColor,
position: parseFloat(position2) / 100,
position: parseFloat(position) / 100,
});
} else {
// If a position is invalid, return an empty array and do not apply any gradient. Same as web.
return [];
}
}
// Case 3: [color]
// Case 4: [position] => transition hint syntax
else if (colorStopParts.length === 1) {
if (colorStopParts[0].endsWith('%')) {
colorStops.push({
color: null,
position: parseFloat(colorStopParts[0]) / 100,
});
} else {
const processedColor = processColor(colorStopParts[0]);
if (processedColor == null) {
// If a color is invalid, return an empty array and do not apply any gradient. Same as web.
return [];
}
colorStops.push({
color: processedColor,
position: null,
});
}
} else {
// If a color stop is invalid, return an empty array and do not apply any gradient. Same as web.
return [];
}
}

const fixedColorStops = getFixedColorStops(colorStops);
Expand Down Expand Up @@ -286,15 +345,15 @@ function getAngleInDegrees(angle?: string): ?number {
// https://drafts.csswg.org/css-images-4/#color-stop-fixup
function getFixedColorStops(
colorStops: $ReadOnlyArray<{
color: ProcessedColorValue,
color: ColorStopColor,
position: number | null,
}>,
): Array<{
color: ProcessedColorValue,
color: ColorStopColor,
position: number,
}> {
let fixedColorStops: Array<{
color: ProcessedColorValue,
color: ColorStopColor,
position: number,
}> = [];
let hasNullPositions = false;
Expand Down
3 changes: 2 additions & 1 deletion packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8375,11 +8375,12 @@ exports[`public API should not change unintentionally Libraries/StyleSheet/proce
"type LinearGradientDirection =
| { type: \\"angle\\", value: number }
| { type: \\"keyword\\", value: string };
type ColorStopColor = ProcessedColorValue | null;
type ParsedGradientValue = {
type: \\"linearGradient\\",
direction: LinearGradientDirection,
colorStops: $ReadOnlyArray<{
color: ProcessedColorValue,
color: ColorStopColor,
position: number,
}>,
};
Expand Down
Loading
Loading