Skip to content

Commit

Permalink
fix: don't reset array item labels when changing order
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@chrisvxd
chrisvxd committed Nov 20, 2023
1 parent 40c64e3 commit 57563e1
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 126 deletions.
4 changes: 2 additions & 2 deletions packages/core/components/IconButton/IconButton.tsx
View file
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -19,7 +19,7 @@ export const IconButton = ({
}: {
children: ReactNode;
href?: string;
onClick?: (e: any) => void | Promise<void>;
onClick?: (e: SyntheticEvent) => void | Promise<void>;
variant?: "primary" | "secondary";
type?: "button" | "submit" | "reset";
disabled?: boolean;
Expand Down
271 changes: 147 additions & 124 deletions packages/core/components/InputOrGroup/fields/ArrayField/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -29,35 +29,54 @@ export const ArrayField = ({

const { state, setUi } = useAppContext();

const arrayState: ArrayState = state.ui.arrayState[arrayFieldId] || {
items: Array.from(value || []).map<ItemWithId>((v) => ({
_arrayId: generateId("ArrayItem"),
data: v,
})),
openId: "",
};
const arrayStateRef = useRef<ArrayState>(
state.ui.arrayState[arrayFieldId] || {
items: [],
openId: "",
}
);

const arrayState = arrayStateRef.current;

const setArrayState = useCallback(
(newArrayState: Partial<ArrayState>, recordHistory: boolean = false) => {
(
partialArrayState: Partial<ArrayState>,
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 });
Expand Down Expand Up @@ -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) => (
<Draggable
id={_arrayId}
index={i}
key={_arrayId}
className={(_, snapshot) =>
getClassNameItem({
isExpanded: arrayState.openId === _arrayId,
isDragging: snapshot.isDragging,
readOnly,
})
}
isDragDisabled={readOnly}
>
{() => (
<>
<div
onClick={() => {
if (arrayState.openId === _arrayId) {
setArrayState({
openId: "",
});
} else {
setArrayState({
openId: _arrayId,
});
}
}}
className={getClassNameItem("summary")}
>
{field.getItemSummary
? field.getItemSummary(data, i)
: `Item #${i}`}
<div className={getClassNameItem("rhs")}>
{!readOnly && (
<div className={getClassNameItem("actions")}>
<div className={getClassNameItem("action")}>
<IconButton
onClick={() => {
const existingValue = [
...(value || []),
];
const existingItems = [
...(arrayState.items || []),
];

existingValue.splice(i, 1);
existingItems.splice(i, 1);

setArrayState(
{ items: existingItems },
true
);
onChange(existingValue);
}}
title="Delete"
>
<Trash size={16} />
</IconButton>
</div>
{arrayState.items.map(({ data }, i) => {
const { _arrayId, _originalIndex = i } =
arrayState.items[i] || {};

return (
<Draggable
id={_arrayId}
index={i}
key={_arrayId}
className={(_, snapshot) =>
getClassNameItem({
isExpanded: arrayState.openId === _arrayId,
isDragging: snapshot.isDragging,
readOnly,
})
}
isDragDisabled={readOnly}
>
{() => (
<>
<div
onClick={() => {
if (arrayState.openId === _arrayId) {
setArrayState({
openId: "",
});
} else {
setArrayState({
openId: _arrayId,
});
}
}}
className={getClassNameItem("summary")}
>
{field.getItemSummary
? field.getItemSummary(data, i)
: `Item #${_originalIndex}`}
<div className={getClassNameItem("rhs")}>
{!readOnly && (
<div className={getClassNameItem("actions")}>
<div className={getClassNameItem("action")}>
<IconButton
onClick={(e) => {
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"
>
<Trash size={16} />
</IconButton>
</div>
)}
<div>
<DragIcon />
</div>
)}
<div>
<DragIcon />
</div>
</div>
<div className={getClassNameItem("body")}>
<fieldset
className={getClassNameItem("fieldset")}
>
{Object.keys(field.arrayFields!).map(
(fieldName) => {
const subField =
field.arrayFields![fieldName];

const subFieldName = `${name}[${i}].${fieldName}`;
const wildcardFieldName = `${name}[*].${fieldName}`;

return (
<InputOrGroup
key={subFieldName}
name={subFieldName}
label={subField.label || fieldName}
readOnly={
typeof readOnlyFields[
subFieldName
] !== "undefined"
? readOnlyFields[subFieldName]
: readOnlyFields[wildcardFieldName]
}
readOnlyFields={readOnlyFields}
field={subField}
value={data[fieldName]}
onChange={(val) =>
onChange(
replace(value, i, {
...data,
[fieldName]: val,
})
)
}
/>
);
}
)}
</fieldset>
</div>
</>
)}
</Draggable>
))
: null}
</div>
<div className={getClassNameItem("body")}>
<fieldset className={getClassNameItem("fieldset")}>
{Object.keys(field.arrayFields!).map(
(fieldName) => {
const subField =
field.arrayFields![fieldName];

const subFieldName = `${name}[${i}].${fieldName}`;
const wildcardFieldName = `${name}[*].${fieldName}`;

return (
<InputOrGroup
key={subFieldName}
name={subFieldName}
label={subField.label || fieldName}
readOnly={
typeof readOnlyFields[subFieldName] !==
"undefined"
? readOnlyFields[subFieldName]
: readOnlyFields[wildcardFieldName]
}
readOnlyFields={readOnlyFields}
field={subField}
value={data[fieldName]}
onChange={(val) =>
onChange(
replace(value, i, {
...data,
[fieldName]: val,
})
)
}
/>
);
}
)}
</fieldset>
</div>
</>
)}
</Draggable>
);
})}

{provided.placeholder}

Expand Down
6 changes: 6 additions & 0 deletions packages/core/lib/__tests__/use-puck-history.spec.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
},
Expand Down Expand Up @@ -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" },
},
Expand Down
1 change: 1 addition & 0 deletions packages/core/types/Config.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ export type Data<

export type ItemWithId = {
_arrayId: string;
_originalIndex: number;
data: any;
};

Expand Down

0 comments on commit 57563e1

Please sign in to comment.