From 80c0d0817906a264e6e9ef1d99fbe50715ad9e9c Mon Sep 17 00:00:00 2001 From: liuly Date: Wed, 9 Sep 2026 23:04:32 +0800 Subject: [PATCH] Fix zoom property crash after deleting a stop (#2153) ## Launch Checklist When a fixed paint value is converted to a zoom function, Maputnik creates exactly two stops. image Deleting either one of those two stops leaves a single remaining value. During the transition back to the ordinary field editor, `ZoomProperty` can render with `stops` undefined. It previously called `.map` unconditionally, causing the editor to crash. image This PR guards the stop list with optional chaining. The change only protects the render path and does not alter zoom-function conversion or value persistence. Now, the remaining stop's value will be restored after deleting the other stop: image The regression test covers `circle-opacity`: 1. Convert a fixed value to a zoom function. 2. Change the second stop's value. 3. Delete the first of the two stops. 4. Verify that the ordinary field editor reappears. 5. Verify that the remaining stop's value is restored. 6. Edit the restored field and verify that the style updates successfully. - [x] Briefly describe the changes in this PR. - [ ] Link to related issues. - [x] Include before/after visuals or gifs if this PR includes visual changes. - [x] Write tests for all new functionality. - [x] Add an entry to `CHANGELOG.md` under the `## main` section. --- CHANGELOG.md | 2 ++ e2e/layer-editor.spec.ts | 17 ++++++++++++++++- src/components/FieldFunction.tsx | 18 ++++++------------ 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2a3232a..70826293 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ - _...Add new stuff here..._ ### 🐞 Bug fixes +- Prevent a crash when converting a function to an expression +- Prevent a crash when deleting either stop from a function with two stops - Preserve expanded layer groups when deleting layers, including the first layer of a group - The map's data listener now fires on tile loads again, so source and vector layer field autocompletion is populated - The `maputnik` desktop binary now opens the default browser automatically on startup (opt out with `--no-browser`) diff --git a/e2e/layer-editor.spec.ts b/e2e/layer-editor.spec.ts index 4c2c49ce..8b35ef9d 100644 --- a/e2e/layer-editor.spec.ts +++ b/e2e/layer-editor.spec.ts @@ -289,6 +289,20 @@ describe("layer editor", () => { }); }); + test("should leave a plain editable value after deleting a stop", async () => { + await when.setFunctionStopValue("circle-radius", "Output value", 1, "0"); + await when.deleteFunctionStop("circle-radius"); + await then(get.elementByTestId("spec-field:circle-radius")).shouldBeVisible(); + await then(get.styleFromLocalStorage()).shouldDeepNestedInclude({ + layers: [{ id, paint: { "circle-radius": 0 } }], + }); + + await when.setValue("spec-field-input:circle-radius", "7"); + await then(get.styleFromLocalStorage()).shouldDeepNestedInclude({ + layers: [{ id, paint: { "circle-radius": 7 } }], + }); + }); + test("should set the base", async () => { await when.setFunctionBase("circle-radius", "2"); await then(get.styleFromLocalStorage()).shouldDeepNestedInclude({ @@ -310,8 +324,9 @@ describe("layer editor", () => { }); }); - test("should convert to an expression", async () => { + test("should convert to an expression without crashing", async () => { await when.makeExpression("circle-radius"); + await then(get.element("[data-wd-key='spec-field-container:circle-radius'] .maputnik-expression-editor")).shouldBeVisible(); await then(get.styleFromLocalStorage()).shouldDeepNestedInclude({ layers: [{ id, paint: { "circle-radius": ["interpolate", ["linear"], ["zoom"], 6, 5, 10, 5] } }], }); diff --git a/src/components/FieldFunction.tsx b/src/components/FieldFunction.tsx index eb995477..3a761520 100644 --- a/src/components/FieldFunction.tsx +++ b/src/components/FieldFunction.tsx @@ -129,16 +129,10 @@ type FieldFunctionProps = { * https://www.mapbox.com/mapbox-gl-style-spec/#types-function-zoom-property */ export const FieldFunction: React.FC = (props) => { - const [dataType, setDataType] = React.useState( - getDataType(props.value, props.fieldSpec) - ); const [isEditing, setIsEditing] = React.useState(false); - - React.useEffect(() => { - if (!isEditing) { - setDataType(getDataType(props.value, props.fieldSpec)); - } - }, [props.value, props.fieldSpec, isEditing]); + // Keep the expression editor mounted while typing, but otherwise select the + // editor from the current value so a collapsed function never renders as stops. + const dataType = isEditing ? "expression" : getDataType(props.value, props.fieldSpec); const getFieldFunctionType = (fieldSpec: any) => { if (fieldSpec.expression.interpolated) { @@ -173,7 +167,7 @@ export const FieldFunction: React.FC = (props) => { const deleteExpression = () => { const { fieldSpec, fieldName } = props; props.onChange(fieldName, fieldSpec.default); - setDataType("value"); + setIsEditing(false); }; const deleteStop = (stopIdx: number) => { @@ -233,10 +227,10 @@ export const FieldFunction: React.FC = (props) => { type: "identity", property: value[1], }); - setDataType("value"); + setIsEditing(false); } else if (isLiteralExpression(value)) { props.onChange(fieldName, value[1]); - setDataType("value"); + setIsEditing(false); } };