From b8531fe260e9620fba615d985ae47b3ec7497375 Mon Sep 17 00:00:00 2001 From: Kenichi Kamiya Date: Fri, 25 Nov 2022 12:24:16 +0900 Subject: [PATCH 1/2] Refactor D&D helper function and the tests --- src/common/listHelpers.test.ts | 82 ++++++++++++++++++++++++++++ src/common/listHelpers.ts | 40 ++++++++++++++ src/userList/UserList.tsx | 5 +- src/userList/UserListHelpers.test.ts | 80 --------------------------- src/userList/UserListHelpers.ts | 31 ----------- 5 files changed, 124 insertions(+), 114 deletions(-) create mode 100644 src/common/listHelpers.test.ts delete mode 100644 src/userList/UserListHelpers.test.ts delete mode 100644 src/userList/UserListHelpers.ts diff --git a/src/common/listHelpers.test.ts b/src/common/listHelpers.test.ts new file mode 100644 index 00000000..67380c11 --- /dev/null +++ b/src/common/listHelpers.test.ts @@ -0,0 +1,82 @@ +import { describe, it } from "node:test"; +import assert from "node:assert"; +import { movePosition } from "./listHelpers"; + +describe("movePosition", () => { + it("does not change given users", () => { + const users = [ + "pankona", + "kachick", + "highwide", + "ohbarye", + "ravelll", + "ujihisa", + ]; + movePosition(users, "pankona", "kachick"); + + assert.deepStrictEqual(users, [ + "pankona", + "kachick", + "highwide", + "ohbarye", + "ravelll", + "ujihisa", + ]); + }); + + it("returns reordered users", () => { + const users = [ + "pankona", + "kachick", + "highwide", + "ohbarye", + "ravelll", + "ujihisa", + ]; + + assert.deepStrictEqual(movePosition(users, "pankona", "ravelll"), [ + "ravelll", + "pankona", + "kachick", + "highwide", + "ohbarye", + "ujihisa", + ]); + + assert.deepStrictEqual(movePosition(users, "ravelll", "pankona"), [ + "kachick", + "highwide", + "ohbarye", + "ravelll", + "pankona", + "ujihisa", + ]); + + assert.deepStrictEqual(movePosition(users, "highwide", "highwide"), [ + "pankona", + "kachick", + "highwide", + "ohbarye", + "ravelll", + "ujihisa", + ]); + + assert.deepStrictEqual(movePosition(users, "pankona", "ujihisa"), [ + "ujihisa", + "pankona", + "kachick", + "highwide", + "ohbarye", + "ravelll", + ]); + + assert.deepStrictEqual(movePosition(users, "ujihisa", "pankona"), [ + "kachick", + "highwide", + "ohbarye", + "ravelll", + "ujihisa", + "pankona", + ]); + }); +}); diff --git a/src/common/listHelpers.ts b/src/common/listHelpers.ts index b9b18b0b..864588e8 100644 --- a/src/common/listHelpers.ts +++ b/src/common/listHelpers.ts @@ -13,3 +13,43 @@ export const shuffleArray = (array: readonly T[]): T[] => { return dup; }; + +// Move an item to new position that already have another item. +// Former item will move to reasonable position, so kept the length of list. +// Items should be unique. +// Implemented for "drag-and-drop" use-case. +export const movePosition = ( + items: readonly T[], + mover: T, + moveTo: T, +): T[] => { + const from = items.findIndex( + (element) => element === mover, + ); + const to = items.findIndex( + (element) => element === moveTo, + ); + + if ((from < 0) || (to < 0)) { + throw new Error("given list does not contain given mover/moveTo"); + } + + const newIndex = (index: number): number => { + if (index > to && index <= from) { + return index - 1; + } else if (index === to) { + return from; + } else if (index < to && index >= from) { + return index + 1; + } + + return index; + }; + + const newItems = [...items]; + items.forEach((item, index) => { + newItems[newIndex(index)] = item; + }); + + return newItems; +}; diff --git a/src/userList/UserList.tsx b/src/userList/UserList.tsx index beb2652d..663f0927 100644 --- a/src/userList/UserList.tsx +++ b/src/userList/UserList.tsx @@ -8,8 +8,7 @@ import buttonCss from "../common/Button.module.css"; import UserRegister from "./UserRegister"; import { useSetPersistedUsers, useUsers } from "../common/usersContexts"; import Button from "../common/Button"; -import { newUsersAfterDropped } from "./UserListHelpers"; -import { shuffleArray } from "../common/listHelpers"; +import { movePosition, shuffleArray } from "../common/listHelpers"; const UserList = () => { const users = useUsers(); @@ -57,7 +56,7 @@ const UserList = () => { const droppedUsername = droppedMatchedGroups?.["droppedUsername"]; if (typeof droppedUsername === "string") { setPersistedUsers( - newUsersAfterDropped(users, user, droppedUsername), + movePosition(users, user, droppedUsername), ); } return false; diff --git a/src/userList/UserListHelpers.test.ts b/src/userList/UserListHelpers.test.ts deleted file mode 100644 index 681b18d8..00000000 --- a/src/userList/UserListHelpers.test.ts +++ /dev/null @@ -1,80 +0,0 @@ -import { newUsersAfterDropped } from "./UserListHelpers"; -import test from "node:test"; -import assert from "node:assert"; - -void test("newUsersAfterDropped does not change given users", () => { - const users = [ - "pankona", - "kachick", - "highwide", - "ohbarye", - "ravelll", - "ujihisa", - ]; - newUsersAfterDropped(users, "pankona", "kachick"); - - assert.deepStrictEqual(users, [ - "pankona", - "kachick", - "highwide", - "ohbarye", - "ravelll", - "ujihisa", - ]); -}); - -void test("newUsersAfterDropped returns reordered users", () => { - const users = [ - "pankona", - "kachick", - "highwide", - "ohbarye", - "ravelll", - "ujihisa", - ]; - - assert.deepStrictEqual(newUsersAfterDropped(users, "pankona", "ravelll"), [ - "ravelll", - "pankona", - "kachick", - "highwide", - "ohbarye", - "ujihisa", - ]); - - assert.deepStrictEqual(newUsersAfterDropped(users, "ravelll", "pankona"), [ - "kachick", - "highwide", - "ohbarye", - "ravelll", - "pankona", - "ujihisa", - ]); - - assert.deepStrictEqual(newUsersAfterDropped(users, "highwide", "highwide"), [ - "pankona", - "kachick", - "highwide", - "ohbarye", - "ravelll", - "ujihisa", - ]); - - assert.deepStrictEqual(newUsersAfterDropped(users, "pankona", "ujihisa"), [ - "ujihisa", - "pankona", - "kachick", - "highwide", - "ohbarye", - "ravelll", - ]); - - assert.deepStrictEqual(newUsersAfterDropped(users, "ujihisa", "pankona"), [ - "kachick", - "highwide", - "ohbarye", - "ravelll", - "ujihisa", - "pankona", - ]); -}); diff --git a/src/userList/UserListHelpers.ts b/src/userList/UserListHelpers.ts deleted file mode 100644 index 61c46934..00000000 --- a/src/userList/UserListHelpers.ts +++ /dev/null @@ -1,31 +0,0 @@ -export const newUsersAfterDropped = ( - originalUsers: readonly string[], - currentUser: string, - droppedUser: string, -): string[] => { - const droppedUserNewIndex = originalUsers.findIndex( - (element) => element === currentUser, - ); - const droppedUserOldIndex = originalUsers.findIndex( - (element) => element === droppedUser, - ); - const newUsers = [...originalUsers]; - - const newIndex = (index: number): number => { - if (index > droppedUserOldIndex && index <= droppedUserNewIndex) { - return index - 1; - } else if (index === droppedUserOldIndex) { - return droppedUserNewIndex; - } else if (index < droppedUserOldIndex && index >= droppedUserNewIndex) { - return index + 1; - } - - return index; - }; - - originalUsers.forEach((user, index) => { - newUsers[newIndex(index)] = user; - }); - - return newUsers; -}; From d730d78908df319f1acdeafe51837642f8b220fa Mon Sep 17 00:00:00 2001 From: Kenichi Kamiya Date: Fri, 25 Nov 2022 12:27:31 +0900 Subject: [PATCH 2/2] Standardize to describe/it style. It is similar to jest and will fit for subtests --- src/common/cssHelpers.test.ts | 12 +++++++----- src/session/timeHelpers.test.ts | 14 ++++++++------ 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/common/cssHelpers.test.ts b/src/common/cssHelpers.test.ts index 3ea81dd9..f0a3ac39 100644 --- a/src/common/cssHelpers.test.ts +++ b/src/common/cssHelpers.test.ts @@ -1,9 +1,11 @@ -import test from "node:test"; +import { describe, it } from "node:test"; import assert from "node:assert"; import { buildClassNames } from "./cssHelpers"; -void test("returns combined classnames as a string that trimmed empty", () => { - assert.strictEqual(buildClassNames(["button", "button-primary"]), "button button-primary"); - assert.strictEqual(buildClassNames(["button", undefined]), "button"); - assert.strictEqual(buildClassNames(["button", undefined, "hidden"]), "button hidden"); +describe("buildClassNames", () => { + it("returns combined classnames as a string that trimmed empty", () => { + assert.strictEqual(buildClassNames(["button", "button-primary"]), "button button-primary"); + assert.strictEqual(buildClassNames(["button", undefined]), "button"); + assert.strictEqual(buildClassNames(["button", undefined, "hidden"]), "button hidden"); + }); }); diff --git a/src/session/timeHelpers.test.ts b/src/session/timeHelpers.test.ts index 81a0a064..a80cb428 100644 --- a/src/session/timeHelpers.test.ts +++ b/src/session/timeHelpers.test.ts @@ -1,10 +1,12 @@ import { readableElapsedTime } from "./timeHelpers"; -import test from "node:test"; +import { describe, it } from "node:test"; import assert from "node:assert"; -void test("returns readable format", () => { - assert.strictEqual(readableElapsedTime(42), "00:00:42"); - assert.strictEqual(readableElapsedTime(142), "00:02:22"); - assert.strictEqual(readableElapsedTime(1001), "00:16:41"); - assert.strictEqual(readableElapsedTime(60000), "16:40:00"); +describe("readableElapsedTime", () => { + it("returns readable format", () => { + assert.strictEqual(readableElapsedTime(42), "00:00:42"); + assert.strictEqual(readableElapsedTime(142), "00:02:22"); + assert.strictEqual(readableElapsedTime(1001), "00:16:41"); + assert.strictEqual(readableElapsedTime(60000), "16:40:00"); + }); });