web: fix toolbar item indentation when moving between groups (#8624)

* web: adjust item depth calculation during move operation

Signed-off-by: Arjun Vijay Prakash <arjunv.prakash12345@gmail.com>

* web: add tests for customize-toolbar.tsx

Signed-off-by: Arjun Vijay Prakash <arjunv.prakash12345@gmail.com>

* refactor(toolbar): export types and functions in customize-toolbar.tsx

Signed-off-by: Arjun Vijay Prakash <arjunv.prakash12345@gmail.com>

* refactor(tests): simplify and import types for customize-toolbar tests

Signed-off-by: Arjun Vijay Prakash <arjunv.prakash12345@gmail.com>

* refactor(toolbar): remove export from internal functions in customize-toolbar.tsx

Signed-off-by: Arjun Vijay Prakash <arjunv.prakash12345@gmail.com>

---------

Signed-off-by: Arjun Vijay Prakash <arjunv.prakash12345@gmail.com>

apps/web/__tests__/customize-toolbar.test.ts

@@ -0,0 +1,182 @@
+/*
+This file is part of the Notesnook project (https://notesnook.com/)
+
+Copyright (C) 2023 Streetwriters (Private) Limited
+
+This program is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+import {
+  Item,
+  Group,
+  Subgroup,
+  TreeNode,
+  moveItem
+} from "../src/dialogs/settings/components/customize-toolbar";
+import { describe, it, expect } from "vitest";
+
+describe("moveItem function", () => {
+  it("should correctly set depth when moving item from subgroup to main group", () => {
+    const group: Group = {
+      type: "group",
+      id: "group1",
+      title: "Group 1",
+      depth: 0
+    };
+
+    const subgroup: Subgroup = {
+      type: "group",
+      id: "subgroup1",
+      title: "Subgroup 1",
+      depth: 1
+    };
+
+    const item: Item = {
+      type: "item",
+      id: "item1",
+      title: "Item 1",
+      depth: 2, // currently in subgroup
+      toolId: "bold",
+      icon: "bold"
+    };
+
+    const items: TreeNode[] = [group, subgroup, item];
+
+    // move item from subgroup to main group
+    const result = moveItem(items, "item1", "group1");
+
+    // find the moved item
+    const movedItem = result.find((i) => i.id === "item1") as Item;
+
+    // the item should now have depth 1 (group depth + 1)
+    expect(movedItem.depth).toBe(1);
+  });
+
+  it("should correctly set depth when moving item from main group to subgroup", () => {
+    const group: Group = {
+      type: "group",
+      id: "group1",
+      title: "Group 1",
+      depth: 0
+    };
+
+    const item: Item = {
+      type: "item",
+      id: "item1",
+      title: "Item 1",
+      depth: 1, // currently in main group
+      toolId: "bold",
+      icon: "bold"
+    };
+
+    const subgroup: Subgroup = {
+      type: "group",
+      id: "subgroup1",
+      title: "Subgroup 1",
+      depth: 1
+    };
+
+    const items: TreeNode[] = [group, item, subgroup];
+
+    // move item from main group to subgroup
+    const result = moveItem(items, "item1", "subgroup1");
+
+    // find the moved item
+    const movedItem = result.find((i) => i.id === "item1") as Item;
+
+    // the item should now have depth 2 (subgroup depth + 1)
+    expect(movedItem.depth).toBe(2);
+  });
+
+  it("should correctly set depth when moving item to another item at same level", () => {
+    const group: Group = {
+      type: "group",
+      id: "group1",
+      title: "Group 1",
+      depth: 0
+    };
+
+    const item1: Item = {
+      type: "item",
+      id: "item1",
+      title: "Item 1",
+      depth: 1,
+      toolId: "bold",
+      icon: "bold"
+    };
+
+    const item2: Item = {
+      type: "item",
+      id: "item2",
+      title: "Item 2",
+      depth: 1,
+      toolId: "italic",
+      icon: "italic"
+    };
+
+    const items: TreeNode[] = [group, item1, item2];
+
+    // move item1 to item2's position
+    const result = moveItem(items, "item1", "item2");
+
+    // find the moved item
+    const movedItem = result.find((i) => i.id === "item1") as Item;
+
+    // the item should maintain the same depth as the target item
+    expect(movedItem.depth).toBe(1);
+  });
+
+  it("should correctly set depth when moving item from subgroup to another subgroup", () => {
+    const group: Group = {
+      type: "group",
+      id: "group1",
+      title: "Group 1",
+      depth: 0
+    };
+
+    const subgroup1: Subgroup = {
+      type: "group",
+      id: "subgroup1",
+      title: "Subgroup 1",
+      depth: 1
+    };
+
+    const item1: Item = {
+      type: "item",
+      id: "item1",
+      title: "Item 1",
+      depth: 2,
+      toolId: "bold",
+      icon: "bold"
+    };
+
+    const subgroup2: Subgroup = {
+      type: "group",
+      id: "subgroup2",
+      title: "Subgroup 2",
+      depth: 1
+    };
+
+    const items: TreeNode[] = [group, subgroup1, item1, subgroup2];
+
+    // move item from subgroup1 to subgroup2
+    const result = moveItem(items, "item1", "subgroup2");
+
+    // find the moved item
+    const movedItem = result.find((i) => i.id === "item1") as Item;
+
+    // the item should now have depth 2 (subgroup depth + 1)
+    expect(movedItem.depth).toBe(2);
+  });
+});

apps/web/src/dialogs/settings/components/customize-toolbar.tsx

@@ -421,19 +421,19 @@ type BaseTreeNode<Type extends TreeNodeType> = {
   depth: number;
 };
 
-type Subgroup = BaseTreeNode<"group"> & {
+export type Subgroup = BaseTreeNode<"group"> & {
   collapsed?: boolean;
 };
 
-type Group = BaseTreeNode<"group">;
+export type Group = BaseTreeNode<"group">;
 
-type Item = BaseTreeNode<"item"> & {
+export type Item = BaseTreeNode<"item"> & {
   toolId: ToolId;
   icon: string;
   collapsed?: boolean;
 };
 
-type TreeNode = Group | Item | Subgroup;
+export type TreeNode = Group | Item | Subgroup;
 
 function flatten(tools: ToolbarGroupDefinition[], depth = 0): TreeNode[] {
   const nodes: TreeNode[] = [];
@@ -544,7 +544,11 @@ function canMoveGroup(
   return true;
 }
 
-function moveItem(items: TreeNode[], fromId: string, toId: string): TreeNode[] {
+export function moveItem(
+  items: TreeNode[],
+  fromId: string,
+  toId: string
+): TreeNode[] {
   const fromIndex = items.findIndex((i) => i.id === fromId);
   const toIndex = items.findIndex((i) => i.id === toId);
 
@@ -555,13 +559,14 @@ function moveItem(items: TreeNode[], fromId: string, toId: string): TreeNode[] {
 
   const movingToGroup = isGroup(toItem) || isSubgroup(toItem);
 
-  // we need to adjust the item depth according to where the item
-  // is going to be moved.
-  if (fromItem.depth !== toItem.depth) fromItem.depth = toItem.depth;
-
-  // if we are moving to the start of the group, we need to adjust the
-  // depth accordingly.
-  if (movingToGroup) fromItem.depth = toItem.depth + 1;
+  // calculate the correct depth based on where the item is being moved
+  if (movingToGroup) {
+    // if moving to a group/subgroup, set depth to group's depth + 1
+    fromItem.depth = toItem.depth + 1;
+  } else {
+    // if moving to another item, set depth to match the target item's depth
+    fromItem.depth = toItem.depth;
+  }
 
   const newArray = arrayMove(items, fromIndex, toIndex);