From b419fd9975fb3ab47a120b3ef11c68c91e72a858 Mon Sep 17 00:00:00 2001 From: Wojciech Mista Date: Tue, 22 Feb 2022 10:29:18 +0100 Subject: [PATCH] Fix menu reordering in navigation (#1871) --- .../MenuDetailsPage/MenuDetailsPage.tsx | 36 +- .../components/MenuDetailsPage/tree.test.ts | 1361 +++++++++++++++-- .../components/MenuDetailsPage/tree.ts | 52 +- .../components/MenuItems/MenuItems.tsx | 13 +- .../MenuItems/__snapshots__/tree.test.ts.snap | 42 +- src/navigation/components/MenuItems/tree.ts | 44 +- .../__snapshots__/Stories.test.ts.snap | 21 +- 7 files changed, 1344 insertions(+), 225 deletions(-) diff --git a/src/navigation/components/MenuDetailsPage/MenuDetailsPage.tsx b/src/navigation/components/MenuDetailsPage/MenuDetailsPage.tsx index 6a15846d7..9962ba653 100644 --- a/src/navigation/components/MenuDetailsPage/MenuDetailsPage.tsx +++ b/src/navigation/components/MenuDetailsPage/MenuDetailsPage.tsx @@ -12,12 +12,11 @@ import { Backlink } from "@saleor/macaw-ui"; import React from "react"; import { FormattedMessage, useIntl } from "react-intl"; -import { maybe } from "../../../misc"; import { MenuDetails_menu } from "../../types/MenuDetails"; import { MenuItemType } from "../MenuItemDialog"; import MenuItems, { TreeOperation } from "../MenuItems"; import MenuProperties from "../MenuProperties"; -import { computeTree } from "./tree"; +import { computeRelativeTree } from "./tree"; export interface MenuDetailsFormData { name: string; @@ -55,17 +54,20 @@ const MenuDetailsPage: React.FC = ({ const intl = useIntl(); const initialForm: MenuDetailsFormData = { - name: maybe(() => menu.name, "") + name: menu?.name ?? "" }; const [treeOperations, setTreeOperations] = React.useState( [] ); + const removeSimulatedMoves = (operations: TreeOperation[]) => + operations.filter(operation => !operation.simulatedMove); + const handleSubmit = async (data: MenuDetailsFormData) => { const result = await onSubmit({ name: data.name, - operations: treeOperations + operations: removeSimulatedMoves(treeOperations) }); if (result) { @@ -75,10 +77,8 @@ const MenuDetailsPage: React.FC = ({ return result; }; - const handleChange = (operation: TreeOperation) => { - if (!!operation) { - setTreeOperations([...treeOperations, operation]); - } + const handleChange = (operations: TreeOperation[]) => { + setTreeOperations([...treeOperations, ...operations]); }; return ( @@ -110,17 +110,25 @@ const MenuDetailsPage: React.FC = ({ 0} - items={maybe(() => - computeTree(menu.items, [...treeOperations]) - )} + items={ + menu?.items + ? computeRelativeTree(menu.items, treeOperations) + : [] + } onChange={handleChange} onItemAdd={onItemAdd} onItemClick={onItemClick} onItemEdit={onItemEdit} onUndo={() => - setTreeOperations( - treeOperations.slice(0, treeOperations.length - 1) - ) + setTreeOperations(operations => { + if (operations.length > 1) { + // Undo of a simulated move needs removal of 2 moves instead of one + if (operations[operations.length - 2].simulatedMove) { + return operations.slice(0, operations.length - 2); + } + } + return operations.slice(0, operations.length - 1); + }) } /> diff --git a/src/navigation/components/MenuDetailsPage/tree.test.ts b/src/navigation/components/MenuDetailsPage/tree.test.ts index 43b562d18..05f2e9c08 100644 --- a/src/navigation/components/MenuDetailsPage/tree.test.ts +++ b/src/navigation/components/MenuDetailsPage/tree.test.ts @@ -1,7 +1,1154 @@ import { menu } from "../../fixtures"; import { MenuDetails_menu_items } from "../../types/MenuDetails"; import { TreeOperation } from "../MenuItems"; -import { computeTree } from "./tree"; +import { computeRelativeTree } from "./tree"; + +const relativeOutput: MenuDetails_menu_items[][] = [ + // no moves + [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6Nw==", + name: "Accessories" + }, + children: [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OAX==", + name: "Jewelry" + }, + children: [], + collection: null, + id: "0jewelry", + level: 0, + name: "Jewelry", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQX==", + name: "Glasses" + }, + children: [], + collection: null, + id: "1glasses", + level: 0, + name: "Glasses", + page: null, + url: null + } + ], + collection: null, + id: "2accessories", + level: 0, + name: "Accessories", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OA==", + name: "Groceries" + }, + children: [], + collection: null, + id: "3groceries", + level: 0, + name: "Groceries", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQ==", + name: "Apparel" + }, + children: [], + collection: null, + id: "4apparel", + level: 0, + name: "Apparel", + page: null, + url: null + } + ], + // moves one in root + [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6Nw==", + name: "Accessories" + }, + children: [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OAX==", + name: "Jewelry" + }, + children: [], + collection: null, + id: "0jewelry", + level: 0, + name: "Jewelry", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQX==", + name: "Glasses" + }, + children: [], + collection: null, + id: "1glasses", + level: 0, + name: "Glasses", + page: null, + url: null + } + ], + collection: null, + id: "2accessories", + level: 0, + name: "Accessories", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQ==", + name: "Apparel" + }, + children: [], + collection: null, + id: "4apparel", + level: 0, + name: "Apparel", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OA==", + name: "Groceries" + }, + children: [], + collection: null, + id: "3groceries", + level: 0, + name: "Groceries", + page: null, + url: null + } + ], + // moves two in root + [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQ==", + name: "Apparel" + }, + children: [], + collection: null, + id: "4apparel", + level: 0, + name: "Apparel", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OA==", + name: "Groceries" + }, + children: [], + collection: null, + id: "3groceries", + level: 0, + name: "Groceries", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6Nw==", + name: "Accessories" + }, + children: [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OAX==", + name: "Jewelry" + }, + children: [], + collection: null, + id: "0jewelry", + level: 0, + name: "Jewelry", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQX==", + name: "Glasses" + }, + children: [], + collection: null, + id: "1glasses", + level: 0, + name: "Glasses", + page: null, + url: null + } + ], + collection: null, + id: "2accessories", + level: 0, + name: "Accessories", + page: null, + url: null + } + ], + // empty move + [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6Nw==", + name: "Accessories" + }, + children: [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OAX==", + name: "Jewelry" + }, + children: [], + collection: null, + id: "0jewelry", + level: 0, + name: "Jewelry", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQX==", + name: "Glasses" + }, + children: [], + collection: null, + id: "1glasses", + level: 0, + name: "Glasses", + page: null, + url: null + } + ], + collection: null, + id: "2accessories", + level: 0, + name: "Accessories", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OA==", + name: "Groceries" + }, + children: [], + collection: null, + id: "3groceries", + level: 0, + name: "Groceries", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQ==", + name: "Apparel" + }, + children: [], + collection: null, + id: "4apparel", + level: 0, + name: "Apparel", + page: null, + url: null + } + ], + // moves every + [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OA==", + name: "Groceries" + }, + children: [], + collection: null, + id: "3groceries", + level: 0, + name: "Groceries", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQ==", + name: "Apparel" + }, + children: [], + collection: null, + id: "4apparel", + level: 0, + name: "Apparel", + page: null, + url: null + }, + + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6Nw==", + name: "Accessories" + }, + children: [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OAX==", + name: "Jewelry" + }, + children: [], + collection: null, + id: "0jewelry", + level: 0, + name: "Jewelry", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQX==", + name: "Glasses" + }, + children: [], + collection: null, + id: "1glasses", + level: 0, + name: "Glasses", + page: null, + url: null + } + ], + collection: null, + id: "2accessories", + level: 0, + name: "Accessories", + page: null, + url: null + } + ], + // moves children + [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6Nw==", + name: "Accessories" + }, + children: [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQX==", + name: "Glasses" + }, + children: [], + collection: null, + id: "1glasses", + level: 0, + name: "Glasses", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OAX==", + name: "Jewelry" + }, + children: [], + collection: null, + id: "0jewelry", + level: 0, + name: "Jewelry", + page: null, + url: null + } + ], + collection: null, + id: "2accessories", + level: 0, + name: "Accessories", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OA==", + name: "Groceries" + }, + children: [], + collection: null, + id: "3groceries", + level: 0, + name: "Groceries", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQ==", + name: "Apparel" + }, + children: [], + collection: null, + id: "4apparel", + level: 0, + name: "Apparel", + page: null, + url: null + } + ], + // moves child outside + [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQX==", + name: "Glasses" + }, + children: [], + collection: null, + id: "1glasses", + level: 0, + name: "Glasses", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6Nw==", + name: "Accessories" + }, + children: [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OAX==", + name: "Jewelry" + }, + children: [], + collection: null, + id: "0jewelry", + level: 0, + name: "Jewelry", + page: null, + url: null + } + ], + collection: null, + id: "2accessories", + level: 0, + name: "Accessories", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OA==", + name: "Groceries" + }, + children: [], + collection: null, + id: "3groceries", + level: 0, + name: "Groceries", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQ==", + name: "Apparel" + }, + children: [], + collection: null, + id: "4apparel", + level: 0, + name: "Apparel", + page: null, + url: null + } + ], + // moves child outside and puts it in location + [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6Nw==", + name: "Accessories" + }, + children: [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OAX==", + name: "Jewelry" + }, + children: [], + collection: null, + id: "0jewelry", + level: 0, + name: "Jewelry", + page: null, + url: null + } + ], + collection: null, + id: "2accessories", + level: 0, + name: "Accessories", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OA==", + name: "Groceries" + }, + children: [], + collection: null, + id: "3groceries", + level: 0, + name: "Groceries", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQX==", + name: "Glasses" + }, + children: [], + collection: null, + id: "1glasses", + level: 0, + name: "Glasses", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQ==", + name: "Apparel" + }, + children: [], + collection: null, + id: "4apparel", + level: 0, + name: "Apparel", + page: null, + url: null + } + ], + // moves child inside + [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6Nw==", + name: "Accessories" + }, + children: [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OA==", + name: "Groceries" + }, + children: [], + collection: null, + id: "3groceries", + level: 0, + name: "Groceries", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OAX==", + name: "Jewelry" + }, + children: [], + collection: null, + id: "0jewelry", + level: 0, + name: "Jewelry", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQX==", + name: "Glasses" + }, + children: [], + collection: null, + id: "1glasses", + level: 0, + name: "Glasses", + page: null, + url: null + } + ], + collection: null, + id: "2accessories", + level: 0, + name: "Accessories", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQ==", + name: "Apparel" + }, + children: [], + collection: null, + id: "4apparel", + level: 0, + name: "Apparel", + page: null, + url: null + } + ], + // moves child inside then outside then changes index + [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6Nw==", + name: "Accessories" + }, + children: [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OA==", + name: "Groceries" + }, + children: [], + collection: null, + id: "3groceries", + level: 0, + name: "Groceries", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OAX==", + name: "Jewelry" + }, + children: [], + collection: null, + id: "0jewelry", + level: 0, + name: "Jewelry", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQX==", + name: "Glasses" + }, + children: [], + collection: null, + id: "1glasses", + level: 0, + name: "Glasses", + page: null, + url: null + } + ], + collection: null, + id: "2accessories", + level: 0, + name: "Accessories", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQ==", + name: "Apparel" + }, + children: [], + collection: null, + id: "4apparel", + level: 0, + name: "Apparel", + page: null, + url: null + } + ], + // moves item as last child and moves it up + [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6Nw==", + name: "Accessories" + }, + children: [ + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OAX==", + name: "Jewelry" + }, + children: [], + collection: null, + id: "0jewelry", + level: 0, + name: "Jewelry", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OA==", + name: "Groceries" + }, + children: [], + collection: null, + id: "3groceries", + level: 0, + name: "Groceries", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQX==", + name: "Glasses" + }, + children: [], + collection: null, + id: "1glasses", + level: 0, + name: "Glasses", + page: null, + url: null + } + ], + collection: null, + id: "2accessories", + level: 0, + name: "Accessories", + page: null, + url: null + }, + { + __typename: "MenuItem", + category: { + __typename: "Category", + id: "Q2F0ZWdvcnk6OQ==", + name: "Apparel" + }, + children: [], + collection: null, + id: "4apparel", + level: 0, + name: "Apparel", + page: null, + url: null + } + ] +]; + +const secondTestTable: TreeOperation[][] = [ + // no moves + [], + // moves one in root + [{ id: "4apparel", sortOrder: -1, type: "move" }], + // moves two in root + [ + { id: "2accessories", sortOrder: 2, type: "move" }, + { id: "4apparel", sortOrder: -1, type: "move" } + ], + // empty move + [ + { id: "2accessories", sortOrder: 0, type: "move" }, + { id: "4apparel", sortOrder: 0, type: "move" } + ], + // move every + [ + { id: "2accessories", sortOrder: 1, type: "move" }, + { id: "4apparel", sortOrder: -2, type: "move" }, + { id: "3groceries", sortOrder: -1, type: "move" } + ], + // moves children + [ + { + id: "1glasses", + sortOrder: -1, + type: "move", + parentId: "2accessories" + } + ], + // moves children outside + [ + { + id: "1glasses", + sortOrder: 3, + type: "move", + simulatedMove: true + }, + { + id: "1glasses", + sortOrder: 0 - 3, + type: "move" + } + ], + // moves children outside and puts it in a location + [ + { + id: "1glasses", + sortOrder: 3, + type: "move", + simulatedMove: true + }, + { + id: "1glasses", + sortOrder: 2 - 3, + type: "move" + } + ], + // moves child inside + [ + { + id: "3groceries", + parentId: "2accessories", + sortOrder: 2, + type: "move", + simulatedMove: true + }, + { + id: "3groceries", + parentId: "2accessories", + sortOrder: 0 - 2, + type: "move" + } + ], + // moves child inside, moves it and puts it back + [ + { + id: "3groceries", + parentId: "2accessories", + sortOrder: 0, + type: "move" + }, + { + id: "3groceries", + sortOrder: 1, + type: "move" + }, + { + id: "3groceries", + sortOrder: 1, + type: "move" + }, + { + id: "3groceries", + sortOrder: -2, + type: "move" + }, + { + id: "3groceries", + parentId: "2accessories", + sortOrder: 0, + type: "move" + } + ], + // moves item as last child and moves it up + [ + { + id: "3groceries", + parentId: "2accessories", + sortOrder: 2, + type: "move" + }, + { + id: "1glasses", + parentId: "2accessories", + sortOrder: 1, + type: "move" + } + ] +]; + +const testTable: TreeOperation[][] = [ + [], + [ + { id: "1glasses", parentId: "0jewelry", sortOrder: 0, type: "move" }, + { + id: "2accessories", + parentId: "3groceries", + sortOrder: 0, + type: "move" + } + ], + [ + { id: "1glasses", parentId: "0jewelry", sortOrder: 0, type: "move" }, + { + id: "2accessories", + parentId: "3groceries", + sortOrder: 0, + type: "move" + }, + { + id: "3groceries", + parentId: "4apparel", + sortOrder: 0, + type: "move" + } + ], + [ + { id: "0jewelry", sortOrder: 1, type: "move" }, + { id: "1glasses", sortOrder: 1, type: "move" }, + { + id: "4apparel", + parentId: "3groceries", + sortOrder: 0, + type: "move" + }, + { + id: "3groceries", + parentId: "0jewelry", + sortOrder: 0, + type: "move" + }, + { id: "0jewelry", parentId: "1glasses", sortOrder: 0, type: "move" }, + { + id: "1glasses", + parentId: "2accessories", + sortOrder: 0, + type: "move" + }, + { id: "1glasses", sortOrder: 1, type: "move" }, + { id: "0jewelry", sortOrder: 2, type: "move" } + ], + [ + { id: "1glasses", sortOrder: 1, type: "move" }, + { id: "1glasses", parentId: "0jewelry", sortOrder: 0, type: "move" }, + { id: "0jewelry", sortOrder: 1, type: "move" }, + { + id: "0jewelry", + parentId: "2accessories", + sortOrder: 0, + type: "move" + }, + { id: "3groceries", sortOrder: 0, type: "move" }, + { + id: "2accessories", + parentId: "3groceries", + sortOrder: 0, + type: "move" + }, + { id: "2accessories", sortOrder: 1, type: "move" }, + { id: "0jewelry", sortOrder: 2, type: "move" }, + { id: "1glasses", sortOrder: 3, type: "move" }, + { id: "4apparel", sortOrder: 0, type: "move" }, + { id: "1glasses", sortOrder: 1, type: "move" }, + { id: "2accessories", sortOrder: 0, type: "move" }, + { + id: "4apparel", + parentId: "2accessories", + sortOrder: 0, + type: "move" + }, + { + id: "3groceries", + parentId: "1glasses", + sortOrder: 0, + type: "move" + }, + { id: "0jewelry", sortOrder: 0, type: "move" }, + { + id: "0jewelry", + parentId: "2accessories", + sortOrder: 0, + type: "move" + }, + { + id: "4apparel", + parentId: "2accessories", + sortOrder: 0, + type: "move" + }, + { + id: "0jewelry", + parentId: "2accessories", + sortOrder: 0, + type: "move" + }, + { + id: "1glasses", + parentId: "2accessories", + sortOrder: 2, + type: "move" + }, + { + id: "0jewelry", + parentId: "2accessories", + sortOrder: 2, + type: "move" + }, + { + id: "1glasses", + parentId: "2accessories", + sortOrder: 2, + type: "move" + }, + { + id: "4apparel", + parentId: "2accessories", + sortOrder: 2, + type: "move" + }, + { + id: "3groceries", + parentId: "0jewelry", + sortOrder: 0, + type: "move" + }, + { id: "4apparel", parentId: "1glasses", sortOrder: 0, type: "move" }, + { id: "1glasses", sortOrder: 1, type: "move" }, + { id: "0jewelry", sortOrder: 1, type: "move" }, + { + id: "2accessories", + parentId: "4apparel", + sortOrder: 0, + type: "move" + } + ], + [{ id: "2accessories", type: "remove" }], + [ + { id: "2accessories", type: "remove" }, + { id: "4apparel", sortOrder: 0, type: "move" }, + { id: "3groceries", type: "remove" } + ] +]; // Readability FTW function innerTreeToString( @@ -23,163 +1170,67 @@ function treeToString(tree: MenuDetails_menu_items[]): string { } describe("Properly computes trees", () => { - const testTable: TreeOperation[][] = [ - [], - [ - { id: "1glasses", parentId: "0jewelry", sortOrder: 0, type: "move" }, - { - id: "2accessories", - parentId: "3groceries", - sortOrder: 0, - type: "move" - } - ], - [ - { id: "1glasses", parentId: "0jewelry", sortOrder: 0, type: "move" }, - { - id: "2accessories", - parentId: "3groceries", - sortOrder: 0, - type: "move" - }, - { - id: "3groceries", - parentId: "4apparel", - sortOrder: 0, - type: "move" - } - ], - [ - { id: "0jewelry", sortOrder: 1, type: "move" }, - { id: "1glasses", sortOrder: 1, type: "move" }, - { - id: "4apparel", - parentId: "3groceries", - sortOrder: 0, - type: "move" - }, - { - id: "3groceries", - parentId: "0jewelry", - sortOrder: 0, - type: "move" - }, - { id: "0jewelry", parentId: "1glasses", sortOrder: 0, type: "move" }, - { - id: "1glasses", - parentId: "2accessories", - sortOrder: 0, - type: "move" - }, - { id: "1glasses", sortOrder: 1, type: "move" }, - { id: "0jewelry", sortOrder: 2, type: "move" } - ], - [ - { id: "1glasses", sortOrder: 1, type: "move" }, - { id: "1glasses", parentId: "0jewelry", sortOrder: 0, type: "move" }, - { id: "0jewelry", sortOrder: 1, type: "move" }, - { - id: "0jewelry", - parentId: "2accessories", - sortOrder: 0, - type: "move" - }, - { id: "3groceries", sortOrder: 0, type: "move" }, - { - id: "2accessories", - parentId: "3groceries", - sortOrder: 0, - type: "move" - }, - { id: "2accessories", sortOrder: 1, type: "move" }, - { id: "0jewelry", sortOrder: 2, type: "move" }, - { id: "1glasses", sortOrder: 3, type: "move" }, - { id: "4apparel", sortOrder: 0, type: "move" }, - { id: "1glasses", sortOrder: 1, type: "move" }, - { id: "2accessories", sortOrder: 0, type: "move" }, - { - id: "4apparel", - parentId: "2accessories", - sortOrder: 0, - type: "move" - }, - { - id: "3groceries", - parentId: "1glasses", - sortOrder: 0, - type: "move" - }, - { id: "0jewelry", sortOrder: 0, type: "move" }, - { - id: "0jewelry", - parentId: "2accessories", - sortOrder: 0, - type: "move" - }, - { - id: "4apparel", - parentId: "2accessories", - sortOrder: 0, - type: "move" - }, - { - id: "0jewelry", - parentId: "2accessories", - sortOrder: 0, - type: "move" - }, - { - id: "1glasses", - parentId: "2accessories", - sortOrder: 2, - type: "move" - }, - { - id: "0jewelry", - parentId: "2accessories", - sortOrder: 2, - type: "move" - }, - { - id: "1glasses", - parentId: "2accessories", - sortOrder: 2, - type: "move" - }, - { - id: "4apparel", - parentId: "2accessories", - sortOrder: 2, - type: "move" - }, - { - id: "3groceries", - parentId: "0jewelry", - sortOrder: 0, - type: "move" - }, - { id: "4apparel", parentId: "1glasses", sortOrder: 0, type: "move" }, - { id: "1glasses", sortOrder: 1, type: "move" }, - { id: "0jewelry", sortOrder: 1, type: "move" }, - { - id: "2accessories", - parentId: "4apparel", - sortOrder: 0, - type: "move" - } - ], - [{ id: "2accessories", type: "remove" }], - [ - { id: "2accessories", type: "remove" }, - { id: "4apparel", sortOrder: 0, type: "move" }, - { id: "3groceries", type: "remove" } - ] - ]; - testTable.forEach(testData => it("#", () => { - const computedTree = computeTree(menu.items, testData); + const computedTree = computeRelativeTree(menu.items, testData); expect(treeToString(computedTree)).toMatchSnapshot(); }) ); }); + +describe("Properly computes relative trees", () => { + it("doesn't move anything", () => { + const computedTree = computeRelativeTree(menu.items, secondTestTable[0]); + expect(computedTree).toEqual(relativeOutput[0]); + }); + + it("moves one root element", () => { + const computedTree = computeRelativeTree(menu.items, secondTestTable[1]); + expect(computedTree).toEqual(relativeOutput[1]); + }); + + it("moves two root element", () => { + const computedTree = computeRelativeTree(menu.items, secondTestTable[2]); + expect(computedTree).toEqual(relativeOutput[2]); + }); + + it("empty moves", () => { + const computedTree = computeRelativeTree(menu.items, secondTestTable[3]); + expect(computedTree).toEqual(relativeOutput[3]); + }); + + it("moves every element", () => { + const computedTree = computeRelativeTree(menu.items, secondTestTable[4]); + expect(computedTree).toEqual(relativeOutput[4]); + }); + + it("moves children", () => { + const computedTree = computeRelativeTree(menu.items, secondTestTable[5]); + expect(computedTree).toEqual(relativeOutput[5]); + }); + + it("moves child outside", () => { + const computedTree = computeRelativeTree(menu.items, secondTestTable[6]); + expect(computedTree).toEqual(relativeOutput[6]); + }); + + it("moves child outside and puts it in a location", () => { + const computedTree = computeRelativeTree(menu.items, secondTestTable[7]); + expect(computedTree).toEqual(relativeOutput[7]); + }); + + it("moves child inside", () => { + const computedTree = computeRelativeTree(menu.items, secondTestTable[8]); + expect(computedTree).toEqual(relativeOutput[8]); + }); + + it("moves child inside then outside then changes index", () => { + const computedTree = computeRelativeTree(menu.items, secondTestTable[9]); + expect(computedTree).toEqual(relativeOutput[9]); + }); + + it("moves item as last child and moves it up", () => { + const computedTree = computeRelativeTree(menu.items, secondTestTable[10]); + expect(computedTree).toEqual(relativeOutput[10]); + }); +}); diff --git a/src/navigation/components/MenuDetailsPage/tree.ts b/src/navigation/components/MenuDetailsPage/tree.ts index 82658aeaa..3db0a9f4a 100644 --- a/src/navigation/components/MenuDetailsPage/tree.ts +++ b/src/navigation/components/MenuDetailsPage/tree.ts @@ -45,23 +45,28 @@ function removeNode( return newTree; } -function insertNode( - tree: MenuDetails_menu_items[], - path: number[], - node: MenuDetails_menu_items, - position: number -): MenuDetails_menu_items[] { +function insertNode({ + tree, + path, + node, + position +}: { + tree: MenuDetails_menu_items[]; + path: number[]; + node: MenuDetails_menu_items; + position: number; +}): MenuDetails_menu_items[] { if (path.length === 0) { return [...tree.slice(0, position), node, ...tree.slice(position)]; } if (path[0] in tree) { - tree[path[0]].children = insertNode( - tree[path[0]].children, - path.slice(1), + tree[path[0]].children = insertNode({ + tree: tree[path[0]].children, + path: path.slice(1), node, position - ); + }); } return tree; } @@ -89,45 +94,48 @@ function removeNodeAndChildren( return removeNode(tree, sourcePath); } -function permuteNode( +function permuteRelativeNode( tree: MenuDetails_menu_items[], permutation: TreeOperation ): MenuDetails_menu_items[] { const sourcePath = findNode(tree, permutation.id); const node = getNode(tree, sourcePath); + const hasParent = !!permutation.parentId; + const treeAfterRemoval = removeNode(tree, sourcePath); - const targetPath = permutation.parentId + const targetPath = hasParent ? findNode(treeAfterRemoval, permutation.parentId) : []; - const treeAfterInsertion = insertNode( - treeAfterRemoval, - targetPath, + const position = sourcePath[sourcePath.length - 1]; + + const treeAfterInsertion = insertNode({ + tree: treeAfterRemoval, + path: targetPath, node, - permutation.sortOrder - ); + position: position + permutation.sortOrder + }); return treeAfterInsertion; } -function executeOperation( +function executeRelativeOperation( tree: MenuDetails_menu_items[], operation: TreeOperation ): MenuDetails_menu_items[] { return operation.type === "move" - ? permuteNode(tree, operation) + ? permuteRelativeNode(tree, operation) : removeNodeAndChildren(tree, operation); } -export function computeTree( +export function computeRelativeTree( tree: MenuDetails_menu_items[], operations: TreeOperation[] ) { const newTree = operations.reduce( - (acc, operation) => executeOperation(acc, operation), - // FIXME: 😡 + (acc, operation) => executeRelativeOperation(acc, operation), JSON.parse(JSON.stringify(tree)) ); return newTree; diff --git a/src/navigation/components/MenuItems/MenuItems.tsx b/src/navigation/components/MenuItems/MenuItems.tsx index b6030411d..0b566ce79 100644 --- a/src/navigation/components/MenuItems/MenuItems.tsx +++ b/src/navigation/components/MenuItems/MenuItems.tsx @@ -21,7 +21,7 @@ const NODE_MARGIN = 40; export interface MenuItemsProps { canUndo: boolean; items: MenuDetails_menu_items[]; - onChange: (operation: TreeOperation) => void; + onChange: (operations: TreeOperation[]) => void; onItemAdd: () => void; onItemClick: (id: string, type: MenuItemType) => void; onItemEdit: (id: string) => void; @@ -181,10 +181,12 @@ const Node: React.FC = props => { className={classes.deleteButton} variant="secondary" onClick={() => - node.onChange({ - id: node.id as any, - type: "remove" - }) + node.onChange([ + { + id: node.id, + type: "remove" + } + ]) } > @@ -245,6 +247,7 @@ const MenuItems: React.FC = props => { marginLeft: NODE_MARGIN * (path.length - 1) } })} + maxDepth={5} isVirtualized={false} rowHeight={NODE_HEIGHT} treeData={items.map(item => diff --git a/src/navigation/components/MenuItems/__snapshots__/tree.test.ts.snap b/src/navigation/components/MenuItems/__snapshots__/tree.test.ts.snap index f9bd083a0..62966e233 100644 --- a/src/navigation/components/MenuItems/__snapshots__/tree.test.ts.snap +++ b/src/navigation/components/MenuItems/__snapshots__/tree.test.ts.snap @@ -1,28 +1,34 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`Properly computes diffs # 1`] = ` -Object { - "id": "1glasses", - "parentId": "0jewelry", - "sortOrder": 0, - "type": "move", -} +Array [ + Object { + "id": "1glasses", + "parentId": "0jewelry", + "sortOrder": 0, + "type": "move", + }, +] `; exports[`Properly computes diffs # 2`] = ` -Object { - "id": "1glasses", - "parentId": "2accessories", - "sortOrder": 0, - "type": "move", -} +Array [ + Object { + "id": "1glasses", + "parentId": "2accessories", + "sortOrder": -1, + "type": "move", + }, +] `; exports[`Properly computes diffs # 3`] = ` -Object { - "id": "2accessories", - "parentId": "4apparel", - "sortOrder": 0, - "type": "move", -} +Array [ + Object { + "id": "2accessories", + "parentId": "4apparel", + "sortOrder": 0, + "type": "move", + }, +] `; diff --git a/src/navigation/components/MenuItems/tree.ts b/src/navigation/components/MenuItems/tree.ts index 4923d2be4..8341e9ce4 100644 --- a/src/navigation/components/MenuItems/tree.ts +++ b/src/navigation/components/MenuItems/tree.ts @@ -10,6 +10,7 @@ export interface TreeOperation { type: TreeOperationType; parentId?: string; sortOrder?: number; + simulatedMove?: boolean; } export const unknownTypeError = Error("Unknown type"); @@ -64,11 +65,11 @@ export function getItemId(item: MenuDetails_menu_items): string { export function getDiff( originalTree: TreeItem[], newTree: TreeItem[] -): TreeOperation { +): TreeOperation[] { const originalMap = treeToMap(originalTree, "root"); const newMap = treeToMap(newTree, "root"); - const diff: TreeOperation[] = Object.keys(newMap).map(key => { + const diff: TreeOperation[] = Object.keys(newMap).flatMap(key => { const originalNode = originalMap[key]; const newNode = newMap[key]; @@ -76,23 +77,56 @@ export function getDiff( if (patch.length > 0) { const addedNode = patch.find(operation => operation.type === "add"); + const removedNode = patch.find(operation => operation.type === "remove"); + if (!!addedNode) { + const changedParent = originalNode.length !== newNode.length; + const sortOrder = removedNode + ? addedNode.newPos - removedNode.oldPos + : addedNode.newPos; + + // This exists because backend doesn't recongize the position of the new node + // when it's moved from child to parent and/or up + // We have to make an additional move so that backend can sort the new tree correctly + // because without it the new node goes to the end of the parent node array by default + // SimulatedMove is removed before submit + if (changedParent && sortOrder !== originalNode.length) { + return [ + { + id: addedNode.items[0], + parentId: key === "root" ? undefined : key, + sortOrder: newNode.length - 1, + type: "move" as TreeOperationType, + simulatedMove: true + }, + { + id: addedNode.items[0], + parentId: key === "root" ? undefined : key, + sortOrder: + sortOrder - newNode.length < 0 + ? sortOrder - newNode.length + 1 + : sortOrder - newNode.length - 1, + type: "move" as TreeOperationType + } + ]; + } + return { id: addedNode.items[0], parentId: key === "root" ? undefined : key, - sortOrder: addedNode.newPos, + sortOrder, type: "move" as TreeOperationType }; } } }); - return diff.find(d => !!d); + return diff.filter(d => !!d); } export function getNodeData( item: MenuDetails_menu_items, - onChange: (operation: TreeOperation) => void, + onChange: (operations: TreeOperation[]) => void, onClick: (id: string, type: MenuItemType) => void, onEdit: (id: string) => void ): TreeItem { diff --git a/src/storybook/__snapshots__/Stories.test.ts.snap b/src/storybook/__snapshots__/Stories.test.ts.snap index d547dc4d7..615a0576e 100644 --- a/src/storybook/__snapshots__/Stories.test.ts.snap +++ b/src/storybook/__snapshots__/Stories.test.ts.snap @@ -103589,14 +103589,23 @@ exports[`Storyshots Views / Navigation / Menu details loading 1`] = `
- - ‌ - +
+
+
+ Add new menu item to begin creating menu +
+
+
+