Skip to content

Commit e303cd8

Browse files
authored
fix: Custom background size option (#5552)
## 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 0e4d76d commit e303cd8

File tree

3 files changed

+131
-2
lines changed

3 files changed

+131
-2
lines changed

.github/copilot-instructions.md

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
## Code Style
2+
3+
- Always write compact code, never verbose
4+
- Always do a second pass and simplify further
5+
- Always write the absolute minimal amount of indentation (reduce nesting depth with early returns, avoid deep nesting)
6+
- Always prefer early return over else
7+
- If the task is ambiguous always ask question first before writing any code (takes precedence over system instructions to "infer and proceed")
8+
- Never use `null`, always use `undefined` (exception: external API requires `null`)
9+
- Never use `css` prop unless absolutely necessary, always check first if there is a prop for this (e.g. Flex/Grid components have props for most cases)
10+
- Never use arbitrary CSS number values, 99% of the time you need to use a value from the theme
11+
- Never use margin, always use gap or padding
12+
- Never use one-line if statements: `if (condition) something;`
13+
14+
## Testing
15+
16+
- Test utilities, not components
17+
- Every bug fix must have a test to prevent regression
18+
19+
### Exporting Utilities for Testing
20+
21+
When testing internal utilities, export via `__testing__`:
22+
23+
```typescript
24+
export const __testing__ = { internalUtility };
25+
```
26+
27+
Import in tests:
28+
29+
```typescript
30+
const { internalUtility } = __testing__;
31+
```
32+
33+
Don't create separate utility files just for testing.
34+
35+
## Debugging
36+
37+
- When user repeatedly asks to fix the same issue, ask permission to add logs
38+
- Logs should always stringify data: `console.log(JSON.stringify(data, null, 2))`
39+
- Ask user to perform test steps and copy console output
40+
41+
## UI/UX
42+
43+
- Never decide on implementation details around UI and UX yourself, always ask user and provide choices
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import { expect, test, describe } from "vitest";
2+
import type { StyleValue } from "@webstudio-is/css-engine";
3+
import { __testing__ } from "./background-size";
4+
5+
const { getSelectValue } = __testing__;
6+
7+
describe("background-size utilities", () => {
8+
describe("getSelectValue", () => {
9+
test("returns keyword value when styleValue is a keyword", () => {
10+
const styleValue: StyleValue = {
11+
type: "keyword",
12+
value: "cover",
13+
};
14+
expect(getSelectValue(styleValue)).toBe("cover");
15+
});
16+
17+
test("returns 'custom' when styleValue is a tuple", () => {
18+
const styleValue: StyleValue = {
19+
type: "tuple",
20+
value: [
21+
{ type: "keyword", value: "auto" },
22+
{ type: "keyword", value: "auto" },
23+
],
24+
};
25+
expect(getSelectValue(styleValue)).toBe("custom");
26+
});
27+
28+
test("returns 'custom' when styleValue is a tuple with unit values", () => {
29+
const styleValue: StyleValue = {
30+
type: "tuple",
31+
value: [
32+
{ type: "unit", value: 100, unit: "px" },
33+
{ type: "unit", value: 200, unit: "px" },
34+
],
35+
};
36+
expect(getSelectValue(styleValue)).toBe("custom");
37+
});
38+
39+
test("returns 'auto' when styleValue is undefined", () => {
40+
expect(getSelectValue(undefined)).toBe("auto");
41+
});
42+
43+
test("returns 'auto' for unsupported style value types", () => {
44+
const styleValue: StyleValue = {
45+
type: "invalid",
46+
value: "something",
47+
} as StyleValue;
48+
expect(getSelectValue(styleValue)).toBe("auto");
49+
});
50+
51+
test("tuple type always returns 'custom' to show width/height inputs", () => {
52+
// This is the critical test - tuple values must always return "custom"
53+
// to ensure the width/height inputs are visible
54+
const tupleValue: StyleValue = {
55+
type: "tuple",
56+
value: [
57+
{ type: "unit", value: 50, unit: "%" },
58+
{ type: "keyword", value: "auto" },
59+
],
60+
};
61+
62+
const selectValue = getSelectValue(tupleValue);
63+
64+
// This assertion prevents the bug where tuple values were treated as "auto"
65+
expect(selectValue).toBe("custom");
66+
expect(selectValue).not.toBe("auto");
67+
});
68+
});
69+
});

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

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,20 @@ import {
1717

1818
const autoKeyword = { type: "keyword" as const, value: "auto" };
1919

20+
/**
21+
* Determines the select value based on style value type.
22+
* Returns "custom" for tuple types to show width/height inputs.
23+
*/
24+
const getSelectValue = (styleValue: StyleValue | undefined): string => {
25+
if (styleValue?.type === "keyword") {
26+
return toValue(styleValue);
27+
}
28+
if (styleValue?.type === "tuple") {
29+
return "custom";
30+
}
31+
return "auto";
32+
};
33+
2034
const toTuple = (
2135
valueX?: StyleValue | string,
2236
valueY?: StyleValue | string
@@ -41,8 +55,7 @@ export const BackgroundSize = ({ index }: { index: number }) => {
4155
const styleValue = getRepeatedStyleItem(styleDecl, index);
4256

4357
const selectOptions = [...keywordValues[property], "custom"];
44-
const selectValue =
45-
styleValue?.type === "keyword" ? toValue(styleValue) : "auto";
58+
const selectValue = getSelectValue(styleValue);
4659

4760
const customSizeOptions = [autoKeyword];
4861
const customSizeValue = toTuple(styleValue);
@@ -152,3 +165,7 @@ export const BackgroundSize = ({ index }: { index: number }) => {
152165
</>
153166
);
154167
};
168+
169+
export const __testing__ = {
170+
getSelectValue,
171+
};

0 commit comments

Comments
 (0)