From 57563e1da1826dbfa08a32fabb27153e4618ab40 Mon Sep 17 00:00:00 2001 From: Chris Villa Date: Wed, 15 Nov 2023 12:18:43 +0000 Subject: [PATCH] fix: don't reset array item labels when changing order When the user is relying on the default label (i.e. Item 1) for items in an array field, the index would reset whenever the user dragged the items between positions. That was because the label was based on the index. This fix stores the original index for the item in the arrayState, which persists between drags. This also introduces some refactors that may improve the general robustness of array fields. --- .../core/components/IconButton/IconButton.tsx | 4 +- .../InputOrGroup/fields/ArrayField/index.tsx | 271 ++++++++++-------- .../lib/__tests__/use-puck-history.spec.tsx | 6 + packages/core/types/Config.tsx | 1 + 4 files changed, 156 insertions(+), 126 deletions(-) diff --git a/packages/core/components/IconButton/IconButton.tsx b/packages/core/components/IconButton/IconButton.tsx index 08c196e8de..eafa403169 100644 --- a/packages/core/components/IconButton/IconButton.tsx +++ b/packages/core/components/IconButton/IconButton.tsx @@ -1,4 +1,4 @@ -import { ReactNode, useState } from "react"; +import { ReactNode, SyntheticEvent, useState } from "react"; import styles from "./IconButton.module.css"; import getClassNameFactory from "../../lib/get-class-name-factory"; import { ClipLoader } from "react-spinners"; @@ -19,7 +19,7 @@ export const IconButton = ({ }: { children: ReactNode; href?: string; - onClick?: (e: any) => void | Promise; + onClick?: (e: SyntheticEvent) => void | Promise; variant?: "primary" | "secondary"; type?: "button" | "submit" | "reset"; disabled?: boolean; diff --git a/packages/core/components/InputOrGroup/fields/ArrayField/index.tsx b/packages/core/components/InputOrGroup/fields/ArrayField/index.tsx index 4110a49092..eef88de16f 100644 --- a/packages/core/components/InputOrGroup/fields/ArrayField/index.tsx +++ b/packages/core/components/InputOrGroup/fields/ArrayField/index.tsx @@ -8,7 +8,7 @@ import { Droppable } from "@hello-pangea/dnd"; import { DragDropContext } from "@hello-pangea/dnd"; import { Draggable } from "../../../Draggable"; import { generateId } from "../../../../lib/generate-id"; -import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { DragIcon } from "../../../DragIcon"; import { ArrayState, ItemWithId } from "../../../../types/Config"; import { useAppContext } from "../../../Puck/context"; @@ -29,35 +29,54 @@ export const ArrayField = ({ const { state, setUi } = useAppContext(); - const arrayState: ArrayState = state.ui.arrayState[arrayFieldId] || { - items: Array.from(value || []).map((v) => ({ - _arrayId: generateId("ArrayItem"), - data: v, - })), - openId: "", - }; + const arrayStateRef = useRef( + state.ui.arrayState[arrayFieldId] || { + items: [], + openId: "", + } + ); + + const arrayState = arrayStateRef.current; const setArrayState = useCallback( - (newArrayState: Partial, recordHistory: boolean = false) => { + ( + partialArrayState: Partial, + recordHistory: boolean = false + ) => { setUi( { arrayState: { ...state.ui.arrayState, - [arrayFieldId]: { ...arrayState, ...newArrayState }, + [arrayFieldId]: { ...arrayState, ...partialArrayState }, }, }, recordHistory ); + + arrayStateRef.current = { ...arrayState, ...partialArrayState }; }, [arrayState] ); // Create a mirror of value with IDs added for drag and drop useEffect(() => { - const newItems = Array.from(value || []).map((item, idx) => ({ - _arrayId: arrayState.items[idx]?._arrayId || generateId("ArrayItem"), - data: item, - })); + let highestIndex = arrayState.items.reduce( + (acc, item) => (item._originalIndex > acc ? item._originalIndex : acc), + -1 + ); + + const newItems = Array.from(value || []).map((item, idx) => { + const arrayStateItem = arrayState.items[idx]; + + return { + _originalIndex: + typeof arrayStateItem?._originalIndex !== "undefined" + ? arrayStateItem._originalIndex + : ++highestIndex, + _arrayId: arrayState.items[idx]?._arrayId || generateId("ArrayItem"), + data: item, + }; + }); // We don't need to record history during this useEffect, as the history has already been set by onDragEnd setArrayState({ items: newItems }); @@ -99,120 +118,124 @@ export const ArrayField = ({ hasItems: Array.isArray(value) && value.length > 0, })} > - {Array.isArray(value) && value.length > 0 - ? arrayState.items.map(({ _arrayId, data }, i) => ( - - getClassNameItem({ - isExpanded: arrayState.openId === _arrayId, - isDragging: snapshot.isDragging, - readOnly, - }) - } - isDragDisabled={readOnly} - > - {() => ( - <> -
{ - if (arrayState.openId === _arrayId) { - setArrayState({ - openId: "", - }); - } else { - setArrayState({ - openId: _arrayId, - }); - } - }} - className={getClassNameItem("summary")} - > - {field.getItemSummary - ? field.getItemSummary(data, i) - : `Item #${i}`} -
- {!readOnly && ( -
-
- { - const existingValue = [ - ...(value || []), - ]; - const existingItems = [ - ...(arrayState.items || []), - ]; - - existingValue.splice(i, 1); - existingItems.splice(i, 1); - - setArrayState( - { items: existingItems }, - true - ); - onChange(existingValue); - }} - title="Delete" - > - - -
+ {arrayState.items.map(({ data }, i) => { + const { _arrayId, _originalIndex = i } = + arrayState.items[i] || {}; + + return ( + + getClassNameItem({ + isExpanded: arrayState.openId === _arrayId, + isDragging: snapshot.isDragging, + readOnly, + }) + } + isDragDisabled={readOnly} + > + {() => ( + <> +
{ + if (arrayState.openId === _arrayId) { + setArrayState({ + openId: "", + }); + } else { + setArrayState({ + openId: _arrayId, + }); + } + }} + className={getClassNameItem("summary")} + > + {field.getItemSummary + ? field.getItemSummary(data, i) + : `Item #${_originalIndex}`} +
+ {!readOnly && ( +
+
+ { + e.stopPropagation(); + + const existingValue = [ + ...(value || []), + ]; + + const existingItems = [ + ...(arrayState.items || []), + ]; + + existingValue.splice(i, 1); + existingItems.splice(i, 1); + + setArrayState( + { items: existingItems }, + true + ); + + onChange(existingValue); + }} + title="Delete" + > + +
- )} -
-
+ )} +
+
-
-
- {Object.keys(field.arrayFields!).map( - (fieldName) => { - const subField = - field.arrayFields![fieldName]; - - const subFieldName = `${name}[${i}].${fieldName}`; - const wildcardFieldName = `${name}[*].${fieldName}`; - - return ( - - onChange( - replace(value, i, { - ...data, - [fieldName]: val, - }) - ) - } - /> - ); - } - )} -
-
- - )} - - )) - : null} +
+
+
+ {Object.keys(field.arrayFields!).map( + (fieldName) => { + const subField = + field.arrayFields![fieldName]; + + const subFieldName = `${name}[${i}].${fieldName}`; + const wildcardFieldName = `${name}[*].${fieldName}`; + + return ( + + onChange( + replace(value, i, { + ...data, + [fieldName]: val, + }) + ) + } + /> + ); + } + )} +
+
+ + )} + + ); + })} {provided.placeholder} diff --git a/packages/core/lib/__tests__/use-puck-history.spec.tsx b/packages/core/lib/__tests__/use-puck-history.spec.tsx index c55d11dff7..01893060b6 100644 --- a/packages/core/lib/__tests__/use-puck-history.spec.tsx +++ b/packages/core/lib/__tests__/use-puck-history.spec.tsx @@ -60,14 +60,17 @@ const mockedAppStateArray1: AppState = { "ArrayField-6f94973722d42695d4e7b8678e7dcca5affc70df": { items: [ { + _originalIndex: 0, _arrayId: "ArrayItem-46a064dbef49e8441afd782b10320b74f27f8bcb", data: { label: "Learn more", href: "#", variant: "primary" }, }, { + _originalIndex: 1, _arrayId: "ArrayItem-5d99e3a7721edad0d127c9a6b010a37097dad610", data: { label: "Button", href: "#", variant: "primary" }, }, { + _originalIndex: 2, _arrayId: "ArrayItem-222ea8b444fa799a8863eb32d057f3080ec99c65", data: { label: "Button", href: "#", variant: "primary" }, }, @@ -103,14 +106,17 @@ const mockedAppStateArray2: AppState = { "ArrayField-6f94973722d42695d4e7b8678e7dcca5affc70df": { items: [ { + _originalIndex: 0, _arrayId: "ArrayItem-5d99e3a7721edad0d127c9a6b010a37097dad610", data: { label: "Button", href: "#", variant: "primary" }, }, { + _originalIndex: 1, _arrayId: "ArrayItem-222ea8b444fa799a8863eb32d057f3080ec99c65", data: { label: "Button", href: "#", variant: "primary" }, }, { + _originalIndex: 2, _arrayId: "ArrayItem-46a064dbef49e8441afd782b10320b74f27f8bcb", data: { label: "Learn more", href: "#", variant: "primary" }, }, diff --git a/packages/core/types/Config.tsx b/packages/core/types/Config.tsx index b55c1a7988..dbcfe885b5 100644 --- a/packages/core/types/Config.tsx +++ b/packages/core/types/Config.tsx @@ -212,6 +212,7 @@ export type Data< export type ItemWithId = { _arrayId: string; + _originalIndex: number; data: any; };