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

fix(web): style code condition values need to be wrapped #1295

Merged
merged 2 commits into from
Nov 29, 2024
Merged

fix(web): style code condition values need to be wrapped #1295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2c8ca50
fix: wrap values for style conditions
airslice Nov 29, 2024
5b608ca
refactor: update test & clean up
airslice Nov 29, 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
30 changes: 26 additions & 4 deletions web/src/beta/features/Editor/Map/LayerStylePanel/Editor/StyleInterface/convert.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ describe("convertToLayerStyleValue", () => {

describe("parseStyleValue", () => {
it("should parse style value and return value as value type", () => {
const value = parseStyleValue("clamp");
const value = parseStyleValue("select", "clamp");

expect(value).toEqual({
valueType: "value",
Expand All @@ -190,7 +190,9 @@ describe("parseStyleValue", () => {
});
});
it("should parse style value and return expression as value type", () => {
const value = parseStyleValue({ expression: "color('#ffffff', 0.8)" });
const value = parseStyleValue("color", {
expression: "color('#ffffff', 0.8)"
});

expect(value).toEqual({
valueType: "expression",
Expand All @@ -203,7 +205,9 @@ describe("parseStyleValue", () => {

describe("parseConditions", () => {
it("should parse conditions correctly", () => {
const conditions = parseConditions([["${marker-size}==='medium'", "12"]]);
const conditions = parseConditions("number", [
["${marker-size}==='medium'", "12"]
]);
airslice marked this conversation as resolved.
Show resolved Hide resolved

expect(conditions).toEqual([
{
Expand All @@ -216,6 +220,21 @@ describe("parseConditions", () => {
});
});

it("should parse conditions with URL values correctly", () => {
const conditions = parseConditions("model", [
["${type}==='1'", "'https://example.com/model.glb'"]
]);

expect(conditions).toEqual([
{
variable: "${type}",
operator: "===",
value: "'1'",
applyValue: "https://example.com/model.glb"
}
]);
});
airslice marked this conversation as resolved.
Show resolved Hide resolved

describe("generateStyleValue", () => {
it("should generate style value'", () => {
expect(generateStyleValue(mockStyleNodes?.model[0])).toEqual(
Expand All @@ -225,7 +244,10 @@ describe("generateStyleValue", () => {
});

describe("generateConditions", () => {
const conditions = generateConditions(mockStyleNodes?.marker[0].conditions);
const conditions = generateConditions(
"number",
mockStyleNodes?.marker[0].conditions
);

it("should generate conditions'", () => {
expect(conditions).toEqual([
Expand Down
121 changes: 110 additions & 11 deletions web/src/beta/features/Editor/Map/LayerStylePanel/Editor/StyleInterface/convert.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import {
StyleValue,
ExpressionCondition,
Expression,
StyleValueType
StyleValueType,
AppearanceField
} from "./types";

export const convertToStyleNodes = (
Expand All @@ -24,6 +25,7 @@ export const convertToStyleNodes = (
.map(([k, v]: [string, unknown]) => {
const nodeRef = appearanceNodes[cur].find((n) => n.id === k);
const { valueType, value, expression, conditions } = parseStyleValue(
nodeRef?.field,
v as StyleValue
);
return {
Expand Down Expand Up @@ -100,16 +102,24 @@ export const checkExpressionAndConditions = (
return "value";
};

export const parseStyleValue = (v: StyleValue) => {
export const parseStyleValue = (
field: AppearanceField | undefined,
v: StyleValue
) => {
const valueType = checkExpressionAndConditions(v);
return {
valueType,
value: valueType === "value" ? v : undefined,
expression:
valueType === "expression" ? (v as Expression).expression : undefined,
valueType === "expression"
? unwrapExpression(field, (v as Expression).expression)
: undefined,
conditions:
valueType === "conditions"
? parseConditions((v as ExpressionCondition).expression.conditions)
? parseConditions(
field,
(v as ExpressionCondition).expression.conditions
)
: undefined
};
};
Expand Down Expand Up @@ -137,15 +147,20 @@ export const generateStyleValue = (node: StyleNode) => {
return node.value ?? "";
}
if (node.valueType === "expression") {
return { expression: node.expression ?? "" };
return { expression: wrapExpression(node.field, node.expression ?? "") };
}
if (node.valueType === "conditions") {
return { expression: { conditions: generateConditions(node.conditions) } };
return {
expression: {
conditions: generateConditions(node.field, node.conditions)
}
};
}
return undefined;
};

export const parseConditions = (
field: AppearanceField | undefined,
conditions: [string, string][]
airslice marked this conversation as resolved.
Show resolved Hide resolved
): StyleCondition[] => {
const operatorRegex = new RegExp(
Expand All @@ -166,7 +181,7 @@ export const parseConditions = (
variable,
operator,
value,
applyValue: unwrapColor(applyValue)
applyValue: unwrapConditionAppliedValue(field, applyValue)
};
}
return null;
Expand All @@ -175,29 +190,113 @@ export const parseConditions = (
};

export const generateConditions = (
field: AppearanceField,
conditions?: StyleCondition[]
): [string, string][] => {
if (!conditions) return [];
return conditions.map((c) => {
return [
`${c.variable} ${c.operator} ${c.value}`,
wrapColor((c.applyValue ?? "").toString())
wrapConditionApplyValue((c.applyValue ?? "").toString(), field)
];
});
};

export const wrapConditionApplyValue = (
value: string,
field: AppearanceField
) => {
switch (field) {
case "color":
return wrapColor(value);
case "image":
case "text":
case "model":
return wrapString(value);
default:
return value;
}
};

export const unwrapConditionAppliedValue = (
field: AppearanceField | undefined,
value: string
) => {
switch (field) {
case "color":
return unwrapColor(value);
case "image":
case "text":
case "model":
return unwrapString(value);
default:
return value;
}
};

export const wrapExpression = (field: AppearanceField, expression: string) => {
if (/^\${.+}$/.test(expression)) {
return expression;
}
switch (field) {
case "color":
return wrapColor(expression);
case "image":
case "text":
case "model":
return wrapString(expression);
default:
return expression;
}
};

export const unwrapExpression = (
field: AppearanceField | undefined,
expression: string
) => {
if (/^\${.+}$/.test(expression)) {
return expression;
}
switch (field) {
case "color":
return unwrapColor(expression);
case "image":
case "text":
case "model":
return unwrapString(expression);
default:
return expression;
}
};

export const wrapColor = (maybeColor: string) => {
// check if color is a valid hex string
if (/^#[0-9A-F]{3}|[0-9A-F]{4}|[0-9A-F]{6}|[0-9A-F]{8}$/i.test(maybeColor)) {
if (
/^#(?:[0-9A-F]{3}|[0-9A-F]{4}|[0-9A-F]{6}|[0-9A-F]{8})$/i.test(maybeColor)
) {
return `color('${maybeColor}')`;
}
return maybeColor;
};

export const unwrapColor = (maybeWrappedColor: string) => {
// check if color is wrapped with color()
if (/^color\('.+'\)$/.test(maybeWrappedColor)) {
return maybeWrappedColor.slice(7, -2);
}
return maybeWrappedColor;
};

export const wrapString = (url: string) => {
if (!url) return url;
if (/^'.+'$/.test(url)) {
return url;
}
return `'${url}'`;
};

export const unwrapString = (maybeWrappedString: string) => {
if (!maybeWrappedString) return maybeWrappedString;
if (/^'.+'$/.test(maybeWrappedString)) {
return maybeWrappedString.slice(1, -1);
}
return maybeWrappedString;
};
airslice marked this conversation as resolved.
Show resolved Hide resolved
Loading