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:
<img width="1141" height="665" alt="image"
src="https://github.com/user-attachments/assets/c0593d6c-8f14-41b3-8a51-bc359446656d"
/>


After:
<img width="1141" height="665" alt="image"
src="https://github.com/user-attachments/assets/1ffeebb7-31ea-4ed5-97f4-fc5f907a6aea"
/>


 - [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>
This commit is contained in:
Harel M
2025-09-16 16:42:07 +03:00
committed by GitHub
parent 174548944f
commit 3c3fcadbb6
19 changed files with 114 additions and 66 deletions
+37 -22
View File
@@ -492,7 +492,7 @@ describe("layers", () => {
then(get.elementByTestId("spec-field-doc")).shouldContainText("Offset distance"); 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({ when.modal.fillLayers({
type: "symbol", type: "symbol",
layer: "example", layer: "example",
@@ -698,33 +698,48 @@ describe("layers", () => {
}); });
}); });
describe("layereditor jsonlint should error", ()=>{ describe("layers editor", () => {
it("add", () => { describe("property fields", () => {
const id = when.modal.fillLayers({ it("should show error", () => {
type: "circle", when.modal.fillLayers({
layer: "example", 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({ describe("jsonlint should error", ()=>{
layers: [ it("add", () => {
{ const id = when.modal.fillLayers({
id: id, type: "circle",
type: "circle", layer: "example",
source: "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", () => { describe("drag and drop", () => {
it("move layer should update local storage", () => { it("move layer should update local storage", () => {
when.modal.open(); when.modal.open();
+15
View File
@@ -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", () => { describe("Handle localStorage QuotaExceededError", () => {
it("handles quota exceeded error when opening style from URL", () => { it("handles quota exceeded error when opening style from URL", () => {
// Clear localStorage to start fresh // Clear localStorage to start fresh
+3 -15
View File
@@ -36,7 +36,7 @@ import LayerWatcher from "../libs/layerwatcher";
import tokens from "../config/tokens.json"; import tokens from "../config/tokens.json";
import isEqual from "lodash.isequal"; import isEqual from "lodash.isequal";
import { type MapOptions } from "maplibre-gl"; 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. // Buffer must be defined globally for @maplibre/maplibre-gl-style-spec validate() function to succeed.
window.Buffer = buffer.Buffer; 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 = { type AppState = {
errors: MappedErrors[], errors: MappedError[],
infos: string[], infos: string[],
mapStyle: StyleSpecificationWithId, mapStyle: StyleSpecificationWithId,
dirtyMapStyle?: StyleSpecification, dirtyMapStyle?: StyleSpecification,
@@ -383,7 +371,7 @@ export default class App extends React.Component<any, AppState> {
}); });
} }
const mappedErrors = layerErrors.concat(errors).map(error => { const mappedErrors: MappedError[] = layerErrors.concat(errors).map(error => {
// Special case: Duplicate layer id // Special case: Duplicate layer id
const dupMatch = error.message.match(/layers\[(\d+)\]: (duplicate layer id "?(.*)"?, previously used)/); const dupMatch = error.message.match(/layers\[(\d+)\]: (duplicate layer id "?(.*)"?, previously used)/);
if (dupMatch) { if (dupMatch) {
+3 -2
View File
@@ -2,9 +2,10 @@ import React from "react";
import {formatLayerId} from "../libs/format"; import {formatLayerId} from "../libs/format";
import {type LayerSpecification, type StyleSpecification} from "maplibre-gl"; import {type LayerSpecification, type StyleSpecification} from "maplibre-gl";
import { Trans, type WithTranslation, withTranslation } from "react-i18next"; import { Trans, type WithTranslation, withTranslation } from "react-i18next";
import { type MappedError } from "../libs/definitions";
type AppMessagePanelInternalProps = { type AppMessagePanelInternalProps = {
errors?: unknown[] errors?: MappedError[]
infos?: string[] infos?: string[]
mapStyle?: StyleSpecification mapStyle?: StyleSpecification
onLayerSelect?(index: number): void; onLayerSelect?(index: number): void;
@@ -19,7 +20,7 @@ class AppMessagePanelInternal extends React.Component<AppMessagePanelInternalPro
render() { render() {
const {t, selectedLayerIndex} = this.props; const {t, selectedLayerIndex} = this.props;
const errors = this.props.errors?.map((error: any, idx) => { const errors = this.props.errors?.map((error, idx) => {
let content; let content;
if (error.parsed && error.parsed.type === "layer") { if (error.parsed && error.parsed.type === "layer") {
const {parsed} = error; const {parsed} = error;
+5 -4
View File
@@ -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 classnames from "classnames";
import FieldDocLabel from "./FieldDocLabel"; import FieldDocLabel from "./FieldDocLabel";
import Doc from "./Doc"; import Doc from "./Doc";
type BlockProps = PropsWithChildren & { export type BlockProps = PropsWithChildren & {
"data-wd-key"?: string "data-wd-key"?: string
label?: string label?: string
action?: React.ReactElement action?: React.ReactElement
style?: object style?: CSSProperties
onChange?(...args: unknown[]): unknown onChange?(...args: unknown[]): unknown
fieldSpec?: object fieldSpec?: object
wideMode?: boolean wideMode?: boolean
@@ -66,7 +66,8 @@ export default class Block extends React.Component<BlockProps, BlockState> {
className={classnames({ className={classnames({
"maputnik-input-block": true, "maputnik-input-block": true,
"maputnik-input-block--wide": this.props.wideMode, "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} onClick={this.onLabelClick}
> >
+2 -1
View File
@@ -6,6 +6,7 @@ import ZoomProperty from "./_ZoomProperty";
import ExpressionProperty from "./_ExpressionProperty"; import ExpressionProperty from "./_ExpressionProperty";
import {function as styleFunction} from "@maplibre/maplibre-gl-style-spec"; import {function as styleFunction} from "@maplibre/maplibre-gl-style-spec";
import {findDefaultFromSpec} from "../libs/spec-helper"; import {findDefaultFromSpec} from "../libs/spec-helper";
import { type MappedLayerErrors } from "../libs/definitions";
function isLiteralExpression(value: any) { function isLiteralExpression(value: any) {
@@ -119,7 +120,7 @@ type FieldFunctionProps = {
fieldName: string fieldName: string
fieldType: string fieldType: string
fieldSpec: any fieldSpec: any
errors?: {[key: string]: {message: string}} errors?: MappedLayerErrors
value?: any value?: any
}; };
+4 -6
View File
@@ -1,6 +1,6 @@
import Block from "./Block"; import Block, { type BlockProps } from "./Block";
import InputSpec, { type FieldSpecType, type InputSpecProps } from "./InputSpec"; 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 { function getElementFromType(fieldSpec: { type?: FieldSpecType, values?: unknown[] }): typeof Fieldset | typeof Block {
switch(fieldSpec.type) { switch(fieldSpec.type) {
@@ -34,15 +34,13 @@ function getElementFromType(fieldSpec: { type?: FieldSpecType, values?: unknown[
} }
} }
export type FieldSpecProps = InputSpecProps & { export type FieldSpecProps = InputSpecProps & BlockProps & FieldsetProps;
name?: string
};
const FieldSpec: React.FC<FieldSpecProps> = (props) => { const FieldSpec: React.FC<FieldSpecProps> = (props) => {
const TypeBlock = getElementFromType(props.fieldSpec!); const TypeBlock = getElementFromType(props.fieldSpec!);
return ( return (
<TypeBlock label={props.label} action={props.action} fieldSpec={props.fieldSpec}> <TypeBlock label={props.label} action={props.action} fieldSpec={props.fieldSpec} error={props.error}>
<InputSpec {...props} /> <InputSpec {...props} />
</TypeBlock> </TypeBlock>
); );
+7 -2
View File
@@ -1,12 +1,14 @@
import React, { type PropsWithChildren, type ReactElement } from "react"; import React, { type PropsWithChildren, type ReactElement } from "react";
import classnames from "classnames";
import FieldDocLabel from "./FieldDocLabel"; import FieldDocLabel from "./FieldDocLabel";
import Doc from "./Doc"; import Doc from "./Doc";
import generateUniqueId from "../libs/document-uid"; import generateUniqueId from "../libs/document-uid";
type FieldsetProps = PropsWithChildren & { export type FieldsetProps = PropsWithChildren & {
label?: string, label?: string,
fieldSpec?: { doc?: string }, fieldSpec?: { doc?: string },
action?: ReactElement, action?: ReactElement,
error?: {message: string}
}; };
@@ -30,7 +32,10 @@ const Fieldset: React.FC<FieldsetProps> = (props) => {
</div> </div>
)} )}
{!props.fieldSpec && ( {!props.fieldSpec && (
<div className="maputnik-input-block-label"> <div className={classnames({
"maputnik-input-block-label": true,
"maputnik-input-block--error": props.error
})}>
{props.label} {props.label}
</div> </div>
)} )}
+2 -2
View File
@@ -14,7 +14,7 @@ import InputButton from "./InputButton";
import Doc from "./Doc"; import Doc from "./Doc";
import ExpressionProperty from "./_ExpressionProperty"; import ExpressionProperty from "./_ExpressionProperty";
import { type WithTranslation, withTranslation } from "react-i18next"; 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 { function combiningFilter(props: FilterEditorInternalProps): LegacyFilterSpecification | ExpressionSpecification {
@@ -95,7 +95,7 @@ type FilterEditorInternalProps = {
/** Properties of the vector layer and the available fields */ /** Properties of the vector layer and the available fields */
properties?: {[key:string]: any} properties?: {[key:string]: any}
filter?: any[] filter?: any[]
errors?: {[key:string]: any} errors?: MappedLayerErrors
onChange(value: LegacyFilterSpecification | ExpressionSpecification): unknown onChange(value: LegacyFilterSpecification | ExpressionSpecification): unknown
} & WithTranslation; } & WithTranslation;
-2
View File
@@ -31,7 +31,6 @@ export type InputSpecProps = {
/** Override the style of the field */ /** Override the style of the field */
style?: object style?: object
"aria-label"?: string "aria-label"?: string
error?: unknown[]
label?: string label?: string
action?: ReactElement action?: ReactElement
}; };
@@ -43,7 +42,6 @@ export default class InputSpec extends React.Component<InputSpecProps> {
childNodes() { childNodes() {
const commonProps = { const commonProps = {
error: this.props.error,
fieldSpec: this.props.fieldSpec, fieldSpec: this.props.fieldSpec,
label: this.props.label, label: this.props.label,
action: this.props.action, action: this.props.action,
+3 -3
View File
@@ -22,7 +22,7 @@ import {formatLayerId} from "../libs/format";
import { type WithTranslation, withTranslation } from "react-i18next"; import { type WithTranslation, withTranslation } from "react-i18next";
import { type TFunction } from "i18next"; import { type TFunction } from "i18next";
import { NON_SOURCE_LAYERS } from "../libs/non-source-layers"; 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 = { type MaputnikLayoutGroup = {
id: string; id: string;
@@ -128,7 +128,7 @@ type LayerEditorInternalProps = {
isFirstLayer?: boolean isFirstLayer?: boolean
isLastLayer?: boolean isLastLayer?: boolean
layerIndex: number layerIndex: number
errors?: any[] errors?: MappedError[]
} & WithTranslation; } & WithTranslation;
type LayerEditorState = { type LayerEditorState = {
@@ -193,7 +193,7 @@ class LayerEditorInternal extends React.Component<LayerEditorInternalProps, Laye
} }
const {errors, layerIndex} = this.props; const {errors, layerIndex} = this.props;
const errorData: {[key in LayerSpecification as string]: {message: string}} = {}; const errorData: MappedLayerErrors = {};
errors!.forEach(error => { errors!.forEach(error => {
if ( if (
error.parsed && error.parsed &&
+2 -2
View File
@@ -22,7 +22,7 @@ import type {LayerSpecification, SourceSpecification} from "maplibre-gl";
import generateUniqueId from "../libs/document-uid"; import generateUniqueId from "../libs/document-uid";
import { findClosestCommonPrefix, layerPrefix } from "../libs/layer"; import { findClosestCommonPrefix, layerPrefix } from "../libs/layer";
import { type WithTranslation, withTranslation } from "react-i18next"; import { type WithTranslation, withTranslation } from "react-i18next";
import { type OnMoveLayerCallback } from "../libs/definitions"; import { type MappedError, type OnMoveLayerCallback } from "../libs/definitions";
type LayerListContainerProps = { type LayerListContainerProps = {
layers: LayerSpecification[] layers: LayerSpecification[]
@@ -33,7 +33,7 @@ type LayerListContainerProps = {
onLayerCopy(...args: unknown[]): unknown onLayerCopy(...args: unknown[]): unknown
onLayerVisibilityToggle(...args: unknown[]): unknown onLayerVisibilityToggle(...args: unknown[]): unknown
sources: Record<string, SourceSpecification & {layers: string[]}>; sources: Record<string, SourceSpecification & {layers: string[]}>;
errors: any[] errors: MappedError[]
}; };
type LayerListContainerInternalProps = LayerListContainerProps & WithTranslation; type LayerListContainerInternalProps = LayerListContainerProps & WithTranslation;
+2 -1
View File
@@ -2,6 +2,7 @@ import React from "react";
import FieldFunction from "./FieldFunction"; import FieldFunction from "./FieldFunction";
import type {LayerSpecification} from "maplibre-gl"; 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"]; const iconProperties = ["background-pattern", "fill-pattern", "line-pattern", "fill-extrusion-pattern", "icon-image"];
@@ -36,7 +37,7 @@ type PropertyGroupProps = {
groupFields: string[] groupFields: string[]
onChange(...args: unknown[]): unknown onChange(...args: unknown[]): unknown
spec: any spec: any
errors?: {[key: string]: {message: string}} errors?: MappedLayerErrors
}; };
export default class PropertyGroup extends React.Component<PropertyGroupProps> { export default class PropertyGroup extends React.Component<PropertyGroupProps> {
+2 -1
View File
@@ -16,6 +16,7 @@ import { type WithTranslation, withTranslation } from "react-i18next";
import labelFromFieldName from "../libs/label-from-field-name"; import labelFromFieldName from "../libs/label-from-field-name";
import DeleteStopButton from "./_DeleteStopButton"; import DeleteStopButton from "./_DeleteStopButton";
import { type MappedLayerErrors } from "../libs/definitions";
@@ -47,7 +48,7 @@ type DataPropertyInternalProps = {
fieldType?: string fieldType?: string
fieldSpec?: object fieldSpec?: object
value?: DataPropertyValue value?: DataPropertyValue
errors?: object errors?: MappedLayerErrors
} & WithTranslation; } & WithTranslation;
type DataPropertyState = { type DataPropertyState = {
+2 -1
View File
@@ -7,6 +7,7 @@ import Block from "./Block";
import InputButton from "./InputButton"; import InputButton from "./InputButton";
import labelFromFieldName from "../libs/label-from-field-name"; import labelFromFieldName from "../libs/label-from-field-name";
import FieldJson from "./FieldJson"; import FieldJson from "./FieldJson";
import { type MappedLayerErrors } from "../libs/definitions";
type ExpressionPropertyInternalProps = { type ExpressionPropertyInternalProps = {
@@ -15,7 +16,7 @@ type ExpressionPropertyInternalProps = {
fieldType?: string fieldType?: string
fieldSpec?: object fieldSpec?: object
value?: any value?: any
errors?: {[key: string]: {message: string}} errors?: MappedLayerErrors
onChange?(...args: unknown[]): unknown onChange?(...args: unknown[]): unknown
onUndo?(...args: unknown[]): unknown onUndo?(...args: unknown[]): unknown
canUndo?(...args: unknown[]): unknown canUndo?(...args: unknown[]): unknown
+2 -1
View File
@@ -15,6 +15,7 @@ import labelFromFieldName from "../libs/label-from-field-name";
import docUid from "../libs/document-uid"; import docUid from "../libs/document-uid";
import sortNumerically from "../libs/sort-numerically"; import sortNumerically from "../libs/sort-numerically";
import { type MappedLayerErrors } from "../libs/definitions";
/** /**
@@ -59,7 +60,7 @@ type ZoomPropertyInternalProps = {
"property-type"?: string "property-type"?: string
"function-type"?: string "function-type"?: string
} }
errors?: object errors?: MappedLayerErrors
value?: ZoomWithStops value?: ZoomWithStops
} & WithTranslation; } & WithTranslation;
+16
View File
@@ -16,3 +16,19 @@ export interface IStyleStore {
getLatestStyle(): Promise<StyleSpecificationWithId>; getLatestStyle(): Promise<StyleSpecificationWithId>;
save(mapStyle: StyleSpecificationWithId): StyleSpecificationWithId; 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}
};
+6
View File
@@ -290,6 +290,12 @@
} }
} }
.maputnik-input-block--error {
.maputnik-input-block-label {
color: vars.$color-red;
}
}
.maputnik-expr-infobox { .maputnik-expr-infobox {
font-size: vars.$font-size-6; font-size: vars.$font-size-6;
background: vars.$color-midgray; background: vars.$color-midgray;
+1 -1
View File
@@ -44,7 +44,7 @@
position: fixed; position: fixed;
bottom: 0; bottom: 0;
right: 0; right: 0;
z-index: 1; z-index: 10;
width: vars.$layout-map-width; width: vars.$layout-map-width;
background-color: vars.$color-black; background-color: vars.$color-black;
} }