From 3c3fcadbb613bdb87ac2f79485f98d5a81d5375b Mon Sep 17 00:00:00 2001 From: Harel M Date: Tue, 16 Sep 2025 16:42:07 +0300 Subject: [PATCH] Added back errors panel (#1384) ## Launch Checklist This PR adds back the error panel which was under the map for some reason. It also highlights problematic layers in the layers list (which already worked). It also highlights the field that has an error related to it. It fixes the error types throughout the code. Before: image After: image - [x] Briefly describe the changes in this PR. - [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. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- cypress/e2e/layers.cy.ts | 59 ++++++++++++++++---------- cypress/e2e/modals.cy.ts | 15 +++++++ src/components/App.tsx | 18 ++------ src/components/AppMessagePanel.tsx | 5 ++- src/components/Block.tsx | 9 ++-- src/components/FieldFunction.tsx | 3 +- src/components/FieldSpec.tsx | 10 ++--- src/components/Fieldset.tsx | 9 +++- src/components/FilterEditor.tsx | 4 +- src/components/InputSpec.tsx | 2 - src/components/LayerEditor.tsx | 6 +-- src/components/LayerList.tsx | 4 +- src/components/PropertyGroup.tsx | 3 +- src/components/_DataProperty.tsx | 3 +- src/components/_ExpressionProperty.tsx | 3 +- src/components/_ZoomProperty.tsx | 3 +- src/libs/definitions.d.ts | 16 +++++++ src/styles/_components.scss | 6 +++ src/styles/_layout.scss | 2 +- 19 files changed, 114 insertions(+), 66 deletions(-) diff --git a/cypress/e2e/layers.cy.ts b/cypress/e2e/layers.cy.ts index fa92dc52..531da62c 100644 --- a/cypress/e2e/layers.cy.ts +++ b/cypress/e2e/layers.cy.ts @@ -492,7 +492,7 @@ describe("layers", () => { then(get.elementByTestId("spec-field-doc")).shouldContainText("Offset distance"); }); - it.only("should hide spec info when clicking a second time", () => { + it("should hide spec info when clicking a second time", () => { when.modal.fillLayers({ type: "symbol", layer: "example", @@ -698,33 +698,48 @@ describe("layers", () => { }); }); - describe("layereditor jsonlint should error", ()=>{ - it("add", () => { - const id = when.modal.fillLayers({ - type: "circle", - layer: "example", + describe("layers editor", () => { + describe("property fields", () => { + it("should show error", () => { + when.modal.fillLayers({ + type: "circle", + layer: "invalid", + }); + + then(get.element(".maputnik-input-block--error .maputnik-input-block-label")).shouldHaveCss("color", "rgb(207, 74, 74)"); }); + }); - then(get.styleFromLocalStorage()).shouldDeepNestedInclude({ - layers: [ - { - id: id, - type: "circle", - source: "example", - }, - ], + describe("jsonlint should error", ()=>{ + it("add", () => { + const id = when.modal.fillLayers({ + type: "circle", + layer: "example", + }); + + then(get.styleFromLocalStorage()).shouldDeepNestedInclude({ + layers: [ + { + id: id, + type: "circle", + source: "example", + }, + ], + }); + + const sourceText = get.elementByText('"source"'); + + sourceText.click(); + sourceText.type("\""); + + const error = get.element(".CodeMirror-lint-marker-error"); + error.should("exist"); }); - - const sourceText = get.elementByText('"source"'); - - sourceText.click(); - sourceText.type("\""); - - const error = get.element(".CodeMirror-lint-marker-error"); - error.should("exist"); }); }); + + describe("drag and drop", () => { it("move layer should update local storage", () => { when.modal.open(); diff --git a/cypress/e2e/modals.cy.ts b/cypress/e2e/modals.cy.ts index 69517297..3ff7504c 100644 --- a/cypress/e2e/modals.cy.ts +++ b/cypress/e2e/modals.cy.ts @@ -382,6 +382,21 @@ describe("modals", () => { }); }); + describe("error panel", () => { + it("not visible when no errors", () => { + then(get.element("maputnik-message-panel-error")).shouldNotExist(); + }); + + it("visible on style error", () => { + when.modal.open(); + when.modal.fillLayers({ + type: "circle", + layer: "invalid", + }); + then(get.element(".maputnik-message-panel-error")).shouldBeVisible(); + }); + }); + describe("Handle localStorage QuotaExceededError", () => { it("handles quota exceeded error when opening style from URL", () => { // Clear localStorage to start fresh diff --git a/src/components/App.tsx b/src/components/App.tsx index 10d3b2d7..ee7e044f 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -36,7 +36,7 @@ import LayerWatcher from "../libs/layerwatcher"; import tokens from "../config/tokens.json"; import isEqual from "lodash.isequal"; import { type MapOptions } from "maplibre-gl"; -import { type OnStyleChangedOpts, type StyleSpecificationWithId } from "../libs/definitions"; +import { type MappedError, type OnStyleChangedOpts, type StyleSpecificationWithId } from "../libs/definitions"; // Buffer must be defined globally for @maplibre/maplibre-gl-style-spec validate() function to succeed. window.Buffer = buffer.Buffer; @@ -82,20 +82,8 @@ function updateRootSpec(spec: any, fieldName: string, newValues: any) { }; } -type MappedErrors = { - message: string - parsed?: { - type: string - data: { - index: number - key: string - message: string - } - } -}; - type AppState = { - errors: MappedErrors[], + errors: MappedError[], infos: string[], mapStyle: StyleSpecificationWithId, dirtyMapStyle?: StyleSpecification, @@ -383,7 +371,7 @@ export default class App extends React.Component { }); } - const mappedErrors = layerErrors.concat(errors).map(error => { + const mappedErrors: MappedError[] = layerErrors.concat(errors).map(error => { // Special case: Duplicate layer id const dupMatch = error.message.match(/layers\[(\d+)\]: (duplicate layer id "?(.*)"?, previously used)/); if (dupMatch) { diff --git a/src/components/AppMessagePanel.tsx b/src/components/AppMessagePanel.tsx index c2fe44e4..711d1d31 100644 --- a/src/components/AppMessagePanel.tsx +++ b/src/components/AppMessagePanel.tsx @@ -2,9 +2,10 @@ import React from "react"; import {formatLayerId} from "../libs/format"; import {type LayerSpecification, type StyleSpecification} from "maplibre-gl"; import { Trans, type WithTranslation, withTranslation } from "react-i18next"; +import { type MappedError } from "../libs/definitions"; type AppMessagePanelInternalProps = { - errors?: unknown[] + errors?: MappedError[] infos?: string[] mapStyle?: StyleSpecification onLayerSelect?(index: number): void; @@ -19,7 +20,7 @@ class AppMessagePanelInternal extends React.Component { + const errors = this.props.errors?.map((error, idx) => { let content; if (error.parsed && error.parsed.type === "layer") { const {parsed} = error; diff --git a/src/components/Block.tsx b/src/components/Block.tsx index d38d2af3..e83e4705 100644 --- a/src/components/Block.tsx +++ b/src/components/Block.tsx @@ -1,13 +1,13 @@ -import React, {type PropsWithChildren, type SyntheticEvent} from "react"; +import React, {type CSSProperties, type PropsWithChildren, type SyntheticEvent} from "react"; import classnames from "classnames"; import FieldDocLabel from "./FieldDocLabel"; import Doc from "./Doc"; -type BlockProps = PropsWithChildren & { +export type BlockProps = PropsWithChildren & { "data-wd-key"?: string label?: string action?: React.ReactElement - style?: object + style?: CSSProperties onChange?(...args: unknown[]): unknown fieldSpec?: object wideMode?: boolean @@ -66,7 +66,8 @@ export default class Block extends React.Component { className={classnames({ "maputnik-input-block": true, "maputnik-input-block--wide": this.props.wideMode, - "maputnik-action-block": this.props.action + "maputnik-action-block": this.props.action, + "maputnik-input-block--error": this.props.error })} onClick={this.onLabelClick} > diff --git a/src/components/FieldFunction.tsx b/src/components/FieldFunction.tsx index 1378f043..fa7d3f61 100644 --- a/src/components/FieldFunction.tsx +++ b/src/components/FieldFunction.tsx @@ -6,6 +6,7 @@ import ZoomProperty from "./_ZoomProperty"; import ExpressionProperty from "./_ExpressionProperty"; import {function as styleFunction} from "@maplibre/maplibre-gl-style-spec"; import {findDefaultFromSpec} from "../libs/spec-helper"; +import { type MappedLayerErrors } from "../libs/definitions"; function isLiteralExpression(value: any) { @@ -119,7 +120,7 @@ type FieldFunctionProps = { fieldName: string fieldType: string fieldSpec: any - errors?: {[key: string]: {message: string}} + errors?: MappedLayerErrors value?: any }; diff --git a/src/components/FieldSpec.tsx b/src/components/FieldSpec.tsx index 241984b3..0cec0f1d 100644 --- a/src/components/FieldSpec.tsx +++ b/src/components/FieldSpec.tsx @@ -1,6 +1,6 @@ -import Block from "./Block"; +import Block, { type BlockProps } from "./Block"; import InputSpec, { type FieldSpecType, type InputSpecProps } from "./InputSpec"; -import Fieldset from "./Fieldset"; +import Fieldset, { type FieldsetProps } from "./Fieldset"; function getElementFromType(fieldSpec: { type?: FieldSpecType, values?: unknown[] }): typeof Fieldset | typeof Block { switch(fieldSpec.type) { @@ -34,15 +34,13 @@ function getElementFromType(fieldSpec: { type?: FieldSpecType, values?: unknown[ } } -export type FieldSpecProps = InputSpecProps & { - name?: string -}; +export type FieldSpecProps = InputSpecProps & BlockProps & FieldsetProps; const FieldSpec: React.FC = (props) => { const TypeBlock = getElementFromType(props.fieldSpec!); return ( - + ); diff --git a/src/components/Fieldset.tsx b/src/components/Fieldset.tsx index 82b318b6..b0848edd 100644 --- a/src/components/Fieldset.tsx +++ b/src/components/Fieldset.tsx @@ -1,12 +1,14 @@ import React, { type PropsWithChildren, type ReactElement } from "react"; +import classnames from "classnames"; import FieldDocLabel from "./FieldDocLabel"; import Doc from "./Doc"; import generateUniqueId from "../libs/document-uid"; -type FieldsetProps = PropsWithChildren & { +export type FieldsetProps = PropsWithChildren & { label?: string, fieldSpec?: { doc?: string }, action?: ReactElement, + error?: {message: string} }; @@ -30,7 +32,10 @@ const Fieldset: React.FC = (props) => { )} {!props.fieldSpec && ( -
+
{props.label}
)} diff --git a/src/components/FilterEditor.tsx b/src/components/FilterEditor.tsx index b9767423..0db7bbc7 100644 --- a/src/components/FilterEditor.tsx +++ b/src/components/FilterEditor.tsx @@ -14,7 +14,7 @@ import InputButton from "./InputButton"; import Doc from "./Doc"; import ExpressionProperty from "./_ExpressionProperty"; import { type WithTranslation, withTranslation } from "react-i18next"; -import type { StyleSpecificationWithId } from "../libs/definitions"; +import type { MappedLayerErrors, StyleSpecificationWithId } from "../libs/definitions"; function combiningFilter(props: FilterEditorInternalProps): LegacyFilterSpecification | ExpressionSpecification { @@ -95,7 +95,7 @@ type FilterEditorInternalProps = { /** Properties of the vector layer and the available fields */ properties?: {[key:string]: any} filter?: any[] - errors?: {[key:string]: any} + errors?: MappedLayerErrors onChange(value: LegacyFilterSpecification | ExpressionSpecification): unknown } & WithTranslation; diff --git a/src/components/InputSpec.tsx b/src/components/InputSpec.tsx index e6a8481b..7eb439b5 100644 --- a/src/components/InputSpec.tsx +++ b/src/components/InputSpec.tsx @@ -31,7 +31,6 @@ export type InputSpecProps = { /** Override the style of the field */ style?: object "aria-label"?: string - error?: unknown[] label?: string action?: ReactElement }; @@ -43,7 +42,6 @@ export default class InputSpec extends React.Component { childNodes() { const commonProps = { - error: this.props.error, fieldSpec: this.props.fieldSpec, label: this.props.label, action: this.props.action, diff --git a/src/components/LayerEditor.tsx b/src/components/LayerEditor.tsx index d4803934..b2adb902 100644 --- a/src/components/LayerEditor.tsx +++ b/src/components/LayerEditor.tsx @@ -22,7 +22,7 @@ import {formatLayerId} from "../libs/format"; import { type WithTranslation, withTranslation } from "react-i18next"; import { type TFunction } from "i18next"; import { NON_SOURCE_LAYERS } from "../libs/non-source-layers"; -import { type OnMoveLayerCallback } from "../libs/definitions"; +import { type MappedError, type MappedLayerErrors, type OnMoveLayerCallback } from "../libs/definitions"; type MaputnikLayoutGroup = { id: string; @@ -128,7 +128,7 @@ type LayerEditorInternalProps = { isFirstLayer?: boolean isLastLayer?: boolean layerIndex: number - errors?: any[] + errors?: MappedError[] } & WithTranslation; type LayerEditorState = { @@ -193,7 +193,7 @@ class LayerEditorInternal extends React.Component { if ( error.parsed && diff --git a/src/components/LayerList.tsx b/src/components/LayerList.tsx index a515b856..852a8b90 100644 --- a/src/components/LayerList.tsx +++ b/src/components/LayerList.tsx @@ -22,7 +22,7 @@ import type {LayerSpecification, SourceSpecification} from "maplibre-gl"; import generateUniqueId from "../libs/document-uid"; import { findClosestCommonPrefix, layerPrefix } from "../libs/layer"; import { type WithTranslation, withTranslation } from "react-i18next"; -import { type OnMoveLayerCallback } from "../libs/definitions"; +import { type MappedError, type OnMoveLayerCallback } from "../libs/definitions"; type LayerListContainerProps = { layers: LayerSpecification[] @@ -33,7 +33,7 @@ type LayerListContainerProps = { onLayerCopy(...args: unknown[]): unknown onLayerVisibilityToggle(...args: unknown[]): unknown sources: Record; - errors: any[] + errors: MappedError[] }; type LayerListContainerInternalProps = LayerListContainerProps & WithTranslation; diff --git a/src/components/PropertyGroup.tsx b/src/components/PropertyGroup.tsx index b4cddaba..308eb64e 100644 --- a/src/components/PropertyGroup.tsx +++ b/src/components/PropertyGroup.tsx @@ -2,6 +2,7 @@ import React from "react"; import FieldFunction from "./FieldFunction"; import type {LayerSpecification} from "maplibre-gl"; +import { type MappedLayerErrors } from "../libs/definitions"; const iconProperties = ["background-pattern", "fill-pattern", "line-pattern", "fill-extrusion-pattern", "icon-image"]; @@ -36,7 +37,7 @@ type PropertyGroupProps = { groupFields: string[] onChange(...args: unknown[]): unknown spec: any - errors?: {[key: string]: {message: string}} + errors?: MappedLayerErrors }; export default class PropertyGroup extends React.Component { diff --git a/src/components/_DataProperty.tsx b/src/components/_DataProperty.tsx index 6d88c8ce..3685efe1 100644 --- a/src/components/_DataProperty.tsx +++ b/src/components/_DataProperty.tsx @@ -16,6 +16,7 @@ import { type WithTranslation, withTranslation } from "react-i18next"; import labelFromFieldName from "../libs/label-from-field-name"; import DeleteStopButton from "./_DeleteStopButton"; +import { type MappedLayerErrors } from "../libs/definitions"; @@ -47,7 +48,7 @@ type DataPropertyInternalProps = { fieldType?: string fieldSpec?: object value?: DataPropertyValue - errors?: object + errors?: MappedLayerErrors } & WithTranslation; type DataPropertyState = { diff --git a/src/components/_ExpressionProperty.tsx b/src/components/_ExpressionProperty.tsx index 85b5a20d..3847b9d2 100644 --- a/src/components/_ExpressionProperty.tsx +++ b/src/components/_ExpressionProperty.tsx @@ -7,6 +7,7 @@ import Block from "./Block"; import InputButton from "./InputButton"; import labelFromFieldName from "../libs/label-from-field-name"; import FieldJson from "./FieldJson"; +import { type MappedLayerErrors } from "../libs/definitions"; type ExpressionPropertyInternalProps = { @@ -15,7 +16,7 @@ type ExpressionPropertyInternalProps = { fieldType?: string fieldSpec?: object value?: any - errors?: {[key: string]: {message: string}} + errors?: MappedLayerErrors onChange?(...args: unknown[]): unknown onUndo?(...args: unknown[]): unknown canUndo?(...args: unknown[]): unknown diff --git a/src/components/_ZoomProperty.tsx b/src/components/_ZoomProperty.tsx index fd1301df..3f5d1e38 100644 --- a/src/components/_ZoomProperty.tsx +++ b/src/components/_ZoomProperty.tsx @@ -15,6 +15,7 @@ import labelFromFieldName from "../libs/label-from-field-name"; import docUid from "../libs/document-uid"; import sortNumerically from "../libs/sort-numerically"; +import { type MappedLayerErrors } from "../libs/definitions"; /** @@ -59,7 +60,7 @@ type ZoomPropertyInternalProps = { "property-type"?: string "function-type"?: string } - errors?: object + errors?: MappedLayerErrors value?: ZoomWithStops } & WithTranslation; diff --git a/src/libs/definitions.d.ts b/src/libs/definitions.d.ts index 00c2b58c..6f03d84a 100644 --- a/src/libs/definitions.d.ts +++ b/src/libs/definitions.d.ts @@ -16,3 +16,19 @@ export interface IStyleStore { getLatestStyle(): Promise; save(mapStyle: StyleSpecificationWithId): StyleSpecificationWithId; } + +export type MappedError = { + message: string + parsed?: { + type: "layer" + data: { + index: number + key: string + message: string + } + } +}; + +export type MappedLayerErrors = { + [key in LayerSpecification as string]: {message: string} +}; diff --git a/src/styles/_components.scss b/src/styles/_components.scss index 74cdf0dc..9ef3c829 100644 --- a/src/styles/_components.scss +++ b/src/styles/_components.scss @@ -290,6 +290,12 @@ } } +.maputnik-input-block--error { + .maputnik-input-block-label { + color: vars.$color-red; + } +} + .maputnik-expr-infobox { font-size: vars.$font-size-6; background: vars.$color-midgray; diff --git a/src/styles/_layout.scss b/src/styles/_layout.scss index 28a9b2ba..5d1edd6f 100644 --- a/src/styles/_layout.scss +++ b/src/styles/_layout.scss @@ -44,7 +44,7 @@ position: fixed; bottom: 0; right: 0; - z-index: 1; + z-index: 10; width: vars.$layout-map-width; background-color: vars.$color-black; }