From aef0a6d340dcd387f94133590ff10e7c039f7fa1 Mon Sep 17 00:00:00 2001 From: ajnart Date: Sat, 8 Aug 2026 18:02:39 +0200 Subject: [PATCH] fix: stabilize responsive layout editing --- .../settings/_layout-preview.module.css | 4 ++ .../app/[locale]/boards/_layout-utils.spec.ts | 12 ++++++ .../src/app/[locale]/boards/_layout-utils.ts | 4 ++ .../items/actions/move-and-resize-item.ts | 9 ++-- .../items/actions/move-item-to-section.ts | 9 ++-- .../actions/test/move-and-resize-item.spec.ts | 12 ++---- .../actions/test/move-item-to-section.spec.ts | 12 ++---- .../components/board/items/item-actions.tsx | 14 ++++--- .../board/sections/section-actions.spec.ts | 42 +++++++++++++++++++ .../board/sections/section-actions.tsx | 42 +++++++++++++++---- .../router/board/test/grid-algorithm.spec.ts | 20 +++++++++ 11 files changed, 135 insertions(+), 45 deletions(-) create mode 100644 apps/nextjs/src/components/board/sections/section-actions.spec.ts diff --git a/apps/nextjs/src/app/[locale]/boards/[name]/settings/_layout-preview.module.css b/apps/nextjs/src/app/[locale]/boards/[name]/settings/_layout-preview.module.css index b5eb423be..4f34359bd 100644 --- a/apps/nextjs/src/app/[locale]/boards/[name]/settings/_layout-preview.module.css +++ b/apps/nextjs/src/app/[locale]/boards/[name]/settings/_layout-preview.module.css @@ -4,6 +4,10 @@ background: color-mix(in srgb, var(--mantine-color-body) 82%, var(--mantine-primary-color-light)); padding: var(--mantine-spacing-xs); min-height: 7rem; + max-height: min(24rem, 48dvh); + overflow-y: auto; + overscroll-behavior: contain; + scrollbar-gutter: stable; } .section { diff --git a/apps/nextjs/src/app/[locale]/boards/_layout-utils.spec.ts b/apps/nextjs/src/app/[locale]/boards/_layout-utils.spec.ts index c0add2c85..3124102f5 100644 --- a/apps/nextjs/src/app/[locale]/boards/_layout-utils.spec.ts +++ b/apps/nextjs/src/app/[locale]/boards/_layout-utils.spec.ts @@ -20,6 +20,18 @@ describe("layout representative widths", () => { }); describe("projectBoardLayout", () => { + test("returns exact saved positions without recursively re-projecting the active layout", () => { + const rootSection = new EmptySectionMockBuilder({ id: "root" }).build(); + const dynamicSection = new DynamicSectionMockBuilder({ id: "dynamic" }) + .addLayout({ layoutId: mobile.id, parentSectionId: rootSection.id, width: 3, height: 2 }) + .build(); + const board = new BoardMockBuilder().addLayout(mobile).addSections([rootSection, dynamicSection]).build(); + + expect(projectBoardLayout(board, mobile, mobile)).toEqual([ + expect.objectContaining({ id: dynamicSection.id, sectionId: rootSection.id, width: 3, height: 2 }), + ]); + }); + test("uses production ordering and sizing for items and dynamic sections", () => { const rootSection = new EmptySectionMockBuilder({ id: "root" }).build(); const dynamicSection = new DynamicSectionMockBuilder({ id: "dynamic" }) diff --git a/apps/nextjs/src/app/[locale]/boards/_layout-utils.ts b/apps/nextjs/src/app/[locale]/boards/_layout-utils.ts index 24dc6bd34..46da849d5 100644 --- a/apps/nextjs/src/app/[locale]/boards/_layout-utils.ts +++ b/apps/nextjs/src/app/[locale]/boards/_layout-utils.ts @@ -24,6 +24,10 @@ export const projectBoardLayout = ( ) => { const elements = getElementsForLayout(board, sourceLayout.id); + if (sourceLayout.id === targetLayout.id && sourceLayout.columnCount === targetLayout.columnCount) { + return elements; + } + return board.sections .filter((section) => section.kind !== "dynamic") .flatMap( diff --git a/apps/nextjs/src/components/board/items/actions/move-and-resize-item.ts b/apps/nextjs/src/components/board/items/actions/move-and-resize-item.ts index 0348b3f06..8b8f3a02f 100644 --- a/apps/nextjs/src/components/board/items/actions/move-and-resize-item.ts +++ b/apps/nextjs/src/components/board/items/actions/move-and-resize-item.ts @@ -1,9 +1,8 @@ -import { getCurrentLayout } from "@homarr/boards/context"; - import type { Board } from "~/app/[locale]/boards/_types"; export interface MoveAndResizeItemInput { itemId: string; + layoutId: string; xOffset: number; yOffset: number; width: number; @@ -11,10 +10,8 @@ export interface MoveAndResizeItemInput { } export const moveAndResizeItemCallback = - ({ itemId, ...layoutInput }: MoveAndResizeItemInput) => + ({ itemId, layoutId, ...layoutInput }: MoveAndResizeItemInput) => (previous: Board): Board => { - const currentLayout = getCurrentLayout(previous); - return { ...previous, items: previous.items.map((item) => @@ -23,7 +20,7 @@ export const moveAndResizeItemCallback = : { ...item, layouts: item.layouts.map((layout) => - layout.layoutId !== currentLayout + layout.layoutId !== layoutId ? layout : { ...layout, diff --git a/apps/nextjs/src/components/board/items/actions/move-item-to-section.ts b/apps/nextjs/src/components/board/items/actions/move-item-to-section.ts index 98d093826..6b7f8d314 100644 --- a/apps/nextjs/src/components/board/items/actions/move-item-to-section.ts +++ b/apps/nextjs/src/components/board/items/actions/move-item-to-section.ts @@ -1,9 +1,8 @@ -import { getCurrentLayout } from "@homarr/boards/context"; - import type { Board } from "~/app/[locale]/boards/_types"; export interface MoveItemToSectionInput { itemId: string; + layoutId: string; sectionId: string; xOffset: number; yOffset: number; @@ -12,10 +11,8 @@ export interface MoveItemToSectionInput { } export const moveItemToSectionCallback = - ({ itemId, ...layoutInput }: MoveItemToSectionInput) => + ({ itemId, layoutId, ...layoutInput }: MoveItemToSectionInput) => (board: Board): Board => { - const currentLayout = getCurrentLayout(board); - return { ...board, items: board.items.map((item) => @@ -24,7 +21,7 @@ export const moveItemToSectionCallback = : { ...item, layouts: item.layouts.map((layout) => - layout.layoutId !== currentLayout + layout.layoutId !== layoutId ? layout : { ...layout, diff --git a/apps/nextjs/src/components/board/items/actions/test/move-and-resize-item.spec.ts b/apps/nextjs/src/components/board/items/actions/test/move-and-resize-item.spec.ts index de3e5804b..d8c10cdd5 100644 --- a/apps/nextjs/src/components/board/items/actions/test/move-and-resize-item.spec.ts +++ b/apps/nextjs/src/components/board/items/actions/test/move-and-resize-item.spec.ts @@ -1,6 +1,4 @@ -import { describe, expect, test, vi } from "vitest"; - -import * as boardContext from "@homarr/boards/context"; +import { describe, expect, test } from "vitest"; import { moveAndResizeItemCallback } from "../move-and-resize-item"; import { BoardMockBuilder } from "./mocks/board-mock"; @@ -11,8 +9,6 @@ describe("moveItemToSectionCallback should move item in section", () => { // Arrange const itemToMove = "2"; const layoutId = "1"; - const spy = vi.spyOn(boardContext, "getCurrentLayout"); - spy.mockReturnValue(layoutId); const newPosition = { xOffset: 20, yOffset: 30, @@ -25,7 +21,7 @@ describe("moveItemToSectionCallback should move item in section", () => { const board = new BoardMockBuilder().addItem(itemA).addItem(itemB).addItem(itemC).build(); // Act - const updatedBoard = moveAndResizeItemCallback({ itemId: itemToMove, ...newPosition })(board); + const updatedBoard = moveAndResizeItemCallback({ itemId: itemToMove, layoutId, ...newPosition })(board); // Assert expect(updatedBoard.items).toHaveLength(3); @@ -43,8 +39,6 @@ describe("moveItemToSectionCallback should move item in section", () => { // Arrange const itemToMove = "2"; const layoutId = "1"; - const spy = vi.spyOn(boardContext, "getCurrentLayout"); - spy.mockReturnValue(layoutId); const newPosition = { xOffset: 20, yOffset: 30, @@ -56,7 +50,7 @@ describe("moveItemToSectionCallback should move item in section", () => { const board = new BoardMockBuilder().addItem(itemA).addItem(itemC).build(); // Act - const updatedBoard = moveAndResizeItemCallback({ itemId: itemToMove, ...newPosition })(board); + const updatedBoard = moveAndResizeItemCallback({ itemId: itemToMove, layoutId, ...newPosition })(board); // Assert expect(updatedBoard.items).toHaveLength(2); diff --git a/apps/nextjs/src/components/board/items/actions/test/move-item-to-section.spec.ts b/apps/nextjs/src/components/board/items/actions/test/move-item-to-section.spec.ts index 41e838ed2..5f930003a 100644 --- a/apps/nextjs/src/components/board/items/actions/test/move-item-to-section.spec.ts +++ b/apps/nextjs/src/components/board/items/actions/test/move-item-to-section.spec.ts @@ -1,6 +1,4 @@ -import { describe, expect, test, vi } from "vitest"; - -import * as boardContext from "@homarr/boards/context"; +import { describe, expect, test } from "vitest"; import { moveItemToSectionCallback } from "../move-item-to-section"; import { BoardMockBuilder } from "./mocks/board-mock"; @@ -11,8 +9,6 @@ describe("moveItemToSectionCallback should move item to section", () => { // Arrange const itemToMove = "2"; const layoutId = "1"; - const spy = vi.spyOn(boardContext, "getCurrentLayout"); - spy.mockReturnValue(layoutId); const newPosition = { sectionId: "3", xOffset: 20, @@ -26,7 +22,7 @@ describe("moveItemToSectionCallback should move item to section", () => { const board = new BoardMockBuilder().addItem(itemA).addItem(itemB).addItem(itemC).build(); // Act - const updatedBoard = moveItemToSectionCallback({ itemId: itemToMove, ...newPosition })(board); + const updatedBoard = moveItemToSectionCallback({ itemId: itemToMove, layoutId, ...newPosition })(board); // Assert expect(updatedBoard.items).toHaveLength(3); @@ -44,8 +40,6 @@ describe("moveItemToSectionCallback should move item to section", () => { // Arrange const itemToMove = "2"; const layoutId = "1"; - const spy = vi.spyOn(boardContext, "getCurrentLayout"); - spy.mockReturnValue(layoutId); const newPosition = { sectionId: "3", xOffset: 20, @@ -58,7 +52,7 @@ describe("moveItemToSectionCallback should move item to section", () => { const board = new BoardMockBuilder().addItem(itemA).addItem(itemC).build(); // Act - const updatedBoard = moveItemToSectionCallback({ itemId: itemToMove, ...newPosition })(board); + const updatedBoard = moveItemToSectionCallback({ itemId: itemToMove, layoutId, ...newPosition })(board); // Assert expect(updatedBoard.items).toHaveLength(2); diff --git a/apps/nextjs/src/components/board/items/item-actions.tsx b/apps/nextjs/src/components/board/items/item-actions.tsx index cc7b637b4..28f23179a 100644 --- a/apps/nextjs/src/components/board/items/item-actions.tsx +++ b/apps/nextjs/src/components/board/items/item-actions.tsx @@ -1,5 +1,6 @@ import { useCallback } from "react"; +import { useCurrentLayout } from "@homarr/boards/context"; import { useUpdateBoard } from "@homarr/boards/updater"; import type { BoardItemAdvancedOptions } from "@homarr/validation/shared"; @@ -31,6 +32,7 @@ interface UpdateItemIntegrations { export const useItemActions = () => { const { updateBoard } = useUpdateBoard(); + const currentLayoutId = useCurrentLayout(); const createItem = useCallback((input: CreateItemInput) => updateBoard(createItemCallback(input)), [updateBoard]); @@ -76,17 +78,17 @@ export const useItemActions = () => { ); const moveAndResizeItem = useCallback( - (input: MoveAndResizeItemInput) => { - updateBoard(moveAndResizeItemCallback(input)); + (input: Omit) => { + updateBoard(moveAndResizeItemCallback({ ...input, layoutId: currentLayoutId })); }, - [updateBoard], + [currentLayoutId, updateBoard], ); const moveItemToSection = useCallback( - (input: MoveItemToSectionInput) => { - updateBoard(moveItemToSectionCallback(input)); + (input: Omit) => { + updateBoard(moveItemToSectionCallback({ ...input, layoutId: currentLayoutId })); }, - [updateBoard], + [currentLayoutId, updateBoard], ); const removeItem = useCallback( diff --git a/apps/nextjs/src/components/board/sections/section-actions.spec.ts b/apps/nextjs/src/components/board/sections/section-actions.spec.ts new file mode 100644 index 000000000..32c413f97 --- /dev/null +++ b/apps/nextjs/src/components/board/sections/section-actions.spec.ts @@ -0,0 +1,42 @@ +import { describe, expect, test } from "vitest"; + +import { BoardMockBuilder } from "~/components/board/items/actions/test/mocks/board-mock"; +import { DynamicSectionMockBuilder } from "~/components/board/items/actions/test/mocks/dynamic-section-mock"; +import { EmptySectionMockBuilder } from "~/components/board/items/actions/test/mocks/empty-section-mock"; +import { wouldCreateSectionCycle } from "./section-actions"; + +describe("wouldCreateSectionCycle", () => { + test("rejects moving a dynamic section into itself or one of its descendants", () => { + const layoutId = "layout"; + const rootSection = new EmptySectionMockBuilder({ id: "root" }).build(); + const parentSection = new DynamicSectionMockBuilder({ id: "parent" }) + .addLayout({ layoutId, parentSectionId: rootSection.id }) + .build(); + const childSection = new DynamicSectionMockBuilder({ id: "child" }) + .addLayout({ layoutId, parentSectionId: parentSection.id }) + .build(); + const board = new BoardMockBuilder().addSections([rootSection, parentSection, childSection]).build(); + + expect( + wouldCreateSectionCycle(board, { + innerSectionId: parentSection.id, + sectionId: parentSection.id, + layoutId, + }), + ).toBe(true); + expect( + wouldCreateSectionCycle(board, { + innerSectionId: parentSection.id, + sectionId: childSection.id, + layoutId, + }), + ).toBe(true); + expect( + wouldCreateSectionCycle(board, { + innerSectionId: childSection.id, + sectionId: rootSection.id, + layoutId, + }), + ).toBe(false); + }); +}); diff --git a/apps/nextjs/src/components/board/sections/section-actions.tsx b/apps/nextjs/src/components/board/sections/section-actions.tsx index a978e712f..f2cdac751 100644 --- a/apps/nextjs/src/components/board/sections/section-actions.tsx +++ b/apps/nextjs/src/components/board/sections/section-actions.tsx @@ -1,8 +1,10 @@ import { useCallback } from "react"; -import { getCurrentLayout } from "@homarr/boards/context"; +import { useCurrentLayout } from "@homarr/boards/context"; import { useUpdateBoard } from "@homarr/boards/updater"; +import type { Board } from "~/app/[locale]/boards/_types"; + interface MoveAndResizeInnerSection { innerSectionId: string; xOffset: number; @@ -21,6 +23,7 @@ interface MoveInnerSectionToSection { export const useSectionActions = () => { const { updateBoard } = useUpdateBoard(); + const currentLayoutId = useCurrentLayout(); const moveAndResizeInnerSection = useCallback( ({ innerSectionId, ...positionProps }: MoveAndResizeInnerSection) => { @@ -31,12 +34,10 @@ export const useSectionActions = () => { if (section.id !== innerSectionId) return section; if (section.kind !== "container") return section; - const currentLayout = getCurrentLayout(previous); - return { ...section, layouts: section.layouts.map((layout) => { - if (layout.layoutId !== currentLayout) return layout; + if (layout.layoutId !== currentLayoutId) return layout; return { ...layout, ...positionProps, @@ -46,12 +47,16 @@ export const useSectionActions = () => { }), })); }, - [updateBoard], + [currentLayoutId, updateBoard], ); const moveInnerSectionToSection = useCallback( ({ innerSectionId, sectionId, ...positionProps }: MoveInnerSectionToSection) => { updateBoard((previous) => { + if (wouldCreateSectionCycle(previous, { innerSectionId, sectionId, layoutId: currentLayoutId })) { + return previous; + } + return { ...previous, sections: previous.sections.map((section) => { @@ -59,12 +64,10 @@ export const useSectionActions = () => { if (section.id !== innerSectionId) return section; if (section.kind !== "container") return section; - const currentLayout = getCurrentLayout(previous); - return { ...section, layouts: section.layouts.map((layout) => { - if (layout.layoutId !== currentLayout) return layout; + if (layout.layoutId !== currentLayoutId) return layout; return { ...layout, ...positionProps, @@ -76,7 +79,7 @@ export const useSectionActions = () => { }; }); }, - [updateBoard], + [currentLayoutId, updateBoard], ); return { @@ -84,3 +87,24 @@ export const useSectionActions = () => { moveInnerSectionToSection, }; }; + +export const wouldCreateSectionCycle = ( + board: Board, + { innerSectionId, sectionId, layoutId }: { innerSectionId: string; sectionId: string; layoutId: string }, +) => { + const visitedSectionIds = new Set(); + let currentSectionId: string | null = sectionId; + + while (currentSectionId) { + if (currentSectionId === innerSectionId) return true; + if (visitedSectionIds.has(currentSectionId)) return true; + visitedSectionIds.add(currentSectionId); + + const currentSection = board.sections.find((section) => section.id === currentSectionId); + if (currentSection?.kind !== "dynamic") return false; + + currentSectionId = currentSection.layouts.find((layout) => layout.layoutId === layoutId)?.parentSectionId ?? null; + } + + return false; +}; diff --git a/packages/api/src/router/board/test/grid-algorithm.spec.ts b/packages/api/src/router/board/test/grid-algorithm.spec.ts index 722c52724..ca028634b 100644 --- a/packages/api/src/router/board/test/grid-algorithm.spec.ts +++ b/packages/api/src/router/board/test/grid-algorithm.spec.ts @@ -78,6 +78,26 @@ describe("Grid Algorithm", () => { ); }, ); + + test("stops projecting a corrupted recursive section graph", () => { + const recursiveSection = algoItem({ id: "recursive", type: "section", width: 3 }); + const duplicateRecursiveLayout = algoItem({ + id: "recursive", + type: "section", + width: 3, + sectionId: "recursive", + }); + + const result = generateResponsiveGridFor({ + items: [recursiveSection, duplicateRecursiveLayout], + width: 3, + previousWidth: 12, + sectionId: ROOT_SECTION_ID, + }); + + expect(result.items).toContainEqual(expect.objectContaining({ id: "recursive", sectionId: ROOT_SECTION_ID })); + expect(result.items).toHaveLength(2); + }); }); const algoItem = (item: Partial): GridAlgorithmItem => ({