From d4bb014a9ba83770fd7880b48a4a5ab5362f6f75 Mon Sep 17 00:00:00 2001 From: Meier Lukas Date: Tue, 14 Jan 2025 19:55:21 +0100 Subject: [PATCH] fix(category): remove not always working and ignoring dynamic sections (#1939) --- .../category/actions/remove-category.ts | 111 ++++++++++++++++ .../actions/test/remove-category.spec.ts | 125 ++++++++++++++++++ .../sections/category/category-actions.ts | 68 +--------- 3 files changed, 241 insertions(+), 63 deletions(-) create mode 100644 apps/nextjs/src/components/board/sections/category/actions/remove-category.ts create mode 100644 apps/nextjs/src/components/board/sections/category/actions/test/remove-category.spec.ts diff --git a/apps/nextjs/src/components/board/sections/category/actions/remove-category.ts b/apps/nextjs/src/components/board/sections/category/actions/remove-category.ts new file mode 100644 index 000000000..19ec37601 --- /dev/null +++ b/apps/nextjs/src/components/board/sections/category/actions/remove-category.ts @@ -0,0 +1,111 @@ +import type { Board, CategorySection, DynamicSection, EmptySection, Section } from "~/app/[locale]/boards/_types"; + +export interface RemoveCategoryInput { + id: string; +} + +export const removeCategoryCallback = + (input: RemoveCategoryInput) => + (previous: Board): Board => { + const currentCategory = previous.sections.find( + (section): section is CategorySection => section.kind === "category" && section.id === input.id, + ); + if (!currentCategory) { + return previous; + } + + const emptySectionsAbove = previous.sections.filter( + (section): section is EmptySection => section.kind === "empty" && section.yOffset < currentCategory.yOffset, + ); + const aboveSection = emptySectionsAbove.sort((sectionA, sectionB) => sectionB.yOffset - sectionA.yOffset).at(0); + + const emptySectionsBelow = previous.sections.filter( + (section): section is EmptySection => section.kind === "empty" && section.yOffset > currentCategory.yOffset, + ); + const removedSection = emptySectionsBelow.sort((sectionA, sectionB) => sectionA.yOffset - sectionB.yOffset).at(0); + + if (!aboveSection || !removedSection) { + return previous; + } + + // Calculate the yOffset for the items in the currentCategory and removedWrapper to add them with the same offset to the aboveWrapper + const aboveYOffset = Math.max( + calculateYHeightWithOffsetForItems(aboveSection), + calculateYHeightWithOffsetForDynamicSections(previous.sections, aboveSection.id), + ); + const categoryYOffset = Math.max( + calculateYHeightWithOffsetForItems(currentCategory), + calculateYHeightWithOffsetForDynamicSections(previous.sections, currentCategory.id), + ); + + const previousCategoryItems = currentCategory.items.map((item) => ({ + ...item, + yOffset: item.yOffset + aboveYOffset, + })); + const previousBelowWrapperItems = removedSection.items.map((item) => ({ + ...item, + yOffset: item.yOffset + aboveYOffset + categoryYOffset, + })); + + return { + ...previous, + sections: [ + ...previous.sections.filter((section) => section.yOffset < aboveSection.yOffset && section.kind !== "dynamic"), + { + ...aboveSection, + items: [...aboveSection.items, ...previousCategoryItems, ...previousBelowWrapperItems], + }, + ...previous.sections + .filter( + (section): section is CategorySection | EmptySection => + section.yOffset > removedSection.yOffset && section.kind !== "dynamic", + ) + .map((section) => ({ + ...section, + position: section.yOffset - 2, + })), + ...previous.sections + .filter((section): section is DynamicSection => section.kind === "dynamic") + .map((dynamicSection) => { + // Move dynamic sections from removed section to above section with required yOffset + if (dynamicSection.parentSectionId === removedSection.id) { + return { + ...dynamicSection, + yOffset: dynamicSection.yOffset + aboveYOffset + categoryYOffset, + parentSectionId: aboveSection.id, + }; + } + + // Move dynamic sections from category to above section with required yOffset + if (dynamicSection.parentSectionId === currentCategory.id) { + return { + ...dynamicSection, + yOffset: dynamicSection.yOffset + aboveYOffset, + parentSectionId: aboveSection.id, + }; + } + + return dynamicSection; + }), + ], + }; + }; + +const calculateYHeightWithOffsetForDynamicSections = (sections: Section[], sectionId: string) => { + return sections.reduce((acc, section) => { + if (section.kind !== "dynamic" || section.parentSectionId !== sectionId) { + return acc; + } + + const yHeightWithOffset = section.yOffset + section.height; + if (yHeightWithOffset > acc) return yHeightWithOffset; + return acc; + }, 0); +}; + +const calculateYHeightWithOffsetForItems = (section: Section) => + section.items.reduce((acc, item) => { + const yHeightWithOffset = item.yOffset + item.height; + if (yHeightWithOffset > acc) return yHeightWithOffset; + return acc; + }, 0); diff --git a/apps/nextjs/src/components/board/sections/category/actions/test/remove-category.spec.ts b/apps/nextjs/src/components/board/sections/category/actions/test/remove-category.spec.ts new file mode 100644 index 000000000..89fd17a5a --- /dev/null +++ b/apps/nextjs/src/components/board/sections/category/actions/test/remove-category.spec.ts @@ -0,0 +1,125 @@ +/* eslint-disable @typescript-eslint/no-non-null-assertion */ +import { describe, expect, test } from "vitest"; + +import type { DynamicSection, Item, Section } from "~/app/[locale]/boards/_types"; +import { removeCategoryCallback } from "../remove-category"; + +describe("Remove Category", () => { + test.each([ + [3, [0, 1, 2, 3, 4, 5, 6], [0, 1, 2, 5, 6], [3, 4], 2], + [5, [0, 1, 2, 3, 4, 5, 6], [0, 1, 2, 3, 4], [5, 6], 4], + [1, [0, 1, 2, 3, 4, 5, 6], [0, 3, 4, 5, 6], [1, 2], 0], + [3, [0, 3, 6, 7, 8], [0, 7, 8], [3, 6], 0], + ])( + "should remove category", + (removeId, initialYOffsets, expectedYOffsets, expectedRemovals, expectedLocationOfItems) => { + const sections = createSections(initialYOffsets); + + const input = removeId.toString(); + + const result = removeCategoryCallback({ id: input })({ sections } as never); + + expect(result.sections.map((section) => parseInt(section.id, 10))).toEqual(expectedYOffsets); + expectedRemovals.forEach((expectedRemoval) => { + expect(result.sections.find((section) => section.id === expectedRemoval.toString())).toBeUndefined(); + }); + const aboveSection = result.sections.find((section) => section.id === expectedLocationOfItems.toString()); + expect(aboveSection?.items.map((item) => parseInt(item.id, 10))).toEqual( + expect.arrayContaining(expectedRemovals), + ); + }, + ); + + test("should correctly move items to above empty section", () => { + const initialYOffsets = [0, 1, 2, 3, 4, 5, 6]; + const sections: Section[] = createSections(initialYOffsets); + const aboveSection = sections.find((section) => section.yOffset === 2)!; + aboveSection.items = [ + createItem({ id: "above-1" }), + createItem({ id: "above-2", yOffset: 3, xOffset: 2, height: 2 }), + ]; + const removedCategory = sections.find((section) => section.yOffset === 3)!; + removedCategory.items = [ + createItem({ id: "category-1" }), + createItem({ id: "category-2", yOffset: 2, xOffset: 4, width: 4 }), + ]; + const removedEmptySection = sections.find((section) => section.yOffset === 4)!; + removedEmptySection.items = [ + createItem({ id: "below-1", xOffset: 5 }), + createItem({ id: "below-2", yOffset: 1, xOffset: 1, height: 2 }), + ]; + sections.push( + createDynamicSection({ + id: "7", + parentSectionId: "3", + yOffset: 7, + height: 3, + items: [createItem({ id: "dynamic-1" })], + }), + ); + + const input = "3"; + + const result = removeCategoryCallback({ id: input })({ sections } as never); + + expect(result.sections.map((section) => parseInt(section.id, 10))).toEqual([0, 1, 2, 5, 6, 7]); + const aboveSectionResult = result.sections.find((section) => section.id === "2")!; + expect(aboveSectionResult.items).toEqual( + expect.arrayContaining([ + createItem({ id: "above-1" }), + createItem({ id: "above-2", yOffset: 3, xOffset: 2, height: 2 }), + createItem({ id: "category-1", yOffset: 5 }), + createItem({ id: "category-2", yOffset: 7, xOffset: 4, width: 4 }), + createItem({ id: "below-1", yOffset: 15, xOffset: 5 }), + createItem({ id: "below-2", yOffset: 16, xOffset: 1, height: 2 }), + ]), + ); + const dynamicSection = result.sections.find((section): section is DynamicSection => section.id === "7")!; + expect(dynamicSection.yOffset).toBe(12); + expect(dynamicSection.parentSectionId).toBe("2"); + }); +}); + +const createItem = (item: Partial<{ id: string; width: number; height: number; yOffset: number; xOffset: number }>) => { + return { + id: item.id ?? "0", + kind: "app", + options: {}, + advancedOptions: { + customCssClasses: [], + }, + height: item.height ?? 1, + width: item.width ?? 1, + yOffset: item.yOffset ?? 0, + xOffset: item.xOffset ?? 0, + integrationIds: [], + } satisfies Item; +}; + +const createDynamicSection = ( + section: Partial< + Pick + >, +) => { + return { + id: section.id ?? "0", + kind: "dynamic", + height: section.height ?? 1, + width: section.width ?? 1, + yOffset: section.yOffset ?? 0, + xOffset: section.xOffset ?? 0, + parentSectionId: section.parentSectionId ?? "0", + items: section.items ?? [], + } satisfies DynamicSection; +}; + +const createSections = (initialYOffsets: number[]) => { + return initialYOffsets.map((yOffset, index) => ({ + id: yOffset.toString(), + kind: index % 2 === 0 ? "empty" : "category", + name: "Category", + yOffset, + xOffset: 0, + items: [createItem({ id: yOffset.toString() })], + })) satisfies Section[]; +}; diff --git a/apps/nextjs/src/components/board/sections/category/category-actions.ts b/apps/nextjs/src/components/board/sections/category/category-actions.ts index 01e54a2b8..6bdcdc9ba 100644 --- a/apps/nextjs/src/components/board/sections/category/category-actions.ts +++ b/apps/nextjs/src/components/board/sections/category/category-actions.ts @@ -2,10 +2,12 @@ import { useCallback } from "react"; import { createId } from "@homarr/db/client"; -import type { CategorySection, EmptySection, Section } from "~/app/[locale]/boards/_types"; +import type { CategorySection, EmptySection } from "~/app/[locale]/boards/_types"; import { useUpdateBoard } from "~/app/[locale]/boards/(content)/_client"; import type { MoveCategoryInput } from "./actions/move-category"; import { moveCategoryCallback } from "./actions/move-category"; +import type { RemoveCategoryInput } from "./actions/remove-category"; +import { removeCategoryCallback } from "./actions/remove-category"; interface AddCategory { name: string; @@ -17,10 +19,6 @@ interface RenameCategory { name: string; } -interface RemoveCategory { - id: string; -} - export const useCategoryActions = () => { const { updateBoard } = useUpdateBoard(); @@ -132,57 +130,8 @@ export const useCategoryActions = () => { ); const removeCategory = useCallback( - ({ id: categoryId }: RemoveCategory) => { - updateBoard((previous) => { - const currentCategory = previous.sections.find( - (section): section is CategorySection => section.kind === "category" && section.id === categoryId, - ); - if (!currentCategory) return previous; - - const aboveWrapper = previous.sections.find( - (section): section is EmptySection => - section.kind === "empty" && section.yOffset === currentCategory.yOffset - 1, - ); - - const removedWrapper = previous.sections.find( - (section): section is EmptySection => - section.kind === "empty" && section.yOffset === currentCategory.yOffset + 1, - ); - - if (!aboveWrapper || !removedWrapper) return previous; - - // Calculate the yOffset for the items in the currentCategory and removedWrapper to add them with the same offset to the aboveWrapper - const aboveYOffset = calculateYHeightWithOffset(aboveWrapper); - const categoryYOffset = calculateYHeightWithOffset(currentCategory); - - const previousCategoryItems = currentCategory.items.map((item) => ({ - ...item, - yOffset: item.yOffset + aboveYOffset, - })); - const previousBelowWrapperItems = removedWrapper.items.map((item) => ({ - ...item, - yOffset: item.yOffset + aboveYOffset + categoryYOffset, - })); - - return { - ...previous, - sections: [ - ...previous.sections.filter((section) => section.yOffset < currentCategory.yOffset - 1), - { - ...aboveWrapper, - items: [...aboveWrapper.items, ...previousCategoryItems, ...previousBelowWrapperItems], - }, - ...previous.sections - .filter( - (section): section is CategorySection | EmptySection => section.yOffset >= currentCategory.yOffset + 2, - ) - .map((section) => ({ - ...section, - position: section.yOffset - 2, - })), - ], - }; - }); + (input: RemoveCategoryInput) => { + updateBoard(removeCategoryCallback(input)); }, [updateBoard], ); @@ -195,10 +144,3 @@ export const useCategoryActions = () => { removeCategory, }; }; - -const calculateYHeightWithOffset = (section: Section) => - section.items.reduce((acc, item) => { - const yHeightWithOffset = item.yOffset + item.height; - if (yHeightWithOffset > acc) return yHeightWithOffset; - return acc; - }, 0);