Skip to content

Commit 5239670

Browse files
authored
fix: Multiple gradient fixes (#5464)
## Description 1. What is this PR about (link the issue and add a short description) ## Steps for reproduction 1. click button 2. expect xyz ## Code Review - [ ] hi @kof, I need you to do - conceptual review (architecture, feature-correctness) - detailed review (read every line) - test it on preview ## Before requesting a review - [ ] made a self-review - [ ] added inline comments where things may be not obvious (the "why", not "what") ## Before merging - [ ] tested locally and on preview environment (preview dev login: 0000) - [ ] updated [test cases](https://github.com/webstudio-is/webstudio/blob/main/apps/builder/docs/test-cases.md) document - [ ] added tests - [ ] if any new env variables are added, added them to `.env` file
1 parent 7139a35 commit 5239670

File tree

4 files changed

+88
-14
lines changed

4 files changed

+88
-14
lines changed

apps/builder/app/builder/features/style-panel/sections/backgrounds/background-gradient.tsx

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,13 @@ export const BackgroundGradient = ({
142142
}) => {
143143
const styleDecl = useComputedStyleDecl("background-image");
144144
let styleValue = styleDecl.cascadedValue;
145+
let computedStyleValue = styleDecl.computedValue;
145146
if (styleValue.type === "layers") {
146147
styleValue = styleValue.value[index];
147148
}
149+
if (computedStyleValue.type === "layers") {
150+
computedStyleValue = computedStyleValue.value[index];
151+
}
148152

149153
const gradientString = toValue(styleValue);
150154
const { normalizedGradientString, initialIsRepeating } =
@@ -157,6 +161,17 @@ export const BackgroundGradient = ({
157161
return ensureGradientHasStops(parsed);
158162
}, [gradientType, normalizedGradientString]);
159163

164+
const computedGradientString = toValue(computedStyleValue);
165+
const { normalizedGradientString: normalizedComputedGradientString } =
166+
normalizeGradientInput(computedGradientString, gradientType);
167+
168+
const computedParsedGradient = useMemo(() => {
169+
const parsed =
170+
parseAnyGradient(normalizedComputedGradientString) ??
171+
createDefaultGradient(gradientType);
172+
return ensureGradientHasStops(parsed);
173+
}, [gradientType, normalizedComputedGradientString]);
174+
160175
const handleGradientSave = useCallback(
161176
(nextGradient: ParsedGradient) => {
162177
const gradientValue = formatGradientValue(nextGradient);
@@ -249,6 +264,7 @@ export const BackgroundGradient = ({
249264
<>
250265
<GradientPickerSection
251266
gradient={gradient}
267+
computedGradient={computedParsedGradient}
252268
gradientType={gradientType}
253269
hintOverrides={hintOverrides}
254270
setHintOverrides={setHintOverrides}
@@ -287,6 +303,7 @@ export const BackgroundGradient = ({
287303

288304
type GradientPickerSectionProps = {
289305
gradient: ParsedGradient;
306+
computedGradient: ParsedGradient;
290307
gradientType: GradientType;
291308
hintOverrides: Map<number, PercentUnitValue>;
292309
setHintOverrides: Dispatch<SetStateAction<Map<number, PercentUnitValue>>>;
@@ -297,16 +314,18 @@ type GradientPickerSectionProps = {
297314

298315
const GradientPickerSection = ({
299316
gradient,
317+
computedGradient,
300318
gradientType,
301319
hintOverrides,
302320
setHintOverrides,
303321
setSelectedStopIndex,
304322
applyGradient,
305323
selectedStopIndex,
306324
}: GradientPickerSectionProps) => {
307-
const gradientForPickerBase = useMemo(() => {
308-
return resolveGradientForPicker(gradient, hintOverrides);
309-
}, [gradient, hintOverrides]);
325+
// Use computed gradient for picker (resolves all CSS variables)
326+
const computedGradientForPicker = useMemo(() => {
327+
return resolveGradientForPicker(computedGradient, hintOverrides);
328+
}, [computedGradient, hintOverrides]);
310329

311330
const handlePickerChange = useCallback(
312331
(nextGradient: ParsedGradient) => {
@@ -329,20 +348,17 @@ const GradientPickerSection = ({
329348
[setSelectedStopIndex]
330349
);
331350

332-
// Gradient stops are already sorted in applyGradient, no need to sort here
333-
const gradientForPicker = gradientForPickerBase;
334-
335351
const previewGradientForTrack = useMemo<ParsedLinearGradient>(() => {
336352
const previewGradient: ParsedLinearGradient = {
337353
type: "linear",
338354
angle: leftToRightAngle,
339-
stops: gradientForPicker.stops,
355+
stops: computedGradientForPicker.stops,
340356
};
341-
if (gradientForPicker.repeating) {
357+
if (computedGradientForPicker.repeating) {
342358
previewGradient.repeating = true;
343359
}
344360
return previewGradient;
345-
}, [gradientForPicker]);
361+
}, [computedGradientForPicker]);
346362

347363
return (
348364
<Flex
@@ -352,7 +368,7 @@ const GradientPickerSection = ({
352368
css={{ padding: theme.panel.padding }}
353369
>
354370
<GradientPicker
355-
gradient={gradientForPicker}
371+
gradient={computedGradientForPicker}
356372
backgroundImage={formatLinearGradient(previewGradientForTrack)}
357373
type={gradientType}
358374
onChange={handlePickerChange}
@@ -362,6 +378,7 @@ const GradientPickerSection = ({
362378
/>
363379
<GradientStopControls
364380
gradient={gradient}
381+
computedGradient={computedGradient}
365382
selectedStopIndex={selectedStopIndex}
366383
setSelectedStopIndex={setSelectedStopIndex}
367384
hintOverrides={hintOverrides}
@@ -687,6 +704,7 @@ const SolidColorControls = ({
687704

688705
type GradientStopControlsProps = {
689706
gradient: ParsedGradient;
707+
computedGradient: ParsedGradient;
690708
selectedStopIndex: number;
691709
setSelectedStopIndex: Dispatch<SetStateAction<number>>;
692710
hintOverrides: Map<number, PercentUnitValue>;
@@ -696,6 +714,7 @@ type GradientStopControlsProps = {
696714

697715
const GradientStopControls = ({
698716
gradient,
717+
computedGradient,
699718
selectedStopIndex,
700719
setSelectedStopIndex,
701720
hintOverrides,
@@ -931,6 +950,9 @@ const GradientStopControls = ({
931950
};
932951

933952
const stopColor = (stop.color ?? fallbackStopColor) as StyleValue;
953+
// Use computed color for display (resolves CSS variables)
954+
const computedStop = computedGradient.stops[stopIndex];
955+
const currentColor = (computedStop?.color ?? stopColor) as StyleValue;
934956

935957
return (
936958
<Flex
@@ -987,7 +1009,7 @@ const GradientStopControls = ({
9871009
<ColorPickerControl
9881010
property="color"
9891011
value={stopColor}
990-
currentColor={stopColor}
1012+
currentColor={currentColor}
9911013
onChange={handleStopColorChange}
9921014
onChangeComplete={handleStopColorChangeComplete}
9931015
onAbort={() => {}}

apps/builder/app/builder/features/style-panel/shared/repeated-style.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,47 @@ test("unpack item from layers value in repeated style", () => {
300300
);
301301
});
302302

303+
test("set item on second layer when property is not defined on first layer", () => {
304+
// This tests the fix for the bug where setting a property like background-clip
305+
// on layer 1 when layer 0 doesn't have that property would create undefined values
306+
const $backgroundClip = createComputedStyleDeclStore("background-clip");
307+
308+
// Set two background images
309+
setRawProperty("background-image", "url(image1.jpg), url(image2.jpg)");
310+
311+
// background-clip is not yet defined, so cascadedValue should be default
312+
expect($backgroundClip.get().source.name).toEqual("default");
313+
314+
// Now set background-clip on the second layer (index 1)
315+
// This should fill index 0 with the initial value (border-box) instead of undefined
316+
setRepeatedStyleItem($backgroundClip.get(), 1, {
317+
type: "keyword",
318+
value: "padding-box",
319+
});
320+
321+
// Should have created layers with initial value at index 0
322+
expect(toValue($backgroundClip.get().cascadedValue)).toEqual(
323+
"border-box, padding-box"
324+
);
325+
expect($backgroundClip.get().cascadedValue.type).toEqual("layers");
326+
327+
// Verify the values are properly set and not undefined
328+
const cascadedValue = $backgroundClip.get().cascadedValue;
329+
if (cascadedValue.type === "layers") {
330+
expect(cascadedValue.value[0]).toEqual({
331+
type: "keyword",
332+
value: "border-box",
333+
});
334+
expect(cascadedValue.value[1]).toEqual({
335+
type: "keyword",
336+
value: "padding-box",
337+
});
338+
// Ensure no undefined values
339+
expect(cascadedValue.value[0]).toBeDefined();
340+
expect(cascadedValue.value[1]).toBeDefined();
341+
}
342+
});
343+
303344
describe("delete repeated item", () => {
304345
test("delete var or other not releated value in repeated style", () => {
305346
const $backgroundImage = createComputedStyleDeclStore("background-image");

apps/builder/app/builder/features/style-panel/shared/repeated-style.tsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,19 @@ export const setRepeatedStyleItem = (
226226
const value = styleDecl.cascadedValue;
227227
const valueType: ItemType = value.type === "tuple" ? "tuple" : "layers";
228228
const oldItems = value.type === valueType ? value.value : [];
229+
230+
// Fill missing items with initial value instead of undefined
229231
const newItems: StyleValue[] = repeatUntil(oldItems, index);
232+
if (oldItems.length === 0 && index > 0) {
233+
const meta = propertiesData[styleDecl.property];
234+
if (meta) {
235+
const initialValue = meta.initial as UnparsedValue;
236+
for (let i = 0; i < index; i++) {
237+
newItems[i] = initialValue;
238+
}
239+
}
240+
}
241+
230242
// unpack item when layers or tuple is provided
231243
newItems[index] = newItem.type === valueType ? newItem.value[0] : newItem;
232244
batch.setProperty(styleDecl.property)({

packages/design-system/src/components/gradient-picker.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -948,12 +948,10 @@ export const GradientPicker = <T extends ParsedGradient>({
948948
})}
949949

950950
{hints.map((hint) => {
951-
const stop = stops[hint.stopIndex];
952-
const hintColor = stop?.color ? toValue(stop.color) : undefined;
953951
return (
954952
<SliderHint
955953
key={`${hint.stopIndex}-${hint.value}`}
956-
style={{ left: `${hint.value}%`, color: hintColor }}
954+
style={{ left: `${hint.value}%` }}
957955
onPointerDown={handleHintPointerDown(hint.stopIndex)}
958956
>
959957
<ChevronFilledUpIcon size="100%" />
@@ -1055,6 +1053,7 @@ const SliderHint = styled(Flex, {
10551053
cursor: "grab",
10561054
touchAction: "none",
10571055
userSelect: "none",
1056+
color: "black",
10581057
"&:active": {
10591058
cursor: "grabbing",
10601059
},

0 commit comments

Comments
 (0)