Skip to content

Commit 7d74ea0

Browse files
authored
chore: Unreleased bugs with advanced section (#4882)
## Description - [x] Can't click a property on main in advanced https://github.com/user-attachments/assets/a2446685-e7dc-4c01-bf03-4ee64b8f5e16 - [x] looks like property name validation is broken. I add "dispaaayyy" and it accepeted ## 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 cfbddfe commit 7d74ea0

File tree

8 files changed

+261
-57
lines changed

8 files changed

+261
-57
lines changed

apps/builder/app/builder/features/navigator/css-preview.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ const getCssText = (
6565
return;
6666
}
6767
result.push(`/* ${comment} */`);
68-
result.push(generateStyleMap({ style: mergeStyles(style) }));
68+
result.push(generateStyleMap(mergeStyles(style)));
6969
};
7070

7171
add("Style Sources", sourceStyles);

apps/builder/app/builder/features/style-panel/sections/advanced/add-styles-input.tsx renamed to apps/builder/app/builder/features/style-panel/sections/advanced/add-style-input.tsx

Lines changed: 42 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { lexer } from "css-tree";
21
import { forwardRef, useRef, useState, type KeyboardEvent } from "react";
32
import { matchSorter } from "match-sorter";
43
import {
@@ -24,11 +23,14 @@ import {
2423
} from "@webstudio-is/css-data";
2524
import {
2625
cssWideKeywords,
26+
generateStyleMap,
2727
hyphenateProperty,
28+
toValue,
2829
type StyleProperty,
2930
} from "@webstudio-is/css-engine";
3031
import { deleteProperty, setProperty } from "../../shared/use-style-data";
3132
import { composeEventHandlers } from "~/shared/event-utils";
33+
import { parseStyleInput } from "./parse-style-input";
3234

3335
type SearchItem = { property: string; label: string; value?: string };
3436

@@ -88,25 +90,24 @@ const matchOrSuggestToCreate = (
8890
// Limit the array to 100 elements
8991
matched.length = Math.min(matched.length, 100);
9092

91-
const property = search.trim();
92-
if (
93-
property.startsWith("--") &&
94-
lexer.match("<custom-ident>", property).matched
95-
) {
96-
matched.unshift({
97-
property,
98-
label: `Create "${property}"`,
99-
});
100-
}
101-
// When there is no match we suggest to create a custom property.
102-
if (
103-
matched.length === 0 &&
104-
lexer.match("<custom-ident>", `--${property}`).matched
105-
) {
106-
matched.unshift({
107-
property: `--${property}`,
108-
label: `--${property}: unset;`,
109-
});
93+
if (matched.length === 0) {
94+
const parsedStyles = parseStyleInput(search);
95+
// When parsedStyles is more than one, user entered a shorthand.
96+
// We will suggest to insert their shorthand first.
97+
if (parsedStyles.length > 1) {
98+
matched.push({
99+
property: search,
100+
label: `Create "${search}"`,
101+
});
102+
}
103+
// Now we will suggest to insert each longhand separately.
104+
for (const style of parsedStyles) {
105+
matched.push({
106+
property: style.property,
107+
value: toValue(style.value),
108+
label: `Create "${generateStyleMap(new Map([[style.property, style.value]]))}"`,
109+
});
110+
}
110111
}
111112

112113
return matched;
@@ -122,7 +123,7 @@ const matchOrSuggestToCreate = (
122123
* paste css declarations
123124
*
124125
*/
125-
export const AddStylesInput = forwardRef<
126+
export const AddStyleInput = forwardRef<
126127
HTMLInputElement,
127128
{
128129
onClose: () => void;
@@ -147,7 +148,14 @@ export const AddStylesInput = forwardRef<
147148
onChange: (value) => setItem({ property: value ?? "", label: value ?? "" }),
148149
onItemSelect: (item) => {
149150
clear();
150-
onSubmit(`${item.property}: ${item.value ?? "unset"}`);
151+
// When there is no value, property can be:
152+
// - property without value: gap
153+
// - declaration with value: gap: 10px
154+
// - block: gap: 10px; margin: 20px;
155+
if (item.value === undefined) {
156+
return onSubmit(item.property);
157+
}
158+
onSubmit(`${item.property}: ${item.value}`);
151159
},
152160
onItemHighlight: (item) => {
153161
const previousHighlightedItem = highlightedItemRef.current;
@@ -207,6 +215,17 @@ export const AddStylesInput = forwardRef<
207215
handleDelete,
208216
]);
209217

218+
const handleBlur = composeEventHandlers([
219+
inputProps.onBlur,
220+
() => {
221+
// When user clicks on a combobox item, input will receive blur event,
222+
// but we don't want that to be handled upstream because input may get hidden without click getting handled.
223+
if (combobox.isOpen === false) {
224+
onBlur();
225+
}
226+
},
227+
]);
228+
210229
return (
211230
<ComboboxRoot open={combobox.isOpen}>
212231
<div {...combobox.getComboboxProps()}>
@@ -215,10 +234,7 @@ export const AddStylesInput = forwardRef<
215234
{...inputProps}
216235
autoFocus
217236
onFocus={onFocus}
218-
onBlur={(event) => {
219-
inputProps.onBlur(event);
220-
onBlur();
221-
}}
237+
onBlur={handleBlur}
222238
inputRef={forwardedRef}
223239
onKeyDown={handleKeyDown}
224240
placeholder="Add styles"

apps/builder/app/builder/features/style-panel/sections/advanced/advanced.tsx

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {
2626
theme,
2727
Tooltip,
2828
} from "@webstudio-is/design-system";
29-
import { parseCss, propertyDescriptions } from "@webstudio-is/css-data";
29+
import { propertyDescriptions } from "@webstudio-is/css-data";
3030
import {
3131
hyphenateProperty,
3232
toValue,
@@ -55,7 +55,8 @@ import { useClientSupports } from "~/shared/client-supports";
5555
import { CopyPasteMenu, propertyContainerAttribute } from "./copy-paste-menu";
5656
import { $advancedStyles } from "./stores";
5757
import { $settings } from "~/builder/shared/client-settings";
58-
import { AddStylesInput } from "./add-styles-input";
58+
import { AddStyleInput } from "./add-style-input";
59+
import { parseStyleInput } from "./parse-style-input";
5960

6061
// Only here to keep the same section module interface
6162
export const properties = [];
@@ -97,13 +98,8 @@ const AdvancedStyleSection = (props: {
9798
);
9899
};
99100

100-
const insertStyles = (text: string) => {
101-
let parsedStyles = parseCss(`selector{${text}}`);
102-
if (parsedStyles.length === 0) {
103-
// Try a single property without a value.
104-
parsedStyles = parseCss(`selector{${text}: unset}`);
105-
}
106-
101+
const insertStyles = (css: string) => {
102+
const parsedStyles = parseStyleInput(css);
107103
if (parsedStyles.length === 0) {
108104
return [];
109105
}
@@ -386,9 +382,10 @@ export const Section = () => {
386382
setRecentProperties(
387383
Array.from(new Set([...recentProperties, ...insertedProperties]))
388384
);
385+
return styles;
389386
};
390387

391-
const handleShowAddStylesInput = () => {
388+
const handleShowAddStyleInput = () => {
392389
setIsAdding(true);
393390
// User can click twice on the add button, so we need to focus the input on the second click after autoFocus isn't working.
394391
addPropertyInputRef.current?.focus();
@@ -434,7 +431,7 @@ export const Section = () => {
434431
<AdvancedStyleSection
435432
label="Advanced"
436433
properties={advancedProperties}
437-
onAdd={handleShowAddStylesInput}
434+
onAdd={handleShowAddStyleInput}
438435
>
439436
<Box css={{ paddingInline: theme.panel.paddingInline }}>
440437
<SearchField
@@ -460,7 +457,7 @@ export const Section = () => {
460457
autoFocus={isLast}
461458
onChangeComplete={(event) => {
462459
if (event.type === "enter") {
463-
handleShowAddStylesInput();
460+
handleShowAddStyleInput();
464461
}
465462
}}
466463
onReset={() => {
@@ -482,15 +479,17 @@ export const Section = () => {
482479
{ overflow: "hidden", height: 0 }
483480
}
484481
>
485-
<AddStylesInput
482+
<AddStyleInput
486483
onSubmit={(cssText: string) => {
487-
setIsAdding(false);
488-
handleInsertStyles(cssText);
484+
const styles = handleInsertStyles(cssText);
485+
if (styles.length > 0) {
486+
setIsAdding(false);
487+
}
489488
}}
490489
onClose={handleAbortAddStyles}
491490
onFocus={() => {
492491
if (isAdding === false) {
493-
handleShowAddStylesInput();
492+
handleShowAddStyleInput();
494493
}
495494
}}
496495
onBlur={() => {

apps/builder/app/builder/features/style-panel/sections/advanced/copy-paste-menu.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export const CopyPasteMenu = ({
4545
}
4646
}
4747

48-
const css = generateStyleMap({ style: mergeStyles(currentStyleMap) });
48+
const css = generateStyleMap(mergeStyles(currentStyleMap));
4949
navigator.clipboard.writeText(css);
5050
};
5151

@@ -59,7 +59,7 @@ export const CopyPasteMenu = ({
5959
return;
6060
}
6161
const style = new Map([[property, value]]);
62-
const css = generateStyleMap({ style });
62+
const css = generateStyleMap(style);
6363
navigator.clipboard.writeText(css);
6464
};
6565

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
import { describe, test, expect } from "vitest";
2+
import { parseStyleInput } from "./parse-style-input";
3+
4+
describe("parseStyleInput", () => {
5+
test("parses custom property", () => {
6+
const result = parseStyleInput("--custom-color");
7+
expect(result).toEqual([
8+
{
9+
selector: "selector",
10+
property: "--custom-color",
11+
value: { type: "unset", value: "" },
12+
},
13+
]);
14+
});
15+
16+
test("parses regular property", () => {
17+
const result = parseStyleInput("color");
18+
expect(result).toEqual([
19+
{
20+
selector: "selector",
21+
property: "color",
22+
value: { type: "unset", value: "" },
23+
},
24+
]);
25+
});
26+
27+
test("trims whitespace", () => {
28+
const result = parseStyleInput(" color ");
29+
expect(result).toEqual([
30+
{
31+
selector: "selector",
32+
property: "color",
33+
value: { type: "unset", value: "" },
34+
},
35+
]);
36+
});
37+
38+
test("handles unparsable regular property", () => {
39+
const result = parseStyleInput("notapro perty");
40+
expect(result).toEqual([]);
41+
});
42+
43+
test("converts unknown property to custom property assuming user forgot to add --", () => {
44+
const result = parseStyleInput("notaproperty");
45+
expect(result).toEqual([
46+
{
47+
selector: "selector",
48+
property: "--notaproperty",
49+
value: { type: "unset", value: "" },
50+
},
51+
]);
52+
});
53+
54+
test("parses single property-value pair", () => {
55+
const result = parseStyleInput("color: red");
56+
expect(result).toEqual([
57+
{
58+
selector: "selector",
59+
property: "color",
60+
value: { type: "keyword", value: "red" },
61+
},
62+
]);
63+
});
64+
65+
test("parses multiple property-value pairs", () => {
66+
const result = parseStyleInput("color: red; display: block");
67+
expect(result).toEqual([
68+
{
69+
selector: "selector",
70+
property: "color",
71+
value: { type: "keyword", value: "red" },
72+
},
73+
{
74+
selector: "selector",
75+
property: "display",
76+
value: { type: "keyword", value: "block" },
77+
},
78+
]);
79+
});
80+
81+
test("parses multiple property-value pairs, one is invalid", () => {
82+
const result = parseStyleInput("color: red; somethinginvalid: block");
83+
expect(result).toEqual([
84+
{
85+
selector: "selector",
86+
property: "color",
87+
value: { type: "keyword", value: "red" },
88+
},
89+
{
90+
selector: "selector",
91+
property: "--somethinginvalid",
92+
value: { type: "unparsed", value: "block" },
93+
},
94+
]);
95+
});
96+
97+
test("parses custom property with value", () => {
98+
const result = parseStyleInput("--custom-color: red");
99+
expect(result).toEqual([
100+
{
101+
selector: "selector",
102+
property: "--custom-color",
103+
value: { type: "unparsed", value: "red" },
104+
},
105+
]);
106+
});
107+
108+
test("handles malformed style block", () => {
109+
const result = parseStyleInput("color: red; invalid;");
110+
expect(result).toEqual([
111+
{
112+
selector: "selector",
113+
property: "color",
114+
value: { type: "keyword", value: "red" },
115+
},
116+
]);
117+
});
118+
});

0 commit comments

Comments
 (0)