From 8b01d98d9b7c1a09acb9917c475d776c04e77dcf Mon Sep 17 00:00:00 2001 From: "k.hiro1818" Date: Sun, 18 Oct 2026 06:57:20 +0000 Subject: [PATCH 1/4] fix(workbooks): restore kanban order when a drag is canceled Add resolveDragEndAction to decide between none, restore and save at drag end, so pressing Escape no longer persists the moved order. Add a plan for the dnd-kit 0.5.1 manual check. Co-Authored-By: Claude Sonnet 5.5 --- .../2026-10-18/dnd-kit-kanban-check/plan.md | 188 ++++++++++++++++++ .../order/_components/KanbanBoard.svelte | 33 +-- .../workbooks/order/_fixtures/kanban.ts | 2 +- .../(admin)/workbooks/order/_types/kanban.ts | 6 + .../workbooks/order/_utils/kanban.test.ts | 93 +++++++++ .../(admin)/workbooks/order/_utils/kanban.ts | 31 +++ 6 files changed, 336 insertions(+), 17 deletions(-) create mode 100644 docs/dev-notes/2026-10-18/dnd-kit-kanban-check/plan.md diff --git a/docs/dev-notes/2026-10-18/dnd-kit-kanban-check/plan.md b/docs/dev-notes/2026-10-18/dnd-kit-kanban-check/plan.md new file mode 100644 index 000000000..5ac261674 --- /dev/null +++ b/docs/dev-notes/2026-10-18/dnd-kit-kanban-check/plan.md @@ -0,0 +1,188 @@ +# dnd-kit 0.5.1 の Kanban 実操作確認と Escape キャンセル修正 + +## 概要 + +[#4165](https://github.com/AtCoder-NoviSteps/AtCoderNoviSteps/pull/4165)（dnd-kit 0.5.0 → 0.5.1）は、[継続ログ](../../2026-06-28/upstream-pr-review/plan.md)で「実操作確認を条件に反映可」と判定済み。 +本計画は次の 2 本からなる。 + +1. **0.5.1 の手動確認**：ローカル環境で管理画面 Kanban（`src/routes/(admin)/workbooks/order/`）を実際に操作し、結果を記録する。コード変更なし。 +2. **Escape キャンセル修正**：調査中に見つけた既存の不具合。ドラッグを Escape でキャンセルしても、移動後の並びが保存される（C1）。列の外で離すと、表示だけ移動後の並びになり DB は元のまま残る（C2）。どちらも 0.5.1 とは無関係なので、#4165 には混ぜず別ブランチで扱う。 + +## 自動テストで代替できない理由 + +| 種別 | 現状 | +| ------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| CI | e2e のステップはコメントアウトされている（`.github/workflows/ci.yml` の `TODO: Run integration test`）。#4165 の CI 成功は build・lint・check・unit test だけを意味する。 | +| e2e | `e2e/workbook_order.spec.ts` は並べ替えを API への直接 POST で検証しており、ドラッグを一度も通らない。マウス操作は過去に不安定で断念した（[2026-02-28 のメモ](../../2026-02-28/workbook-order/plan.md)「教訓 > DnD」）。 | +| unit | `_utils/kanban.ts` の純関数だけが対象。テスト環境は node が標準で、jsdom には画面配置の計算がない（`getBoundingClientRect` が 0 を返す）。そのため dnd-kit の重なり判定や並び位置の計算を動かせない。 | +| 見た目 | 半透明・ハイライト・自動スクロール・素早い操作の追従は、テストで判定すること自体が難しい。 | + +## 前提・環境 + +- 保存を伴う確認はローカルで行う。共有 DB には書き込まない。 +- #4165 の取得：`git fetch origin pull/4165/head:pr-4165` → `git switch pr-4165` → `pnpm install` → `pnpm dev`。 +- データ：`pnpm db:seed` で placement が作られる（`prisma/seed.ts` の `addWorkBookPlacements`）。未配置の問題集があれば、画面の初期化ボタン（`initializePlacements` action）で作る。カードが 2 枚以上ある列と、スクロールが出るほどカードの多い列が必要。 +- 管理者でログインする。ブラウザーは普段使うもの 1 つでよい。 +- ローカル DB なので、確認後に元の順序へ戻す必要はない。 + +--- + +## Phase 1：0.5.0 での基準確認（staging、コード変更なし） + +既存の不具合と 0.5.1 による退行を切り分けるため、staging（0.5.0）で次の 2 項目だけ先に確認する。 + +- **C1**：カードを別の列へ動かした状態で Escape を押す（ポインター操作中、キーボード操作中の両方）。表示が元に戻るか、リロード後の並びが表示と一致するかを記録する。 +- **C2**：カードを別の列へ動かした後、どの列にも重ならない場所で離す。表示とリロード後の並びが一致するかを記録する。 + +コード上の予想：C1 は「表示もリロード後も移動後の並びになる（保存される）」。根拠は後述の「設計判断」を参照。C2 は「表示だけ移動後の並びになり、DB は元のまま（表示と DB がずれる）」。どちらも未検証。 + +## Phase 2：0.5.1 の手動確認（#4165、コード変更なし） + +`pr-4165` ブランチで次を確認する。優先度の「高」は 0.5.1 の変更がこのコードの依存箇所に直接当たるもの。 + +| ID | 操作 | 期待 | 対応する 0.5.1 の変更 | 優先度 | +| --- | --------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------- | ------------------------------------------------- | ------ | +| M1 | 列をまたいで移動（未分類 → グラフ）し、リロード | ドラッグ中に移動先の列が濃いグレー、掴んだカードが半透明になる。固まらない。リロード後も同じ | #2135 Svelte 版の状態購読 | 高 | +| M2 | 同じ列の中で並べ替え、リロード | 挿入位置が正しく、リロード後も同じ | #2135 | 高 | +| M3 | カードの余白（ラベル付近）を掴み、別カードのタイトル上で離す | 離した直後に問題集の詳細ページへ移動しない | #2152 ドロップ直後のクリックの扱い | 高 | +| M4 | タイトルリンクを普通にクリック | 詳細ページへ移動する | #2152 | 高 | +| M5 | 動かしたカードを続けて再ドラッグ（往復、3 回以上） | カードがポインターに追従し、位置がずれない。挿入位置が正しい | #2149 initialIndex / initialGroup、#2142 スタイル | 中 | +| M6 | キーボード：Tab でフォーカス → Space / Enter で掴む → 矢印で移動 → Space / Enter で確定 | 各キーが反応し、確定後にリロードしても同じ | #2094 キー判定を `event.key` へ変更 | 中 | +| M7 | タブ切替（解法別 ⇄ カリキュラム）の後に M1・M5 | M1・M5 と同じ | #2142 | 中 | +| M8 | カードの多い列で上下端へ、級列の多いカリキュラムタブで左右端へドラッグ | 自動スクロールが効き、離した位置に入る | #2126 スクロール位置測定の再利用 | 低 | +| M9 | 掴んですぐ素早く動かす | カードがポインター位置に追いつく | #2151 開始処理中のポインター移動 | 低 | +| C1 | Phase 1 と同じ | Phase 1 の結果と同じ（変化があれば記録） | — | — | +| C2 | Phase 1 と同じ | Phase 1 の結果と同じ | — | — | + +影響しないと判断した変更：#2131（`transition: null`。transition を指定していない）、#2153（ボタン以外の role の `aria-pressed`）、#2076（modifiers の import。未使用）、#2134（`isSortable` の型。未使用）。 + +NG が出たら、同じ操作を staging（0.5.0）でも行い、退行かどうかを切り分ける。退行なら #4165 を保留し、上流への報告を検討する。回避用のコードは足さない。 + +**記録**：結果は本ファイル末尾の「確認結果」に書く。[継続ログ](../../2026-06-28/upstream-pr-review/plan.md)の「2026-10-16 レビュー」にある #4165 の判定欄には、結論と本ファイルへのリンクだけを追記する。#4165 のマージ判断と push は人が行う。 + +## Phase 3：キャンセル判定の util 化（テスト先行） + +- **層**：utils（`_utils/kanban.ts`）と types（`_types/kanban.ts`）。 +- **責務**：ドラッグ終了時に「何もしない・元に戻す・保存する」のどれを行うかを決める。 +- **既存の検索結果**：`src/lib/utils/`・`src/lib/services/`・`src/features/*/utils/`・`src/features/*/services/` に、ドラッグ終了の判定や `canceled` を扱うコードはない。差分の計算は既存の `reCalcPriorities` を中で使う。 +- **ブランチ**：#4165 反映後の staging から切る。Issue は人が作り、ブランチ名は `#` とする。 + +```typescript +// _types/kanban.ts +export type DragEndAction = + | { kind: 'none' } + | { kind: 'restore'; columns: KanbanColumns } + | { kind: 'save'; updates: PlacementUpdates }; + +// _utils/kanban.ts +export function resolveDragEndAction(params: { + canceled: boolean; + before: KanbanColumns | null; + after: KanbanColumns; + columnKey: ColumnKey; +}): DragEndAction; +``` + +判定の順序： + +1. `canceled` が true：`before` があれば `restore`、なければ `none`。 +2. `reCalcPriorities(before ?? {}, after, columnKey)` が空：`none`。 +3. それ以外：`save`。列の外で離した場合（C2）もここに入り、表示どおりに保存される。 + +テスト（`kanban.test.ts` の `describe('resolveDragEndAction')`、期待値はリテラルで書く）： + +- `returns restore with the snapshot when a reordered drag is canceled` +- `returns none when the drag is canceled without a snapshot` +- `returns none when the order is unchanged` +- `returns save with recalculated updates when cards were reordered` +- `treats a missing snapshot as an empty board when not canceled` + +完了条件：`pnpm test:unit` が通る。 + +## Phase 4：KanbanBoard への組み込みと手動確認 + +- **層**：components（`_components/KanbanBoard.svelte`）。 +- **責務**：`resolveDragEndAction` の結果を状態更新と保存に結び付ける。 + +```typescript +async function onDragEnd(event: DragEndEventArg) { + const capturedTab = activeTab; + const capturedSnapshot = snapshot; + // Clear before awaiting so a drag started during the save keeps its own snapshot + snapshot = null; + + const action = resolveDragEndAction({ + canceled: event.canceled, + before: capturedSnapshot, + after: allItems[capturedTab], + columnKey: TAB_CONFIGS[capturedTab].columnKey, + }); + + if (action.kind === 'restore') { + allItems[capturedTab] = action.columns; + return; + } + + if (action.kind === 'none') { + return; + } + + try { + await saveUpdates(action.updates); + } catch { + if (capturedSnapshot) { + allItems[capturedTab] = capturedSnapshot; + } + + errorMessage = '保存に失敗しました'; + } +} +``` + +手動確認（ローカル）： + +- C1 を再確認：Escape 後に表示が元に戻り、リロード後も元の並びのまま。ポインター操作中、キーボード操作中の両方で確認する。 +- C2 を再確認：列の外で離した後、リロードしても離した時点の表示と同じ並びになる。 +- キャンセル直後に同じカードを再ドラッグでき、固まらない（過去に `{#each}` の DOM と dnd-kit の並べ替え処理が食い違って固まった事例があるため）。 +- M1・M2・M5 を再確認：通常のドラッグが従来どおり保存される。 +- 保存失敗時の復元：DevTools でオフラインにしてドラッグし、エラー表示が出て元の並びに戻る。 + +完了条件：`pnpm format`・`pnpm lint`・`pnpm check`・`pnpm test:unit`・`git diff --check` が通る。cross-review の条件（30 ファイル以上・認証・DB・共有の公開インターフェース）には当たらないため、AI レビューは任意。push は人が行う。 + +--- + +## 設計判断 + +- **キャンセル時に自前で元に戻す理由**：dnd-kit 側にもキャンセル時に DOM の並びを戻す処理がある。ただし、アプリの状態が変わっていると、その処理は何もせずに終わる（`@dnd-kit/dom` 0.5.0 `sortable.js` の dragend 監視にある `hasChanged` の分岐）。このコードは `onDragOver` の `move()` で状態を書き換えるので、ここに当たる。さらに `onDragEnd` は `event.canceled` を見ずに差分を保存する。dnd-kit 公式ガイドの複数リスト例も、キャンセル時に直前の状態へ戻していたと記憶しているが、今回は原文を取得できず未確認。 +- **判定を util に出す理由**：DnD の経路には自動テストがなく、コンポーネントのテストでも jsdom ではドラッグを動かせない。分岐を純関数にすれば unit テストで固定できる（`testing.md`「Component testing」）。 +- **引数を dnd-kit のイベント型ではなく boolean にする理由**：テストで `Draggable` などのクラスを偽装せずに済み、util が dnd-kit の型に依存しない。 +- **`snapshot` を await より前に null へ戻す理由**：現状は保存完了後の `finally` で null にしている。保存待ちの間に次のドラッグを始めると、前のドラッグの `finally` が新しい snapshot を消す。すると次の保存では `reCalcPriorities({}, …)` になり、タブ内の全カードを更新する。保存に失敗しても戻せない。同じ関数の同じ変数の扱いで、直すコストがほぼないため今回含める。不要と判断されれば外す。 +- **列の外で離した場合（C2）は表示どおりに保存する**：Phase 1・2 で表示と DB のずれを確認したうえで決めた（2026-10-18）。離した時点のカードは最後に重ねた位置に見えているので、それを保存すれば表示と DB が一致する。取り消したいときは Escape を使う。これにより、以前の `source`・`target` が無いときに何もしない早期 return は不要になる。 + +## 却下した案 + +- **#4165 に修正を同梱する**：依存更新と挙動修正が混ざり、切り戻しの単位が崩れる。 +- **コンポーネントに `if (event.canceled)` を足すだけにする**：分岐をテストで固定できない。 +- **dnd-kit のキャンセル時の DOM 復元に任せる**：上記のとおり、アプリの状態が変わっていると復元されない（0.5.1 の `sortable.js` でも同じ）。 +- **列の外で離したら元に戻す**：誤って列の外で離したときに保存されない利点はある。ただ、最後に重ねた位置に見えているカードが離した後に元の位置へ戻るので、見た目と結果が食い違う。取り消しは Escape で足りる。 +- **`onDragOver` で `canceled` を見る**：`move()` がすでにキャンセル時は items をそのまま返す（`@dnd-kit/helpers` の `mutate`）。 +- **マウス操作の e2e を足す**：過去に不安定で断念しており、CI でも e2e は動いていない。 +- **キーボード操作の e2e をお試しで足す**：今回は見送り。 + +## 確認結果 + +| ID | 0.5.0（Phase 1） | 0.5.1（Phase 2） | 修正後（Phase 4） | メモ | +| --- | ---------------- | ---------------- | ----------------- | ---------------------------------------------------- | +| M1 | — | | | | +| M2 | — | | | | +| M3 | — | | — | | +| M4 | — | | — | | +| M5 | — | | | | +| M6 | — | | — | | +| M7 | — | | — | | +| M8 | — | | — | | +| M9 | — | | — | | +| C1 | | | | 予想どおり保存されることを確認（2026-10-18） | +| C2 | | | | 予想どおり表示と DB がずれることを確認（2026-10-18） | + +確認日・ブラウザー： diff --git a/src/routes/(admin)/workbooks/order/_components/KanbanBoard.svelte b/src/routes/(admin)/workbooks/order/_components/KanbanBoard.svelte index 34e7ffdea..c39bb9616 100644 --- a/src/routes/(admin)/workbooks/order/_components/KanbanBoard.svelte +++ b/src/routes/(admin)/workbooks/order/_components/KanbanBoard.svelte @@ -31,7 +31,7 @@ parseInitialCategories, parseInitialGrades, parseTab, - reCalcPriorities, + resolveDragEndAction, saveUpdates, TAB_CONFIGS, } from '../_utils/kanban'; @@ -92,34 +92,35 @@ } async function onDragEnd(event: DragEndEventArg) { - if (!event.operation?.source || !event.operation?.target) { - return; - } - const capturedTab = activeTab; const capturedSnapshot = snapshot; + // Clear before awaiting so a drag started during the save keeps its own snapshot + snapshot = null; + + const action = resolveDragEndAction({ + canceled: event.canceled, + before: capturedSnapshot, + after: allItems[capturedTab], + columnKey: TAB_CONFIGS[capturedTab].columnKey, + }); + + if (action.kind === 'restore') { + allItems[capturedTab] = action.columns; + return; + } - const updates = reCalcPriorities( - capturedSnapshot ?? {}, - allItems[capturedTab], - TAB_CONFIGS[capturedTab].columnKey, - ); - - if (updates.length === 0) { - snapshot = null; + if (action.kind === 'none') { return; } try { - await saveUpdates(updates); + await saveUpdates(action.updates); } catch { if (capturedSnapshot) { allItems[capturedTab] = capturedSnapshot; } errorMessage = '保存に失敗しました'; - } finally { - snapshot = null; } } diff --git a/src/routes/(admin)/workbooks/order/_fixtures/kanban.ts b/src/routes/(admin)/workbooks/order/_fixtures/kanban.ts index 5940b9779..f289b7962 100644 --- a/src/routes/(admin)/workbooks/order/_fixtures/kanban.ts +++ b/src/routes/(admin)/workbooks/order/_fixtures/kanban.ts @@ -70,7 +70,7 @@ export const workbooks: WorkbooksWithPlacement = [ }, ]; -// KanbanColumns snapshot used as the "before drag" baseline in reCalcPriorities tests. +// KanbanColumns snapshot used as the "before drag" baseline in reCalcPriorities and resolveDragEndAction tests. export const solutionColumnsBefore: KanbanColumns = { [SolutionCategory.GRAPH]: [ { id: 101, workBookId: 1, title: 'Graph Basics', isPublished: true }, diff --git a/src/routes/(admin)/workbooks/order/_types/kanban.ts b/src/routes/(admin)/workbooks/order/_types/kanban.ts index af4231461..3b89ba402 100644 --- a/src/routes/(admin)/workbooks/order/_types/kanban.ts +++ b/src/routes/(admin)/workbooks/order/_types/kanban.ts @@ -30,6 +30,12 @@ export type PlacementUpdate = { export type PlacementUpdates = PlacementUpdate[]; +// What the board does when a drag ends +export type DragEndAction = + | { kind: 'none' } + | { kind: 'restore'; columns: KanbanColumns } + | { kind: 'save'; updates: PlacementUpdates }; + // Props required for dnd-kit sortable positioning export type SortableProps = { columnId: string; // droppable zone ID (the column this card belongs to) diff --git a/src/routes/(admin)/workbooks/order/_utils/kanban.test.ts b/src/routes/(admin)/workbooks/order/_utils/kanban.test.ts index 0bd533308..246fdcd8b 100644 --- a/src/routes/(admin)/workbooks/order/_utils/kanban.test.ts +++ b/src/routes/(admin)/workbooks/order/_utils/kanban.test.ts @@ -11,6 +11,7 @@ import { parseInitialGrades, parseTab, reCalcPriorities, + resolveDragEndAction, } from './kanban'; describe('parseTab', () => { @@ -336,3 +337,95 @@ describe('reCalcPriorities', () => { expect(reCalcPriorities({}, {}, 'solutionCategory')).toEqual([]); }); }); + +describe('resolveDragEndAction', () => { + const before = solutionColumnsBefore; + const swapped = { + ...before, + [SolutionCategory.GRAPH]: [ + before[SolutionCategory.GRAPH][1], + before[SolutionCategory.GRAPH][0], + ], + }; + + describe('canceled drag', () => { + test('returns restore with the snapshot when a reordered drag is canceled', () => { + const action = resolveDragEndAction({ + canceled: true, + before, + after: swapped, + columnKey: 'solutionCategory', + }); + + expect(action).toEqual({ + kind: 'restore', + columns: { + [SolutionCategory.GRAPH]: [ + { id: 101, workBookId: 1, title: 'Graph Basics', isPublished: true }, + { id: 102, workBookId: 2, title: 'Graph Advanced', isPublished: false }, + ], + [SolutionCategory.PENDING]: [ + { id: 103, workBookId: 3, title: 'Pending Book', isPublished: true }, + ], + }, + }); + }); + + test('returns none when the drag is canceled without a snapshot', () => { + const action = resolveDragEndAction({ + canceled: true, + before: null, + after: swapped, + columnKey: 'solutionCategory', + }); + + expect(action).toEqual({ kind: 'none' }); + }); + }); + + describe('completed drag', () => { + test('returns none when the order is unchanged', () => { + const action = resolveDragEndAction({ + canceled: false, + before, + after: structuredClone(before), + columnKey: 'solutionCategory', + }); + + expect(action).toEqual({ kind: 'none' }); + }); + + test('returns save with recalculated updates when cards were reordered', () => { + const action = resolveDragEndAction({ + canceled: false, + before, + after: swapped, + columnKey: 'solutionCategory', + }); + + expect(action).toEqual({ + kind: 'save', + updates: [ + { id: 102, priority: 1, solutionCategory: SolutionCategory.GRAPH, taskGrade: null }, + { id: 101, priority: 2, solutionCategory: SolutionCategory.GRAPH, taskGrade: null }, + ], + }); + }); + + test('treats a missing snapshot as an empty board when not canceled', () => { + const action = resolveDragEndAction({ + canceled: false, + before: null, + after: { + [TaskGrade.Q10]: [{ id: 201, workBookId: 4, title: 'Curriculum Q10', isPublished: true }], + }, + columnKey: 'taskGrade', + }); + + expect(action).toEqual({ + kind: 'save', + updates: [{ id: 201, priority: 1, solutionCategory: null, taskGrade: TaskGrade.Q10 }], + }); + }); + }); +}); diff --git a/src/routes/(admin)/workbooks/order/_utils/kanban.ts b/src/routes/(admin)/workbooks/order/_utils/kanban.ts index f2be07383..0567362b2 100644 --- a/src/routes/(admin)/workbooks/order/_utils/kanban.ts +++ b/src/routes/(admin)/workbooks/order/_utils/kanban.ts @@ -9,6 +9,7 @@ import type { ActiveTab, KanbanColumns, ColumnKey, + DragEndAction, PlacementUpdates, TabConfig, } from '../_types/kanban'; @@ -174,6 +175,36 @@ export function reCalcPriorities( }); } +/** + * Decides what the board does when a drag ends. + * + * A canceled drag restores the snapshot, because dnd-kit skips its own DOM revert once + * onDragOver has changed the app state. A drop outside every column saves the order as displayed. + * + * @param params.before - Snapshot taken on drag start, or `null` if none was taken + * @param params.after - Current state after the drag operation + */ +export function resolveDragEndAction(params: { + canceled: boolean; + before: KanbanColumns | null; + after: KanbanColumns; + columnKey: ColumnKey; +}): DragEndAction { + const { canceled, before, after, columnKey } = params; + + if (canceled) { + return before ? { kind: 'restore', columns: before } : { kind: 'none' }; + } + + const updates = reCalcPriorities(before ?? {}, after, columnKey); + + if (updates.length === 0) { + return { kind: 'none' }; + } + + return { kind: 'save', updates }; +} + /** * Sends placement updates to the server. * Throws if the response is not OK. From 5d590f2bd41d3ef58d066d6904d823339e1077e4 Mon Sep 17 00:00:00 2001 From: "k.hiro1818" Date: Sun, 18 Oct 2026 06:58:24 +0000 Subject: [PATCH 2/4] refactor(test): share swapped kanban columns fixture Co-Authored-By: Claude Sonnet 5.5 --- .../workbooks/order/_fixtures/kanban.ts | 11 +++++- .../workbooks/order/_utils/kanban.test.ts | 36 ++++--------------- 2 files changed, 17 insertions(+), 30 deletions(-) diff --git a/src/routes/(admin)/workbooks/order/_fixtures/kanban.ts b/src/routes/(admin)/workbooks/order/_fixtures/kanban.ts index f289b7962..e2785b4dc 100644 --- a/src/routes/(admin)/workbooks/order/_fixtures/kanban.ts +++ b/src/routes/(admin)/workbooks/order/_fixtures/kanban.ts @@ -70,7 +70,7 @@ export const workbooks: WorkbooksWithPlacement = [ }, ]; -// KanbanColumns snapshot used as the "before drag" baseline in reCalcPriorities and resolveDragEndAction tests. +// "Before drag" baseline: GRAPH has two cards and PENDING has one. export const solutionColumnsBefore: KanbanColumns = { [SolutionCategory.GRAPH]: [ { id: 101, workBookId: 1, title: 'Graph Basics', isPublished: true }, @@ -80,3 +80,12 @@ export const solutionColumnsBefore: KanbanColumns = { { id: 103, workBookId: 3, title: 'Pending Book', isPublished: true }, ], }; + +// solutionColumnsBefore with the two GRAPH cards swapped; PENDING is unchanged. +export const solutionColumnsGraphSwapped: KanbanColumns = { + ...solutionColumnsBefore, + [SolutionCategory.GRAPH]: [ + solutionColumnsBefore[SolutionCategory.GRAPH][1], + solutionColumnsBefore[SolutionCategory.GRAPH][0], + ], +}; diff --git a/src/routes/(admin)/workbooks/order/_utils/kanban.test.ts b/src/routes/(admin)/workbooks/order/_utils/kanban.test.ts index 246fdcd8b..f91d03e23 100644 --- a/src/routes/(admin)/workbooks/order/_utils/kanban.test.ts +++ b/src/routes/(admin)/workbooks/order/_utils/kanban.test.ts @@ -3,7 +3,7 @@ import { describe, test, expect } from 'vitest'; import { SolutionCategory } from '$features/workbooks/types/workbook_placement'; import { TaskGrade } from '$lib/types/task'; -import { workbooks, solutionColumnsBefore } from '../_fixtures/kanban'; +import { workbooks, solutionColumnsBefore, solutionColumnsGraphSwapped } from '../_fixtures/kanban'; import { buildKanbanItems, buildUpdatedUrl, @@ -235,15 +235,7 @@ describe('reCalcPriorities', () => { }); test('returns updates for reordered cards within a column', () => { - const after = { - ...before, - [SolutionCategory.GRAPH]: [ - before[SolutionCategory.GRAPH][1], - before[SolutionCategory.GRAPH][0], - ], // swapped - }; - - const updates = reCalcPriorities(before, after, 'solutionCategory'); + const updates = reCalcPriorities(before, solutionColumnsGraphSwapped, 'solutionCategory'); expect(updates).toHaveLength(2); expect(updates[0]).toMatchObject({ id: 102, @@ -260,15 +252,8 @@ describe('reCalcPriorities', () => { }); test('returns updates only for changed columns', () => { - const after = { - [SolutionCategory.GRAPH]: [ - before[SolutionCategory.GRAPH][1], - before[SolutionCategory.GRAPH][0], - ], // changed - [SolutionCategory.PENDING]: before[SolutionCategory.PENDING], // unchanged - }; - - const updates = reCalcPriorities(before, after, 'solutionCategory'); + // GRAPH is swapped, PENDING is unchanged + const updates = reCalcPriorities(before, solutionColumnsGraphSwapped, 'solutionCategory'); const updatedIds = updates.map((update) => update.id); expect(updatedIds).not.toContain(103); expect(updatedIds).toContain(101); @@ -340,20 +325,13 @@ describe('reCalcPriorities', () => { describe('resolveDragEndAction', () => { const before = solutionColumnsBefore; - const swapped = { - ...before, - [SolutionCategory.GRAPH]: [ - before[SolutionCategory.GRAPH][1], - before[SolutionCategory.GRAPH][0], - ], - }; describe('canceled drag', () => { test('returns restore with the snapshot when a reordered drag is canceled', () => { const action = resolveDragEndAction({ canceled: true, before, - after: swapped, + after: solutionColumnsGraphSwapped, columnKey: 'solutionCategory', }); @@ -375,7 +353,7 @@ describe('resolveDragEndAction', () => { const action = resolveDragEndAction({ canceled: true, before: null, - after: swapped, + after: solutionColumnsGraphSwapped, columnKey: 'solutionCategory', }); @@ -399,7 +377,7 @@ describe('resolveDragEndAction', () => { const action = resolveDragEndAction({ canceled: false, before, - after: swapped, + after: solutionColumnsGraphSwapped, columnKey: 'solutionCategory', }); From bbafcee2bd1124bc7aaf5156387e8b405c9171bd Mon Sep 17 00:00:00 2001 From: "k.hiro1818" Date: Sun, 18 Oct 2026 08:06:23 +0000 Subject: [PATCH 3/4] docs: rewrite dnd-kit kanban plan as completed record Co-Authored-By: Claude Sonnet 5.5 --- .../2026-10-18/dnd-kit-kanban-check/plan.md | 233 ++++++------------ 1 file changed, 76 insertions(+), 157 deletions(-) diff --git a/docs/dev-notes/2026-10-18/dnd-kit-kanban-check/plan.md b/docs/dev-notes/2026-10-18/dnd-kit-kanban-check/plan.md index 5ac261674..a105898f7 100644 --- a/docs/dev-notes/2026-10-18/dnd-kit-kanban-check/plan.md +++ b/docs/dev-notes/2026-10-18/dnd-kit-kanban-check/plan.md @@ -1,188 +1,107 @@ -# dnd-kit 0.5.1 の Kanban 実操作確認と Escape キャンセル修正 +# dnd-kit 0.5.1 の Kanban 実操作確認とドラッグ終了処理の修正 ## 概要 -[#4165](https://github.com/AtCoder-NoviSteps/AtCoderNoviSteps/pull/4165)（dnd-kit 0.5.0 → 0.5.1）は、[継続ログ](../../2026-06-28/upstream-pr-review/plan.md)で「実操作確認を条件に反映可」と判定済み。 -本計画は次の 2 本からなる。 +管理画面 Kanban（`src/routes/(admin)/workbooks/order/`）について、次の 2 つを行った。 -1. **0.5.1 の手動確認**：ローカル環境で管理画面 Kanban（`src/routes/(admin)/workbooks/order/`）を実際に操作し、結果を記録する。コード変更なし。 -2. **Escape キャンセル修正**：調査中に見つけた既存の不具合。ドラッグを Escape でキャンセルしても、移動後の並びが保存される（C1）。列の外で離すと、表示だけ移動後の並びになり DB は元のまま残る（C2）。どちらも 0.5.1 とは無関係なので、#4165 には混ぜず別ブランチで扱う。 +1. **dnd-kit 0.5.0 → 0.5.1 の手動確認**：[#4165](https://github.com/AtCoder-NoviSteps/AtCoderNoviSteps/pull/4165) は、[継続ログ](../../2026-06-28/upstream-pr-review/plan.md)で「実操作確認を条件に反映可」と判定されていた。2026-10-18 に staging へマージ済み。 +2. **ドラッグ終了処理の不具合修正**：確認中に見つかった既存の不具合 C1・C2 と、snapshot の競合を直す。0.5.1 とは無関係なので、#4165 には混ぜず [#4183](https://github.com/AtCoder-NoviSteps/AtCoderNoviSteps/issues/4183)（ブランチ `#4183`）で扱う。 -## 自動テストで代替できない理由 +## 状況（2026-10-18 時点） -| 種別 | 現状 | -| ------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| CI | e2e のステップはコメントアウトされている（`.github/workflows/ci.yml` の `TODO: Run integration test`）。#4165 の CI 成功は build・lint・check・unit test だけを意味する。 | -| e2e | `e2e/workbook_order.spec.ts` は並べ替えを API への直接 POST で検証しており、ドラッグを一度も通らない。マウス操作は過去に不安定で断念した（[2026-02-28 のメモ](../../2026-02-28/workbook-order/plan.md)「教訓 > DnD」）。 | -| unit | `_utils/kanban.ts` の純関数だけが対象。テスト環境は node が標準で、jsdom には画面配置の計算がない（`getBoundingClientRect` が 0 を返す）。そのため dnd-kit の重なり判定や並び位置の計算を動かせない。 | -| 見た目 | 半透明・ハイライト・自動スクロール・素早い操作の追従は、テストで判定すること自体が難しい。 | +| Phase | 内容 | 状況 | +| ----- | ------------------------------------------------------------ | ---------------------------------------------------------------------------------- | +| 1・2 | 0.5.0 と 0.5.1 での手動確認（コード変更なし） | M1〜M9 をローカルで確認し、#4165 はマージ済み。C1・C2 が予想どおり起きることを確認 | +| 3 | 判定を純関数 `resolveDragEndAction` に切り出し、テストを先行 | 完了 | +| 4 | `KanbanBoard.svelte` への組み込み | 完了（format・lint・check・unit test とローカルでの手動確認） | -## 前提・環境 +## 不具合と原因 -- 保存を伴う確認はローカルで行う。共有 DB には書き込まない。 -- #4165 の取得：`git fetch origin pull/4165/head:pr-4165` → `git switch pr-4165` → `pnpm install` → `pnpm dev`。 -- データ：`pnpm db:seed` で placement が作られる（`prisma/seed.ts` の `addWorkBookPlacements`）。未配置の問題集があれば、画面の初期化ボタン（`initializePlacements` action）で作る。カードが 2 枚以上ある列と、スクロールが出るほどカードの多い列が必要。 -- 管理者でログインする。ブラウザーは普段使うもの 1 つでよい。 -- ローカル DB なので、確認後に元の順序へ戻す必要はない。 - ---- - -## Phase 1：0.5.0 での基準確認（staging、コード変更なし） - -既存の不具合と 0.5.1 による退行を切り分けるため、staging（0.5.0）で次の 2 項目だけ先に確認する。 - -- **C1**：カードを別の列へ動かした状態で Escape を押す（ポインター操作中、キーボード操作中の両方）。表示が元に戻るか、リロード後の並びが表示と一致するかを記録する。 -- **C2**：カードを別の列へ動かした後、どの列にも重ならない場所で離す。表示とリロード後の並びが一致するかを記録する。 - -コード上の予想：C1 は「表示もリロード後も移動後の並びになる（保存される）」。根拠は後述の「設計判断」を参照。C2 は「表示だけ移動後の並びになり、DB は元のまま（表示と DB がずれる）」。どちらも未検証。 - -## Phase 2：0.5.1 の手動確認（#4165、コード変更なし） - -`pr-4165` ブランチで次を確認する。優先度の「高」は 0.5.1 の変更がこのコードの依存箇所に直接当たるもの。 - -| ID | 操作 | 期待 | 対応する 0.5.1 の変更 | 優先度 | -| --- | --------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------- | ------------------------------------------------- | ------ | -| M1 | 列をまたいで移動（未分類 → グラフ）し、リロード | ドラッグ中に移動先の列が濃いグレー、掴んだカードが半透明になる。固まらない。リロード後も同じ | #2135 Svelte 版の状態購読 | 高 | -| M2 | 同じ列の中で並べ替え、リロード | 挿入位置が正しく、リロード後も同じ | #2135 | 高 | -| M3 | カードの余白（ラベル付近）を掴み、別カードのタイトル上で離す | 離した直後に問題集の詳細ページへ移動しない | #2152 ドロップ直後のクリックの扱い | 高 | -| M4 | タイトルリンクを普通にクリック | 詳細ページへ移動する | #2152 | 高 | -| M5 | 動かしたカードを続けて再ドラッグ（往復、3 回以上） | カードがポインターに追従し、位置がずれない。挿入位置が正しい | #2149 initialIndex / initialGroup、#2142 スタイル | 中 | -| M6 | キーボード：Tab でフォーカス → Space / Enter で掴む → 矢印で移動 → Space / Enter で確定 | 各キーが反応し、確定後にリロードしても同じ | #2094 キー判定を `event.key` へ変更 | 中 | -| M7 | タブ切替（解法別 ⇄ カリキュラム）の後に M1・M5 | M1・M5 と同じ | #2142 | 中 | -| M8 | カードの多い列で上下端へ、級列の多いカリキュラムタブで左右端へドラッグ | 自動スクロールが効き、離した位置に入る | #2126 スクロール位置測定の再利用 | 低 | -| M9 | 掴んですぐ素早く動かす | カードがポインター位置に追いつく | #2151 開始処理中のポインター移動 | 低 | -| C1 | Phase 1 と同じ | Phase 1 の結果と同じ（変化があれば記録） | — | — | -| C2 | Phase 1 と同じ | Phase 1 の結果と同じ | — | — | - -影響しないと判断した変更：#2131（`transition: null`。transition を指定していない）、#2153（ボタン以外の role の `aria-pressed`）、#2076（modifiers の import。未使用）、#2134（`isSortable` の型。未使用）。 - -NG が出たら、同じ操作を staging（0.5.0）でも行い、退行かどうかを切り分ける。退行なら #4165 を保留し、上流への報告を検討する。回避用のコードは足さない。 +- **C1：Escape でキャンセルしても、移動後の並びが保存される。** `onDragOver` が `move()` でアプリの状態を書き換えるため、dnd-kit はキャンセル時の DOM 復元を行わない（`@dnd-kit/dom` の `sortable.js` の dragend 監視で、`hasChanged` が真なら何もせず return する。0.5.0・0.5.1 とも同じ）。さらに旧 `onDragEnd` は `event.canceled` を見ずに差分を保存していた。 +- **C2：列の外で離すと、表示だけ移動後の並びになり、DB は元のまま残る。** 列の外では target が無い。旧 `onDragEnd` は target が無いと何もせず return したが、表示は `onDragOver` の時点ですでに移動後の並びになっていた。 +- **snapshot の競合**：旧コードは snapshot を保存完了後の `finally` で null にしていた。保存待ちの間に次のドラッグを始めると、前のドラッグの `finally` が新しい snapshot を消す。次の保存は `reCalcPriorities({}, …)` になってタブ内の全カードを更新し、失敗しても元に戻せない。 -**記録**：結果は本ファイル末尾の「確認結果」に書く。[継続ログ](../../2026-06-28/upstream-pr-review/plan.md)の「2026-10-16 レビュー」にある #4165 の判定欄には、結論と本ファイルへのリンクだけを追記する。#4165 のマージ判断と push は人が行う。 +## 意思決定 -## Phase 3：キャンセル判定の util 化（テスト先行） +- **キャンセルされたら、ドラッグ開始時の snapshot に戻す。** dnd-kit の DOM 復元には頼れないため（C1 の原因を参照）。 +- **列の外で離したら、表示どおりに保存する（2026-10-18 決定）。** 離した時点のカードは最後に重ねた位置に見えているので、それを保存すれば表示と DB が一致する。取り消したいときは Escape を使う。この決定により、target が無いときの早期 return は削除した。 +- **snapshot は await より前に null へ戻す。** 同じ関数の同じ変数の扱いで、直すコストがほぼないため同じ PR に含めた。 +- **「何もしない・元に戻す・保存する」の判定を純関数にする。** DnD の経路には自動テストが無く、jsdom ではドラッグを動かせない。分岐を純関数にすれば unit テストで固定できる。 +- **判定関数の引数は、dnd-kit のイベント型ではなく `canceled: boolean` にする。** テストで dnd-kit のクラスを偽装せずに済み、util が dnd-kit の型に依存しない。 -- **層**：utils（`_utils/kanban.ts`）と types（`_types/kanban.ts`）。 -- **責務**：ドラッグ終了時に「何もしない・元に戻す・保存する」のどれを行うかを決める。 -- **既存の検索結果**：`src/lib/utils/`・`src/lib/services/`・`src/features/*/utils/`・`src/features/*/services/` に、ドラッグ終了の判定や `canceled` を扱うコードはない。差分の計算は既存の `reCalcPriorities` を中で使う。 -- **ブランチ**：#4165 反映後の staging から切る。Issue は人が作り、ブランチ名は `#` とする。 - -```typescript -// _types/kanban.ts -export type DragEndAction = - | { kind: 'none' } - | { kind: 'restore'; columns: KanbanColumns } - | { kind: 'save'; updates: PlacementUpdates }; - -// _utils/kanban.ts -export function resolveDragEndAction(params: { - canceled: boolean; - before: KanbanColumns | null; - after: KanbanColumns; - columnKey: ColumnKey; -}): DragEndAction; -``` +## 却下した案 -判定の順序： +- **#4165 に修正を同梱する**：依存更新と挙動修正が混ざり、切り戻しの単位が崩れる。 +- **コンポーネントに `if (event.canceled)` を足すだけにする**：分岐をテストで固定できない。 +- **dnd-kit のキャンセル時の DOM 復元に任せる**：アプリの状態が変わっていると復元されない。 +- **列の外で離したら元に戻す**：誤って離したときに保存されない利点はある。ただ、最後に重ねた位置に見えているカードが、離した後に元の位置へ戻るので、見た目と結果が食い違う。取り消しは Escape で足りる。 +- **`onDragOver` で `canceled` を見る**：`move()` はキャンセル時に items をそのまま返すので意味がない（`@dnd-kit/helpers` の `mutate`）。 +- **`move()` を `onDragEnd` だけで呼び、snapshot と復元をなくす**：理由は 2 つ。1 つ目は、`{#each}` の DOM と `OptimisticSortingPlugin` が食い違って固まること（[2026-02-28 のメモ](../../2026-02-28/workbook-order/plan.md)「@dnd-kit/svelte 実装上の注意点」）。2 つ目は C2 が悪化すること。列の外で離すと target が無く `move()` は何もしないが、dnd-kit はキャンセル時しか DOM を戻さないので、画面だけ移動後の並びになる。 +- **ドラッグ操作の e2e を足す**：マウス操作は過去に不安定で断念しており、CI でも e2e は動いていない。キーボード操作の e2e も今回は見送り。 -1. `canceled` が true：`before` があれば `restore`、なければ `none`。 -2. `reCalcPriorities(before ?? {}, after, columnKey)` が空：`none`。 -3. それ以外：`save`。列の外で離した場合（C2）もここに入り、表示どおりに保存される。 +## 既知の課題（#4183 では扱わない） -テスト（`kanban.test.ts` の `describe('resolveDragEndAction')`、期待値はリテラルで書く）： +- **保存失敗時の復元が、後続のドラッグの結果を上書きする**：保存 A の待機中にドラッグ B を始め、その後 A が失敗すると、A のドラッグ前の並びで B の結果を上書きする。B が保存済みなら、表示と DB がずれる。以前からある競合で、snapshot の修正では解消しない。保存の失敗と保存待ち中のドラッグが重なったときだけ起きるので、記録のみとした。直す場合は、保存が終わるまで次のドラッグを始められないようにするのが最小。 -- `returns restore with the snapshot when a reordered drag is canceled` -- `returns none when the drag is canceled without a snapshot` -- `returns none when the order is unchanged` -- `returns save with recalculated updates when cards were reordered` -- `treats a missing snapshot as an empty board when not canceled` +## レビュー（2026-10-18） -完了条件：`pnpm test:unit` が通る。 +- **Codex**：Critical・Severe は無し。Moderate の 1 件は「既知の課題」に記録した。Minor の「並びが変わっていないキャンセルでも restore を返す」テストの追加は見送った。この場合は元に戻しても戻さなくても画面が同じで、価値が低いため。復元で dnd-kit の並びがずれたり固まったりしないかはコードから判断できないとされたので、Phase 4 の手動確認で見る。 +- **simplify（4 観点）**：Reuse・Simplification の指摘を受け、テストで 3 か所に重複していた並びを fixture `solutionColumnsGraphSwapped` に切り出した。Efficiency・Altitude は指摘なし。Altitude の補足を受けて、「`move()` を `onDragEnd` だけで呼ぶ」案を却下理由とともに追記した。 -## Phase 4：KanbanBoard への組み込みと手動確認 +## 手動確認 -- **層**：components（`_components/KanbanBoard.svelte`）。 -- **責務**：`resolveDragEndAction` の結果を状態更新と保存に結び付ける。 +### 自動テストで代替できない理由 -```typescript -async function onDragEnd(event: DragEndEventArg) { - const capturedTab = activeTab; - const capturedSnapshot = snapshot; - // Clear before awaiting so a drag started during the save keeps its own snapshot - snapshot = null; +- **CI**：e2e のステップはコメントアウトされている（`.github/workflows/ci.yml` の `TODO: Run integration test`）。 +- **e2e**：`e2e/workbook_order.spec.ts` は並べ替えを API への直接 POST で検証しており、ドラッグを通らない。 +- **unit**：jsdom には画面配置の計算が無い（`getBoundingClientRect` が 0 を返す）ので、dnd-kit の重なり判定や並び位置の計算を動かせない。 +- **見た目**：半透明・ハイライト・自動スクロール・素早い操作への追従は、テストで判定すること自体が難しい。 - const action = resolveDragEndAction({ - canceled: event.canceled, - before: capturedSnapshot, - after: allItems[capturedTab], - columnKey: TAB_CONFIGS[capturedTab].columnKey, - }); +### 環境 - if (action.kind === 'restore') { - allItems[capturedTab] = action.columns; - return; - } +- 保存を伴う確認はローカルで行い、共有 DB には書き込まない。ローカル DB なので、確認後に並びを戻す必要はない。 +- データは `pnpm db:seed` で placement が作られる。未配置の問題集があれば、画面の初期化ボタンで作る。カードが 2 枚以上ある列と、スクロールが出るほどカードの多い列が必要。 +- 管理者でログインする。ブラウザーは普段使うもの 1 つでよい。 +- devcontainer で `pnpm i` が registry.npmjs.org からの取得で止まった場合は、`sudo /usr/local/bin/init-firewall.sh` で許可リストを作り直す。ファイアウォールは起動時に 1 回だけ DNS を引いて IP を許可するので、そのとき一部の IP しか返らないと、他の IP への接続が拒否される（推定。ipset の中身は未確認）。 - if (action.kind === 'none') { - return; - } +### dnd-kit 更新時の確認項目 - try { - await saveUpdates(action.updates); - } catch { - if (capturedSnapshot) { - allItems[capturedTab] = capturedSnapshot; - } +| ID | 操作 | 期待 | +| --- | --------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------- | +| M1 | 列をまたいで移動（未分類 → グラフ）し、リロード | ドラッグ中に移動先の列が濃いグレー、掴んだカードが半透明になる。固まらない。リロード後も同じ | +| M2 | 同じ列の中で並べ替え、リロード | 挿入位置が正しく、リロード後も同じ | +| M3 | カードの余白（ラベル付近）を掴み、別カードのタイトル上で離す | 離した直後に問題集の詳細ページへ移動しない | +| M4 | タイトルリンクを普通にクリック | 詳細ページへ移動する | +| M5 | 動かしたカードを続けて再ドラッグ（往復、3 回以上） | カードがポインターに追従し、位置がずれない。挿入位置が正しい | +| M6 | キーボード：Tab でフォーカス → Space / Enter で掴む → 矢印で移動 → Space / Enter で確定 | 各キーが反応し、確定後にリロードしても同じ | +| M7 | タブ切替（解法別 ⇄ カリキュラム）の後に M1・M5 | M1・M5 と同じ | +| M8 | カードの多い列で上下端へ、級列の多いカリキュラムタブで左右端へドラッグ | 自動スクロールが効き、離した位置に入る | +| M9 | 掴んですぐ素早く動かす | カードがポインター位置に追いつく | - errorMessage = '保存に失敗しました'; - } -} -``` - -手動確認（ローカル）： - -- C1 を再確認：Escape 後に表示が元に戻り、リロード後も元の並びのまま。ポインター操作中、キーボード操作中の両方で確認する。 -- C2 を再確認：列の外で離した後、リロードしても離した時点の表示と同じ並びになる。 -- キャンセル直後に同じカードを再ドラッグでき、固まらない（過去に `{#each}` の DOM と dnd-kit の並べ替え処理が食い違って固まった事例があるため）。 -- M1・M2・M5 を再確認：通常のドラッグが従来どおり保存される。 -- 保存失敗時の復元：DevTools でオフラインにしてドラッグし、エラー表示が出て元の並びに戻る。 - -完了条件：`pnpm format`・`pnpm lint`・`pnpm check`・`pnpm test:unit`・`git diff --check` が通る。cross-review の条件（30 ファイル以上・認証・DB・共有の公開インターフェース）には当たらないため、AI レビューは任意。push は人が行う。 +0.5.1 で M1〜M9 を選んだ根拠は、上流の変更との対応である。M1・M2 は #2135（Svelte 版の状態購読）、M3・M4 は #2152（ドロップ直後のクリック）、M5 は #2149・#2142、M6 は #2094（キー判定を `event.key` へ変更）、M7 は #2142、M8 は #2126、M9 は #2151 に対応する。NG が出たら旧バージョンでも同じ操作を行い、退行かどうかを切り分ける。回避用のコードは足さない。 ---- +### #4183 の確認項目（Phase 4） -## 設計判断 +- **C1**：Escape 後に表示が元に戻り、リロード後も元の並びのまま。ポインター操作中とキーボード操作中の両方で確認する。 +- **C2**：列の外で離した後、リロードしても離した時点の表示と同じ並びになる。 +- **キャンセル直後の再ドラッグ**：同じカードをすぐ再ドラッグでき、固まらない。 +- **通常のドラッグ**：M1・M2・M5 が従来どおり保存される。 +- **保存失敗時の復元**：DevTools でオフラインにしてドラッグすると、エラー表示が出て元の並びに戻る。 -- **キャンセル時に自前で元に戻す理由**：dnd-kit 側にもキャンセル時に DOM の並びを戻す処理がある。ただし、アプリの状態が変わっていると、その処理は何もせずに終わる（`@dnd-kit/dom` 0.5.0 `sortable.js` の dragend 監視にある `hasChanged` の分岐）。このコードは `onDragOver` の `move()` で状態を書き換えるので、ここに当たる。さらに `onDragEnd` は `event.canceled` を見ずに差分を保存する。dnd-kit 公式ガイドの複数リスト例も、キャンセル時に直前の状態へ戻していたと記憶しているが、今回は原文を取得できず未確認。 -- **判定を util に出す理由**：DnD の経路には自動テストがなく、コンポーネントのテストでも jsdom ではドラッグを動かせない。分岐を純関数にすれば unit テストで固定できる（`testing.md`「Component testing」）。 -- **引数を dnd-kit のイベント型ではなく boolean にする理由**：テストで `Draggable` などのクラスを偽装せずに済み、util が dnd-kit の型に依存しない。 -- **`snapshot` を await より前に null へ戻す理由**：現状は保存完了後の `finally` で null にしている。保存待ちの間に次のドラッグを始めると、前のドラッグの `finally` が新しい snapshot を消す。すると次の保存では `reCalcPriorities({}, …)` になり、タブ内の全カードを更新する。保存に失敗しても戻せない。同じ関数の同じ変数の扱いで、直すコストがほぼないため今回含める。不要と判断されれば外す。 -- **列の外で離した場合（C2）は表示どおりに保存する**：Phase 1・2 で表示と DB のずれを確認したうえで決めた（2026-10-18）。離した時点のカードは最後に重ねた位置に見えているので、それを保存すれば表示と DB が一致する。取り消したいときは Escape を使う。これにより、以前の `source`・`target` が無いときに何もしない早期 return は不要になる。 +## 確認結果 -## 却下した案 +| ID | 修正前 | 修正後（#4183） | メモ | +| -------------- | -------------------------------- | --------------- | ---------- | +| C1 | 保存される（予想どおり） | OK | 2026-10-18 | +| C2 | 表示と DB がずれる（予想どおり） | OK | 2026-10-18 | +| 再ドラッグ | — | OK | 2026-10-18 | +| M1 | OK | OK | 2026-10-18 | +| M2 | OK | OK | 2026-10-18 | +| M5 | OK | OK | 2026-10-18 | +| 保存失敗の復元 | — | OK | 2026-10-18 | -- **#4165 に修正を同梱する**：依存更新と挙動修正が混ざり、切り戻しの単位が崩れる。 -- **コンポーネントに `if (event.canceled)` を足すだけにする**：分岐をテストで固定できない。 -- **dnd-kit のキャンセル時の DOM 復元に任せる**：上記のとおり、アプリの状態が変わっていると復元されない（0.5.1 の `sortable.js` でも同じ）。 -- **列の外で離したら元に戻す**：誤って列の外で離したときに保存されない利点はある。ただ、最後に重ねた位置に見えているカードが離した後に元の位置へ戻るので、見た目と結果が食い違う。取り消しは Escape で足りる。 -- **`onDragOver` で `canceled` を見る**：`move()` がすでにキャンセル時は items をそのまま返す（`@dnd-kit/helpers` の `mutate`）。 -- **マウス操作の e2e を足す**：過去に不安定で断念しており、CI でも e2e は動いていない。 -- **キーボード操作の e2e をお試しで足す**：今回は見送り。 +修正前の M1〜M9 は、0.5.1（#4165）でローカルで確認した。この表に無い M3・M4・M6〜M9 も同じ確認に含まれる。 -## 確認結果 +修正後の確認は、#4183 のブランチでローカルで行った。 -| ID | 0.5.0（Phase 1） | 0.5.1（Phase 2） | 修正後（Phase 4） | メモ | -| --- | ---------------- | ---------------- | ----------------- | ---------------------------------------------------- | -| M1 | — | | | | -| M2 | — | | | | -| M3 | — | | — | | -| M4 | — | | — | | -| M5 | — | | | | -| M6 | — | | — | | -| M7 | — | | — | | -| M8 | — | | — | | -| M9 | — | | — | | -| C1 | | | | 予想どおり保存されることを確認（2026-10-18） | -| C2 | | | | 予想どおり表示と DB がずれることを確認（2026-10-18） | - -確認日・ブラウザー： +確認日：2026-10-18 From fe8cd37dccb9f45a4da776a662ba3920ad8772cb Mon Sep 17 00:00:00 2001 From: "k.hiro1818" Date: Sun, 18 Oct 2026 08:06:51 +0000 Subject: [PATCH 4/4] docs: remove dnd-kit kanban check plan Co-Authored-By: Claude Sonnet 5.5 --- .../2026-10-18/dnd-kit-kanban-check/plan.md | 107 ------------------ 1 file changed, 107 deletions(-) delete mode 100644 docs/dev-notes/2026-10-18/dnd-kit-kanban-check/plan.md diff --git a/docs/dev-notes/2026-10-18/dnd-kit-kanban-check/plan.md b/docs/dev-notes/2026-10-18/dnd-kit-kanban-check/plan.md deleted file mode 100644 index a105898f7..000000000 --- a/docs/dev-notes/2026-10-18/dnd-kit-kanban-check/plan.md +++ /dev/null @@ -1,107 +0,0 @@ -# dnd-kit 0.5.1 の Kanban 実操作確認とドラッグ終了処理の修正 - -## 概要 - -管理画面 Kanban（`src/routes/(admin)/workbooks/order/`）について、次の 2 つを行った。 - -1. **dnd-kit 0.5.0 → 0.5.1 の手動確認**：[#4165](https://github.com/AtCoder-NoviSteps/AtCoderNoviSteps/pull/4165) は、[継続ログ](../../2026-06-28/upstream-pr-review/plan.md)で「実操作確認を条件に反映可」と判定されていた。2026-10-18 に staging へマージ済み。 -2. **ドラッグ終了処理の不具合修正**：確認中に見つかった既存の不具合 C1・C2 と、snapshot の競合を直す。0.5.1 とは無関係なので、#4165 には混ぜず [#4183](https://github.com/AtCoder-NoviSteps/AtCoderNoviSteps/issues/4183)（ブランチ `#4183`）で扱う。 - -## 状況（2026-10-18 時点） - -| Phase | 内容 | 状況 | -| ----- | ------------------------------------------------------------ | ---------------------------------------------------------------------------------- | -| 1・2 | 0.5.0 と 0.5.1 での手動確認（コード変更なし） | M1〜M9 をローカルで確認し、#4165 はマージ済み。C1・C2 が予想どおり起きることを確認 | -| 3 | 判定を純関数 `resolveDragEndAction` に切り出し、テストを先行 | 完了 | -| 4 | `KanbanBoard.svelte` への組み込み | 完了（format・lint・check・unit test とローカルでの手動確認） | - -## 不具合と原因 - -- **C1：Escape でキャンセルしても、移動後の並びが保存される。** `onDragOver` が `move()` でアプリの状態を書き換えるため、dnd-kit はキャンセル時の DOM 復元を行わない（`@dnd-kit/dom` の `sortable.js` の dragend 監視で、`hasChanged` が真なら何もせず return する。0.5.0・0.5.1 とも同じ）。さらに旧 `onDragEnd` は `event.canceled` を見ずに差分を保存していた。 -- **C2：列の外で離すと、表示だけ移動後の並びになり、DB は元のまま残る。** 列の外では target が無い。旧 `onDragEnd` は target が無いと何もせず return したが、表示は `onDragOver` の時点ですでに移動後の並びになっていた。 -- **snapshot の競合**：旧コードは snapshot を保存完了後の `finally` で null にしていた。保存待ちの間に次のドラッグを始めると、前のドラッグの `finally` が新しい snapshot を消す。次の保存は `reCalcPriorities({}, …)` になってタブ内の全カードを更新し、失敗しても元に戻せない。 - -## 意思決定 - -- **キャンセルされたら、ドラッグ開始時の snapshot に戻す。** dnd-kit の DOM 復元には頼れないため（C1 の原因を参照）。 -- **列の外で離したら、表示どおりに保存する（2026-10-18 決定）。** 離した時点のカードは最後に重ねた位置に見えているので、それを保存すれば表示と DB が一致する。取り消したいときは Escape を使う。この決定により、target が無いときの早期 return は削除した。 -- **snapshot は await より前に null へ戻す。** 同じ関数の同じ変数の扱いで、直すコストがほぼないため同じ PR に含めた。 -- **「何もしない・元に戻す・保存する」の判定を純関数にする。** DnD の経路には自動テストが無く、jsdom ではドラッグを動かせない。分岐を純関数にすれば unit テストで固定できる。 -- **判定関数の引数は、dnd-kit のイベント型ではなく `canceled: boolean` にする。** テストで dnd-kit のクラスを偽装せずに済み、util が dnd-kit の型に依存しない。 - -## 却下した案 - -- **#4165 に修正を同梱する**：依存更新と挙動修正が混ざり、切り戻しの単位が崩れる。 -- **コンポーネントに `if (event.canceled)` を足すだけにする**：分岐をテストで固定できない。 -- **dnd-kit のキャンセル時の DOM 復元に任せる**：アプリの状態が変わっていると復元されない。 -- **列の外で離したら元に戻す**：誤って離したときに保存されない利点はある。ただ、最後に重ねた位置に見えているカードが、離した後に元の位置へ戻るので、見た目と結果が食い違う。取り消しは Escape で足りる。 -- **`onDragOver` で `canceled` を見る**：`move()` はキャンセル時に items をそのまま返すので意味がない（`@dnd-kit/helpers` の `mutate`）。 -- **`move()` を `onDragEnd` だけで呼び、snapshot と復元をなくす**：理由は 2 つ。1 つ目は、`{#each}` の DOM と `OptimisticSortingPlugin` が食い違って固まること（[2026-02-28 のメモ](../../2026-02-28/workbook-order/plan.md)「@dnd-kit/svelte 実装上の注意点」）。2 つ目は C2 が悪化すること。列の外で離すと target が無く `move()` は何もしないが、dnd-kit はキャンセル時しか DOM を戻さないので、画面だけ移動後の並びになる。 -- **ドラッグ操作の e2e を足す**：マウス操作は過去に不安定で断念しており、CI でも e2e は動いていない。キーボード操作の e2e も今回は見送り。 - -## 既知の課題（#4183 では扱わない） - -- **保存失敗時の復元が、後続のドラッグの結果を上書きする**：保存 A の待機中にドラッグ B を始め、その後 A が失敗すると、A のドラッグ前の並びで B の結果を上書きする。B が保存済みなら、表示と DB がずれる。以前からある競合で、snapshot の修正では解消しない。保存の失敗と保存待ち中のドラッグが重なったときだけ起きるので、記録のみとした。直す場合は、保存が終わるまで次のドラッグを始められないようにするのが最小。 - -## レビュー（2026-10-18） - -- **Codex**：Critical・Severe は無し。Moderate の 1 件は「既知の課題」に記録した。Minor の「並びが変わっていないキャンセルでも restore を返す」テストの追加は見送った。この場合は元に戻しても戻さなくても画面が同じで、価値が低いため。復元で dnd-kit の並びがずれたり固まったりしないかはコードから判断できないとされたので、Phase 4 の手動確認で見る。 -- **simplify（4 観点）**：Reuse・Simplification の指摘を受け、テストで 3 か所に重複していた並びを fixture `solutionColumnsGraphSwapped` に切り出した。Efficiency・Altitude は指摘なし。Altitude の補足を受けて、「`move()` を `onDragEnd` だけで呼ぶ」案を却下理由とともに追記した。 - -## 手動確認 - -### 自動テストで代替できない理由 - -- **CI**：e2e のステップはコメントアウトされている（`.github/workflows/ci.yml` の `TODO: Run integration test`）。 -- **e2e**：`e2e/workbook_order.spec.ts` は並べ替えを API への直接 POST で検証しており、ドラッグを通らない。 -- **unit**：jsdom には画面配置の計算が無い（`getBoundingClientRect` が 0 を返す）ので、dnd-kit の重なり判定や並び位置の計算を動かせない。 -- **見た目**：半透明・ハイライト・自動スクロール・素早い操作への追従は、テストで判定すること自体が難しい。 - -### 環境 - -- 保存を伴う確認はローカルで行い、共有 DB には書き込まない。ローカル DB なので、確認後に並びを戻す必要はない。 -- データは `pnpm db:seed` で placement が作られる。未配置の問題集があれば、画面の初期化ボタンで作る。カードが 2 枚以上ある列と、スクロールが出るほどカードの多い列が必要。 -- 管理者でログインする。ブラウザーは普段使うもの 1 つでよい。 -- devcontainer で `pnpm i` が registry.npmjs.org からの取得で止まった場合は、`sudo /usr/local/bin/init-firewall.sh` で許可リストを作り直す。ファイアウォールは起動時に 1 回だけ DNS を引いて IP を許可するので、そのとき一部の IP しか返らないと、他の IP への接続が拒否される（推定。ipset の中身は未確認）。 - -### dnd-kit 更新時の確認項目 - -| ID | 操作 | 期待 | -| --- | --------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------- | -| M1 | 列をまたいで移動（未分類 → グラフ）し、リロード | ドラッグ中に移動先の列が濃いグレー、掴んだカードが半透明になる。固まらない。リロード後も同じ | -| M2 | 同じ列の中で並べ替え、リロード | 挿入位置が正しく、リロード後も同じ | -| M3 | カードの余白（ラベル付近）を掴み、別カードのタイトル上で離す | 離した直後に問題集の詳細ページへ移動しない | -| M4 | タイトルリンクを普通にクリック | 詳細ページへ移動する | -| M5 | 動かしたカードを続けて再ドラッグ（往復、3 回以上） | カードがポインターに追従し、位置がずれない。挿入位置が正しい | -| M6 | キーボード：Tab でフォーカス → Space / Enter で掴む → 矢印で移動 → Space / Enter で確定 | 各キーが反応し、確定後にリロードしても同じ | -| M7 | タブ切替（解法別 ⇄ カリキュラム）の後に M1・M5 | M1・M5 と同じ | -| M8 | カードの多い列で上下端へ、級列の多いカリキュラムタブで左右端へドラッグ | 自動スクロールが効き、離した位置に入る | -| M9 | 掴んですぐ素早く動かす | カードがポインター位置に追いつく | - -0.5.1 で M1〜M9 を選んだ根拠は、上流の変更との対応である。M1・M2 は #2135（Svelte 版の状態購読）、M3・M4 は #2152（ドロップ直後のクリック）、M5 は #2149・#2142、M6 は #2094（キー判定を `event.key` へ変更）、M7 は #2142、M8 は #2126、M9 は #2151 に対応する。NG が出たら旧バージョンでも同じ操作を行い、退行かどうかを切り分ける。回避用のコードは足さない。 - -### #4183 の確認項目（Phase 4） - -- **C1**：Escape 後に表示が元に戻り、リロード後も元の並びのまま。ポインター操作中とキーボード操作中の両方で確認する。 -- **C2**：列の外で離した後、リロードしても離した時点の表示と同じ並びになる。 -- **キャンセル直後の再ドラッグ**：同じカードをすぐ再ドラッグでき、固まらない。 -- **通常のドラッグ**：M1・M2・M5 が従来どおり保存される。 -- **保存失敗時の復元**：DevTools でオフラインにしてドラッグすると、エラー表示が出て元の並びに戻る。 - -## 確認結果 - -| ID | 修正前 | 修正後（#4183） | メモ | -| -------------- | -------------------------------- | --------------- | ---------- | -| C1 | 保存される（予想どおり） | OK | 2026-10-18 | -| C2 | 表示と DB がずれる（予想どおり） | OK | 2026-10-18 | -| 再ドラッグ | — | OK | 2026-10-18 | -| M1 | OK | OK | 2026-10-18 | -| M2 | OK | OK | 2026-10-18 | -| M5 | OK | OK | 2026-10-18 | -| 保存失敗の復元 | — | OK | 2026-10-18 | - -修正前の M1〜M9 は、0.5.1（#4165）でローカルで確認した。この表に無い M3・M4・M6〜M9 も同じ確認に含まれる。 - -修正後の確認は、#4183 のブランチでローカルで行った。 - -確認日：2026-10-18