From 44942e697613caa20e0706ac553eb1ed0ef11ef5 Mon Sep 17 00:00:00 2001 From: Karim Date: Sat, 4 Jul 2026 05:01:15 +0200 Subject: [PATCH] =?UTF-8?q?Undo/Redo=20f=C3=BCr=20Projekt-=C3=84nderungen?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit setProject wird jetzt von einer History gewrappt (undoStack/redoStack, Limit 100). Echte Aenderungen (Referenzvergleich) landen auf dem Undo-Stack, eine neue Aktion leert den Redo-Stack. Hochfrequente Drag-Mutatoren (Griffe/Body-Move) bekommen einen coalesceKey, damit ein Drag nicht hunderte Einzelschritte erzeugt. Tastatur-Bindung (Ctrl+Z/Ctrl+Shift+Z/Ctrl+Y) folgt im TopBar-Commit (App.tsx). --- src/state/appStore.ts | 10 +- src/state/history.test.ts | 136 +++++++++++++++++++++++++ src/state/historySlice.ts | 107 ++++++++++++++++++++ src/state/projectSlice.ts | 207 +++++++++++++++++++++++++++----------- 4 files changed, 398 insertions(+), 62 deletions(-) create mode 100644 src/state/history.test.ts create mode 100644 src/state/historySlice.ts diff --git a/src/state/appStore.ts b/src/state/appStore.ts index bf50d69..e0be59c 100644 --- a/src/state/appStore.ts +++ b/src/state/appStore.ts @@ -10,6 +10,7 @@ import { createStore } from "./store"; import type { StoreApi } from "./store"; import { createProjectSlice } from "./projectSlice"; import type { ProjectSlice } from "./projectSlice"; +import type { HistorySlice } from "./historySlice"; import { createSelectionSlice } from "./selectionSlice"; import type { SelectionSlice } from "./selectionSlice"; import { createViewSlice } from "./viewSlice"; @@ -21,6 +22,7 @@ import type { SiteSlice } from "./siteSlice"; /** Gesamtzustand: Vereinigung aller Slice-Felder + Actions. */ export type RootState = ProjectSlice & + HistorySlice & SelectionSlice & ViewSlice & LayoutSlice & @@ -29,8 +31,12 @@ export type RootState = ProjectSlice & const store = createStore((api) => ({ // Jede Slice-Factory bekommt die volle RootState-API. Die Slices tippen nur // ihren eigenen Ausschnitt; die Cast auf die enge Slice-API ist sicher, weil - // `set/get` immer über den Gesamt-RootState laufen. - ...createProjectSlice(api as unknown as StoreApi), + // `set/get` immer über den Gesamt-RootState laufen. ProjectSlice bringt die + // History (undo/redo) gleich mit (setProject ist der einzige Durchlauf- + // punkt jeder Projekt-Mutation, siehe projectSlice.ts). + ...createProjectSlice( + api as unknown as StoreApi, + ), ...createSelectionSlice(api as unknown as StoreApi), ...createViewSlice(api as unknown as StoreApi), ...createLayoutSlice(api as unknown as StoreApi), diff --git a/src/state/history.test.ts b/src/state/history.test.ts new file mode 100644 index 0000000..e05c59c --- /dev/null +++ b/src/state/history.test.ts @@ -0,0 +1,136 @@ +/** + * Undo/Redo für das Projekt-Modell (`src/state/historySlice.ts` + + * `setProject`-Wrapper in `src/state/projectSlice.ts`). + * + * • Ein diskreter Store-Aufruf (z. B. `toggleLevelVisible`) muss GENAU einen + * Undo-Schritt erzeugen. + * • `undo`/`redo` müssen den jeweils anderen Stack korrekt befüllen und + * dürfen bei leerem Stack nicht crashen (No-Op). + * • Eine neue Aktion nach `undo()` muss den Redo-Stack verwerfen. + * • Hochfrequente Drag-Aktionen (z. B. `moveGripOf`, bei JEDEM Maus-Move- + * Schritt aufgerufen statt über ein Draft/Commit-Muster) müssen zu EINEM + * Undo-Schritt koalesziert werden, nicht zu einem pro Aufruf. + */ + +import { describe, it, expect, beforeEach } from "vitest"; +import { getState, setState } from "./appStore"; +import { sampleProject } from "../model/sampleProject"; +import { HISTORY_LIMIT } from "./historySlice"; + +// Frischen Ausgangszustand vor jedem Test herstellen: eigenes Projekt + +// leere Stacks, damit die Tests unabhängig voneinander laufen (derselbe +// Store ist ein Singleton über die ganze Datei hinweg). +beforeEach(() => { + setState({ + project: sampleProject, + undoStack: [], + redoStack: [], + canUndo: false, + canRedo: false, + }); +}); + +describe("Undo/Redo", () => { + it("undo macht eine Projekt-Änderung rückgängig", () => { + const before = getState().project; + getState().toggleLevelVisible("eg"); + const afterToggle = getState().project; + + expect(afterToggle).not.toBe(before); + expect(afterToggle.drawingLevels.find((l) => l.id === "eg")?.visible).toBe(false); + expect(getState().canUndo).toBe(true); + + getState().undo(); + + expect(getState().project.drawingLevels.find((l) => l.id === "eg")?.visible).toBe(true); + expect(getState().canUndo).toBe(false); + expect(getState().canRedo).toBe(true); + }); + + it("redo stellt die rückgängig gemachte Änderung wieder her", () => { + getState().toggleLevelVisible("eg"); + getState().undo(); + + getState().redo(); + + expect(getState().project.drawingLevels.find((l) => l.id === "eg")?.visible).toBe(false); + expect(getState().canRedo).toBe(false); + expect(getState().canUndo).toBe(true); + }); + + it("undo bei leerem Stack ist ein No-Op (kein Crash)", () => { + expect(getState().undoStack.length).toBe(0); + const before = getState().project; + + expect(() => getState().undo()).not.toThrow(); + + expect(getState().project).toBe(before); + expect(getState().canUndo).toBe(false); + }); + + it("redo bei leerem Stack ist ein No-Op (kein Crash)", () => { + expect(getState().redoStack.length).toBe(0); + const before = getState().project; + + expect(() => getState().redo()).not.toThrow(); + + expect(getState().project).toBe(before); + expect(getState().canRedo).toBe(false); + }); + + it("eine neue Aktion nach undo() verwirft den Redo-Stack", () => { + getState().toggleLevelVisible("eg"); + getState().undo(); + expect(getState().redoStack.length).toBe(1); + + getState().toggleLevelVisible("og"); + + expect(getState().redoStack.length).toBe(0); + expect(getState().canRedo).toBe(false); + // Die neue Aktion selbst bleibt natürlich undo-fähig. + expect(getState().canUndo).toBe(true); + }); + + it("die allererste Änderung bleibt exakt EIN Undo-Schritt (kein Start-Eintrag)", () => { + expect(getState().undoStack.length).toBe(0); + getState().toggleLevelVisible("eg"); + expect(getState().undoStack.length).toBe(1); + }); + + it("history-Stack verwirft die ältesten Einträge über HISTORY_LIMIT hinaus", () => { + for (let i = 0; i < HISTORY_LIMIT + 10; i++) { + getState().toggleLevelVisible("eg"); + } + expect(getState().undoStack.length).toBe(HISTORY_LIMIT); + }); + + it("ein Grip-Drag (viele setProject-Aufrufe in Serie) koalesziert zu EINEM Undo-Schritt", () => { + const w1Before = getState().project.walls.find((w) => w.id === "W1"); + expect(w1Before?.start).toEqual({ x: 0, y: 0 }); + + // Simuliert einen Drag: viele schnelle moveGripOf-Aufrufe auf DASSELBE + // Element/denselben Griff, wie sie bei jedem Maus-Move-Event entstehen + // (siehe gripHandlers.onGripMove in App.tsx → moveGripOf). + for (let i = 1; i <= 20; i++) { + getState().moveGripOf(null, "W1", 0, { x: i * 0.1, y: 0 }); + } + + const w1After = getState().project.walls.find((w) => w.id === "W1"); + expect(w1After?.start).toEqual({ x: 2, y: 0 }); + // Trotz 20 Aufrufen: genau EIN Undo-Schritt. + expect(getState().undoStack.length).toBe(1); + + getState().undo(); + + const w1Restored = getState().project.walls.find((w) => w.id === "W1"); + expect(w1Restored?.start).toEqual({ x: 0, y: 0 }); + expect(getState().undoStack.length).toBe(0); + }); + + it("zwei Drags auf UNTERSCHIEDLICHE Griffe erzeugen ZWEI Undo-Schritte", () => { + getState().moveGripOf(null, "W1", 0, { x: 1, y: 0 }); + getState().moveGripOf(null, "W1", 1, { x: 6, y: 0 }); + + expect(getState().undoStack.length).toBe(2); + }); +}); diff --git a/src/state/historySlice.ts b/src/state/historySlice.ts new file mode 100644 index 0000000..7cc44b7 --- /dev/null +++ b/src/state/historySlice.ts @@ -0,0 +1,107 @@ +// History-Slice: Undo/Redo für das Projekt-Modell. Reine Snapshot-Historie — +// dank immutabler Updates in `projectSlice.ts` (jede echte Änderung liefert +// eine NEUE `Project`-Referenz mit structural sharing) ist ein Stack aus +// `Project`-Werten billig (kein Deep-Clone nötig, unveränderte Teilbäume +// werden zwischen den Snapshots geteilt). +// +// Die eigentliche Buchführung (WANN wird gepusht, Koaleszenz von Drag-Serien) +// lebt bewusst in `projectSlice.ts` bei `setProject` selbst — das ist die +// EINZIGE Stelle, durch die jede Projekt-Mutation läuft. Dieses Modul liefert +// nur den reaktiven Zustand (Stacks + Flags) und die `undo`/`redo`-Aktionen, +// die DIREKT über `set()` schreiben (NICHT über `setProject`), damit sie +// keinen neuen History-Eintrag erzeugen (sonst Endlosschleife/kaputte +// Historie). Bezeichner englisch, Kommentare deutsch (CONVENTIONS.md). + +import type { Project } from "../model/types"; +import type { StoreApi } from "./store"; + +/** Obergrenze der Undo-Tiefe — ältere Einträge fallen hinten raus (Speicher/Performance). */ +export const HISTORY_LIMIT = 100; + +/** Felder, die diese Slice in den RootState beisteuert. */ +export interface HistorySlice { + /** Ältere Projekt-Stände (oben = zuletzt verlassener Stand, für `undo`). */ + undoStack: Project[]; + /** Durch `undo` verlassene Stände (oben = zuletzt verlassener, für `redo`). */ + redoStack: Project[]; + /** Abgeleitet aus `undoStack.length > 0` — bequem für UI-Bindings (Buttons/Menüs). */ + canUndo: boolean; + /** Abgeleitet aus `redoStack.length > 0`. */ + canRedo: boolean; + /** Einen Schritt zurück. No-Op (kein Crash), wenn der Undo-Stack leer ist. */ + undo: () => void; + /** Einen Schritt vor. No-Op (kein Crash), wenn der Redo-Stack leer ist. */ + redo: () => void; +} + +/** + * Reines Patch-Berechnen für einen History-Push — von `setProject` (in + * `projectSlice.ts`) bei jeder ECHTEN Projekt-Änderung aufgerufen (kein + * Store-Zugriff nötig, daher hier als freie Funktion statt Store-Aktion: so + * lässt sich der Push in EINEM gemeinsamen `set()`-Aufruf mit der eigentlichen + * `project`-Änderung zusammenfassen, statt einen zweiten, verschachtelten + * `set()`-Aufruf auszulösen). + * + * Eine neue Aktion verwirft immer die Redo-Historie (Standardverhalten jedes + * Undo/Redo-Systems: sobald der Nutzer nach einem `undo()` etwas Neues tut, + * ist der alte "vordere" Zweig weg). + */ +export function pushHistoryPatch( + undoStack: Project[], + prevProject: Project, +): Pick { + const next = [...undoStack, prevProject]; + if (next.length > HISTORY_LIMIT) next.shift(); + return { undoStack: next, redoStack: [], canUndo: true, canRedo: false }; +} + +/** + * Baut `undo`/`redo` + initialen Zustand. `onAfterJump` wird nach jedem + * Sprung aufgerufen (projectSlice.ts nutzt das, um die Koaleszenz-Buchführung + * für Drag-Serien zurückzusetzen — sonst könnte eine neue Aktion direkt nach + * einem `undo()` fälschlich mit der VORHERIGEN Serie verschmolzen und dadurch + * NIE auf dem Undo-Stack landen). + */ +export function createHistorySlice( + api: StoreApi, + onAfterJump?: () => void, +): HistorySlice { + const { set, get } = api; + + return { + undoStack: [], + redoStack: [], + canUndo: false, + canRedo: false, + + undo: () => { + const { undoStack, redoStack, project } = get(); + if (undoStack.length === 0) return; + const prev = undoStack[undoStack.length - 1]; + const rest = undoStack.slice(0, -1); + set({ + project: prev, + undoStack: rest, + redoStack: [...redoStack, project], + canUndo: rest.length > 0, + canRedo: true, + }); + onAfterJump?.(); + }, + + redo: () => { + const { undoStack, redoStack, project } = get(); + if (redoStack.length === 0) return; + const next = redoStack[redoStack.length - 1]; + const rest = redoStack.slice(0, -1); + set({ + project: next, + undoStack: [...undoStack, project], + redoStack: rest, + canUndo: true, + canRedo: rest.length > 0, + }); + onAfterJump?.(); + }, + }; +} diff --git a/src/state/projectSlice.ts b/src/state/projectSlice.ts index d6a2f39..71f1b7d 100644 --- a/src/state/projectSlice.ts +++ b/src/state/projectSlice.ts @@ -28,6 +28,8 @@ import type { CopyMode, TransformOp, TransformSelection } from "../tools/transfo import { centroid as roomCentroid } from "../geometry/roomArea"; import { t } from "../i18n"; import type { StoreApi } from "./store"; +import { createHistorySlice, pushHistoryPatch } from "./historySlice"; +import type { HistorySlice } from "./historySlice"; /** Felder, die diese Slice in den RootState beisteuert. */ export interface ProjectSlice { @@ -265,24 +267,76 @@ export interface ProjectSlice { /** * Minimaler View-Ausschnitt, den Projekt-Aktionen lesen/schreiben dürfen * (Cross-Slice). Der Store komponiert ProjectSlice mit dieser Sicht, sodass - * `get()`/`set` typsicher auf `activeLevelId` zugreifen können. + * `get()`/`set` typsicher auf `activeLevelId` zugreifen können. Die + * History-Felder gehören ebenfalls dazu: `setProject` (unten) ist der EINZIGE + * Durchlaufpunkt jeder Projekt-Mutation und pusht dort bei jeder ECHTEN + * Änderung auf `undoStack`. */ type ProjectSliceDeps = { activeLevelId: string; -}; +} & HistorySlice; export function createProjectSlice( api: StoreApi, -): ProjectSlice { +): ProjectSlice & HistorySlice { const { set, get } = api; - // Bequemer immutabler Projekt-Updater (wie setProject in App). - const setProject: ProjectSlice["setProject"] = (next) => - set((s) => ({ - project: typeof next === "function" ? (next as (p: Project) => Project)(s.project) : next, - })); + // Koaleszenz-Buchführung für Drag-Serien (Griff ziehen, Element verschieben, + // Kante ziehen, …): diese Aktionen rufen `setProject` bei JEDEM + // Maus-Move-Schritt auf (kein separates Draft/Preview + Commit-Muster wie + // bei den Zeichenwerkzeugen — dort läuft die Vorschau über lokalen + // `draft`-State in App.tsx, und `setProject` wird erst einmal beim Commit + // aufgerufen, siehe `applyToolResult`/`onToolCommit` in App.tsx). Ohne + // Koaleszenz würde EIN Drag zig Undo-Schritte erzeugen. Ephemer (wie ein + // Ref) — kein reaktiver State nötig, wird bei jedem `setProject`-Aufruf neu + // bewertet und bei `undo`/`redo` zurückgesetzt (siehe `onAfterJump` unten). + let lastCoalesceKey: string | undefined; + let lastCoalesceAt = 0; + const COALESCE_WINDOW_MS = 400; + + const history = createHistorySlice( + api as unknown as StoreApi, + () => { + // Nach einem Sprung (undo/redo) muss die NÄCHSTE Aktion garantiert + // pushen, auch wenn sie zufällig denselben coalesceKey trägt wie die + // Serie vor dem Sprung — sonst würde sie mit der alten Serie + // verschmolzen und verschwände spurlos (kein Undo-Eintrag). + lastCoalesceKey = undefined; + }, + ); + + // Bequemer immutabler Projekt-Updater (wie setProject in App). Zweiter, + // optionaler Parameter `coalesceKey`: NUR von den paar bekannten + // Hochfrequenz-Aktionen (Grip-Drag & Co., s. u.) gesetzt — alle anderen ~50 + // Aktionen in dieser Datei rufen `setProject(fn)` ohne Key auf und pushen + // damit IMMER diskret (kein Koaleszieren zwischen unterschiedlichen + // Aktionen möglich, auch nicht bei zufällig identischem Timing in Tests). + const setProject = ( + next: Project | ((p: Project) => Project), + coalesceKey?: string, + ) => + set((s) => { + const nextProject = + typeof next === "function" ? (next as (p: Project) => Project)(s.project) : next; + // No-Op-Schutz: echte Immutable-Updates liefern bei tatsächlicher + // Änderung immer eine NEUE Referenz — bleibt sie gleich, gab es keine + // Änderung, also kein History-Eintrag (und `set` selbst benachrichtigt + // dann ebenfalls nicht, da der Patch leer ist). + if (nextProject === s.project) return {}; + const now = Date.now(); + const coalesced = + coalesceKey !== undefined && + coalesceKey === lastCoalesceKey && + now - lastCoalesceAt < COALESCE_WINDOW_MS; + lastCoalesceKey = coalesceKey; + lastCoalesceAt = now; + if (coalesced) return { project: nextProject }; + return { project: nextProject, ...pushHistoryPatch(s.undoStack, s.project) }; + }); return { + ...history, + project: sampleProject, setProject, @@ -685,14 +739,26 @@ export function createProjectSlice( })), // ── Editier-Griffe ───────────────────────────────────────────────────── + // Diese Aktionen laufen AD-HOC bei jedem Maus-Move-Schritt eines Drags + // (kein Draft/Commit-Muster wie bei den Zeichenwerkzeugen) — daher mit + // coalesceKey, damit EIN Drag EINEN Undo-Schritt erzeugt statt vieler. moveGripOf: (drawingId, wallId, index, pt) => - setProject((p) => moveGrip(p, drawingId, wallId, index, pt)), + setProject( + (p) => moveGrip(p, drawingId, wallId, index, pt), + `moveGripOf:${drawingId ?? ""}:${wallId ?? ""}:${index}`, + ), moveElementByOf: (drawingId, wallId, delta) => - setProject((p) => moveElementBy(p, drawingId, wallId, delta)), + setProject( + (p) => moveElementBy(p, drawingId, wallId, delta), + `moveElementByOf:${drawingId ?? ""}:${wallId ?? ""}`, + ), moveEdgeOf: (drawingId, wallId, aIndex, bIndex, delta) => - setProject((p) => moveEdge(p, drawingId, wallId, aIndex, bIndex, delta)), + setProject( + (p) => moveEdge(p, drawingId, wallId, aIndex, bIndex, delta), + `moveEdgeOf:${drawingId ?? ""}:${wallId ?? ""}:${aIndex}:${bIndex}`, + ), // ── Selektions-Attribute ─────────────────────────────────────────────── setElementColor: (kind, id, color) => @@ -781,25 +847,36 @@ export function createProjectSlice( setProject((p) => setWallThickness(p, wallId, thickness)), // ── Decken-Editieren ────────────────────────────────────────────────── + // (coalesceKey: siehe Kommentar bei moveGripOf — auch hier läuft die + // Mutation bei jedem Maus-Move-Schritt eines Drags.) moveCeilingGrip: (ceilingId, index, pt) => - setProject((p) => mapCeiling(p, ceilingId, (c) => ({ - ...c, - outline: c.outline.map((v, i) => (i === index ? pt : v)), - }))), + setProject( + (p) => mapCeiling(p, ceilingId, (c) => ({ + ...c, + outline: c.outline.map((v, i) => (i === index ? pt : v)), + })), + `moveCeilingGrip:${ceilingId}:${index}`, + ), moveCeilingBy: (ceilingId, delta) => - setProject((p) => mapCeiling(p, ceilingId, (c) => ({ - ...c, - outline: c.outline.map((v) => ({ x: v.x + delta.x, y: v.y + delta.y })), - }))), + setProject( + (p) => mapCeiling(p, ceilingId, (c) => ({ + ...c, + outline: c.outline.map((v) => ({ x: v.x + delta.x, y: v.y + delta.y })), + })), + `moveCeilingBy:${ceilingId}`, + ), moveCeilingEdge: (ceilingId, aIndex, bIndex, delta) => - setProject((p) => mapCeiling(p, ceilingId, (c) => ({ - ...c, - outline: c.outline.map((v, i) => - i === aIndex || i === bIndex ? { x: v.x + delta.x, y: v.y + delta.y } : v, - ), - }))), + setProject( + (p) => mapCeiling(p, ceilingId, (c) => ({ + ...c, + outline: c.outline.map((v, i) => + i === aIndex || i === bIndex ? { x: v.x + delta.x, y: v.y + delta.y } : v, + ), + })), + `moveCeilingEdge:${ceilingId}:${aIndex}:${bIndex}`, + ), updateCeiling: (id, patch) => setProject((p) => mapCeiling(p, id, (c) => ({ ...c, ...patch }))), @@ -815,55 +892,65 @@ export function createProjectSlice( })), // ── Treppen-Editieren ────────────────────────────────────────────────── + // (coalesceKey: siehe Kommentar bei moveGripOf.) moveStairBy: (stairId, delta) => - setProject((p) => - mapStair(p, stairId, (s) => ({ - ...s, - start: { x: s.start.x + delta.x, y: s.start.y + delta.y }, - ...(s.center - ? { center: { x: s.center.x + delta.x, y: s.center.y + delta.y } } - : {}), - })), + setProject( + (p) => + mapStair(p, stairId, (s) => ({ + ...s, + start: { x: s.start.x + delta.x, y: s.start.y + delta.y }, + ...(s.center + ? { center: { x: s.center.x + delta.x, y: s.center.y + delta.y } } + : {}), + })), + `moveStairBy:${stairId}`, ), updateStair: (id, patch) => setProject((p) => mapStair(p, id, (s) => ({ ...s, ...patch }))), // ── Raum-Editieren ────────────────────────────────────────────────────── + // (coalesceKey: siehe Kommentar bei moveGripOf.) moveRoomBy: (roomId, delta) => - setProject((p) => - mapRoom(p, roomId, (r) => ({ - ...r, - boundary: r.boundary.map((v) => ({ x: v.x + delta.x, y: v.y + delta.y })), - ...(r.stampAnchor - ? { - stampAnchor: { - x: r.stampAnchor.x + delta.x, - y: r.stampAnchor.y + delta.y, - }, - } - : {}), - })), + setProject( + (p) => + mapRoom(p, roomId, (r) => ({ + ...r, + boundary: r.boundary.map((v) => ({ x: v.x + delta.x, y: v.y + delta.y })), + ...(r.stampAnchor + ? { + stampAnchor: { + x: r.stampAnchor.x + delta.x, + y: r.stampAnchor.y + delta.y, + }, + } + : {}), + })), + `moveRoomBy:${roomId}`, ), moveRoomGrip: (roomId, index, pt) => - setProject((p) => - mapRoom(p, roomId, (r) => ({ - ...r, - boundary: r.boundary.map((v, i) => (i === index ? pt : v)), - })), + setProject( + (p) => + mapRoom(p, roomId, (r) => ({ + ...r, + boundary: r.boundary.map((v, i) => (i === index ? pt : v)), + })), + `moveRoomGrip:${roomId}:${index}`, ), moveRoomEdge: (roomId, aIndex, bIndex, delta) => - setProject((p) => - mapRoom(p, roomId, (r) => ({ - ...r, - boundary: r.boundary.map((v, i) => - i === aIndex || i === bIndex - ? { x: v.x + delta.x, y: v.y + delta.y } - : v, - ), - })), + setProject( + (p) => + mapRoom(p, roomId, (r) => ({ + ...r, + boundary: r.boundary.map((v, i) => + i === aIndex || i === bIndex + ? { x: v.x + delta.x, y: v.y + delta.y } + : v, + ), + })), + `moveRoomEdge:${roomId}:${aIndex}:${bIndex}`, ), updateRoom: (id, patch) =>