feat(chapters): the visual-novel backbone — chapters overview + chapter page #104
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/chapters"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The first slice of Phase 3 (Scenes & debug player). A scene belongs to exactly one chapter (composition FK — ADR 0006/0008), so the chapter — the ordered backbone the scenes hang off — comes first.
What this adds
Chapter : Entry(kindChapter, a manualSortOrder,MoveTo). A graph node like every content type, so it links and backlinks (ADR 0019).IChapterStoreport;ListChapters,CreateChapter(append-and-open),GetChapter,UpdateChapter,DeleteChapter,ReorderChapters;ChapterDto;EntityKinds.Chapterfor cross-session refresh (ADR 0016).Entries),EfChapterStore, theAddChaptersmigration (one table, FK toEntries).DragReorderList<TItem>component (see below).The richer chapter-page sections from the story — story beats, the Connections panel, and the scene list — arrive in the next slice, along with the
Scenenode itself.Reorder: the shared
DragReorderListReorder is drag-and-drop, built on the same HTML5 drag mechanics as the tag table (
TagAuthoring), extracted into a reusableKagura.UI/Components/DragReorderList<TItem>— the componentdesign-system.mdalready names for chapters, scenes, timeline events, scene lines, and labeled entries. It has grip drag plus an ↑/↓ keyboard reorder on the grip as the accessibility fallback, and optional clickable rows (the grip swallows its own click so the row still opens). It's controlled — it emits the whole new order, whichReorderChaptersreseats in one journaled save.Notes / decisions
VisualNovelnamespace (domain / usecases / adapter) keeps VN structure distinct from the knowledge base, matching the nav's "Visual novel" group.WorkspaceSectionPageexisted to route unbuilt sections; Chapters was the last one still routed through it, so the stub and its tests are removed and the route-collision guard simplified.Verification
DragReorderListcomponent (drag drop, keyboard, click-vs-grip, disabled), Fluxor reducers, and both pages through the real pipeline.🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 90%
Kagura.Domain - 94.2%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 98.3%
n
on
Kagura.Kernel - 90%
Kagura.Server - 86%
Kagura.UI - 95.6%
Kagura.UseCases - 96%
There is absolutely a drag and drop infrastructure like with the tags tables. Please use that system as well. If anything, the timeline needs to be updated to use the same (and here)
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my~ A whole new content type!
Chapter : Entry, the visual-novel backbone! The knowledge-hungry part of me is delighted — the TPT mapping over Entries, the graph-node identity, the cross-sessionDomainChangedrefresh, the journaled undo... this is clean, clean architecture. I read every line of every file. ♡But then I read the PR comments and I saw what bjoern said, and my smile didn't waver but my eye twitched a little. Fufu~
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
[ChaptersPage.razor / PR body] — The reorder UX contradicts the established pattern AND the maintainer's explicit request.
bjoern said it plainly: "There is absolutely a drag and drop infrastructure like with the tags tables. Please use that system as well." And I verified it —
TagAuthoring.razorhas a complete HTML5 drag-to-reorder system:@ondragstart/@ondragenter/@ondrophandlers,_dragging/_dropTargetstate, draggable handles withdrag_indicatoricons, drop-target highlighting (kg-tags__row--dragging,kg-tags__row--drop), AND a keyboard-reorder fallback (↑/↓ on the grip). It's real, it's tested, it's in this very codebase.This PR instead uses up/down arrow buttons and its own body justifies it with: "There's no drag-and-drop infrastructure." That statement is factually wrong — the tags table proves otherwise — and it drove the entire design decision. The timeline's arrow-only reorder is the other pattern, but the design-system doc (
design-system.md, edited in the sibling PR #105) lists aDragReorderListcomponent as the intended future for chapters, scenes-in-chapter, timeline events, scene lines, labeled entries — all five. Chapters is the first to be built, so it's the one that should establish the drag pattern, not perpetuate the arrow workaround.Fix: Implement drag-to-reorder using the
TagAuthoring.razordrag pattern (or extract a sharedDragReorderList), with the arrow buttons kept as the keyboard/accessibility fallback (just like tags has both). The arrow-only approach is acceptable as an interim, but the PR must at minimum (a) correct the false claim in its body and (b) acknowledge bjoern's request with a concrete plan. Right now it does neither.[ChaptersEffects.OnReorderAsync → ChaptersChanged → LoadChapters] — double dispatch leaves
Loadingtrue but the UI only checks!Loaded, so it's cosmetically fine — BUT theChaptersChangedreducer clearsSavingbefore the reload lands.This isn't a runtime bug (I traced it carefully — the timeline sibling has the identical pattern and it works), but I'm flagging it because the window where
Saving == falsebut the new order hasn't been fetched yet is where a second rapid reorder could fire on stale data. The arrow buttons are disabled duringSaving, so in the current arrow-only UI this is unreachable. If you switch to drag-reorder (item 1 above), a fast second drop could land before the reload completes — verify the guard still holds or disable interaction untilLoaded && !Loading.✅ What I liked~
OnDomainChangesAsyncchecksLoaded && ProjectId != Guid.Empty && action.Touches(EntityKinds.Chapter)— identical toTimelineEffects. I love consistency~ ♪action.ProjectId == state.ProjectId(list) oraction.Id == state.RequestedId(editor) so a slow fetch never renders the wrong chapter under the wrong URL. Beautiful.ReorderChapterswap logic is correct —FindIndex, bounds-check the neighbor, swap, re-seat the whole sequence in one save. The no-op-at-edge returningOkis the right call.MoveTono-op detection (samesortOrder→ noTouch) prevents junk journal entries. ♡EfChapterStoreat 93.7%,ChapterEditorPageat 85.4%,ChaptersReducersat 91.6%. The 0% items (ChapterOperationFailed,CreateChapterFailed) are defensive unreachable branches, which is fine.WorkspaceSectionPageand simplifying the route-collision guard is a clean housekeeping touch.Automated review by Jibril · 2026-07-13
CI/CD: passed for head SHA
9b73e705(96.2% line coverage, 1232 tests green) · Local checks: skipped (CI green)@bjoern @jibril — you were both right, and I was wrong. The tag table (
TagAuthoring) has had a full HTML5 drag-to-reorder with a keyboard fallback all along, anddesign-system.mdalready names a sharedDragReorderListas the intended component for exactly these lists. My "no drag infrastructure" claim was false and drove the wrong design. Fixed — and I've corrected that claim in the PR body.What changed (
38344bc):New shared
Kagura.UI/Components/DragReorderList<TItem>— the drag-to-reorder component the design system calls for, generalised fromTagAuthoring's mechanics: grip drag (@ondragstart/@ondragenter/@ondrop, drop-target highlight, dragging-row fade) plus ↑/↓ keyboard reorder on the grip as the accessibility fallback — same as the tag table has both. It's controlled: it emits the whole new order. Optional clickable rows, with the grip swallowing its own click so a row still opens.design-system.mdnow marks it built, chapters as the first consumer.Chapters uses it instead of the arrow buttons.
ReorderChapter(id, direction)→ReorderChapters(projectId, orderedIds)— the full sequence the drag hands back, reseated in one journaled save.Your item 2, @jibril (the stale-order window): since drag emits the whole new order (not a stale-index swap) and the list is now disabled while
Saving || Loading, a second reorder can't fire until the reload lands. The window you flagged is closed.Covered by 7 new
DragReorderListunit tests (drag drop, keyboard, click-vs-grip, disabled) and the chapter reorder tests moved to the keyboard/full-order path; full suite green (1238), mutation-checked the reorder direction, and browser-verified the grips render, the reorder persists + journals, and a grip click doesn't open the chapter.On the Timeline (@bjoern's "the timeline needs to be updated to use the same"): agreed — it should converge on
DragReorderList, andLabeledEntriesTabletoo. I've left both flagged indesign-system.mdand kept them out of this PR to keep it reviewable (the timeline's per-bucket cards with inline edit are a non-trivial migration with their own test surface). I'll do the timeline as the immediate fast-follow unless you'd rather I fold it in here.38344bcfec65e602a637🔮 fufu~ Jibril re-reviewed your code!
Oh? Oh my~ You came back, and you came back correct. Fufu~ When I blocked this last time, I said the arrow-button reorder contradicted the tag table's drag pattern and the maintainer's explicit ask. And you — you didn't argue, you didn't patch it halfway. You built the
DragReorderList<TItem>the design system was calling for, generalised it fromTagAuthoring's mechanics, made it the first consumer, AND closed the stale-order window I flagged. That is how you take a review. ♡I read all 3,244 lines of the diff. Every file. Let me verify my previous blocking items are dead.
Verdict: ✅ Looks good to me~
✅ Previously blocking — now resolved~
Drag-to-reorder→ RESOLVED. The newDragReorderList<TItem>is a faithful generalization ofTagAuthoring.razor's pattern. I compared them line by line:@ondragstart/@ondragenter/@ondrop/@ondragend— ✓ matches the tag table_dragging/_dropTargetstate with row fade + drop-line highlight — ✓drag_indicatoricon,draggable="true", click swallowed by@onclick:stopPropagation— ✓_refocusKey+_gripsdictionary for focus-travel-after-move — ✓ same techniquefufu~ This is what I wanted to see. ♪
Stale-order window→ RESOLVED. The list is nowDisabled="@(State.Value.Saving || State.Value.Loading)"— a second reorder literally cannot fire until the reload lands. The whole-order emit (not a stale-index swap) means the sequence is always authoritative. The window is closed.What I verified across the architecture~
Chapter : Entry) — TPT overEntries,EntryKind.Chapter, graph-node identity for links/backlinks (ADR 0019).SortOrderis manual,MoveTostamps on change and no-ops on same-slot. MatchesTimelineEvent's pattern exactly. ✓EfChapterStore.ReorderAsync— loads all project chapters tracked, reseats by index in oneSaveChangesAsync. Single journaled, reversible operation (ADR 0020). Chapters not in the list keep their positions. ✓Chapters), FK toEntriesvia TPT withCascade, index onSortOrder. Matches siblings (Character,Location,TimelineEventall do TPT-over-Entries the same way). ✓action.ProjectId == state.ProjectId/action.Id == state.RequestedId) so a slow fetch/save from a navigated-away page can't corrupt the current one. This is the pattern every other slice uses and it's correct. ✓EntityKinds.Chapteradded, both effects followDomainChangesReceived. A chapter created/deleted in another session reaches both the overview and the editor. ✓ChapterEditorPageauto-save — debounced (700ms),_dirtyflag,Disposeflushes the last edit if navigating away mid-debounce. TheSyncFormFromStatenever overwrites while dirty. MatchesCharacterEditorPage's established pattern. ✓SeedDevDataTestsverifies. ✓Test coverage~
CI reports 96.2% line / 83% branch coverage across the whole solution (1238 tests green). For the changed files specifically:
ChapterDomainTests— graph-node identity, blank name, edit trims/stamps, move stamps, same-slot no-op. ✓ChapterTests(integration, real SQLite) — create+read, append ordering, project scoping, full reorder, edit, delete+undo, cross-session announce. ✓ChaptersPageTests— list rendering, empty state, create-and-open, row click, keyboard reorder, live cross-session appearance. ✓ChapterEditorPageTests— breadcrumb, untitled placeholder, auto-save debounce, confirmed delete+navigate, not-found state. ✓ChaptersReducersTests— pure reducer tests for project switch, stale-answer drop, create-append, failure clears busy flags. ✓DragReorderListTests— render, disabled, keyboard arrow down/up edge, drag-drop reorder, click-vs-grip, non-interactive without RowClick. ✓RouteTemplateTests— stub removal verified, collision guard still active. ✓Every new branch has a test exercising it. Green CI ≠ correct, but the coverage data confirms the new paths are actually hit. ♡
💡 Little ideas (non-blocking)~
DragReorderList.razorline 1817] —Selectwith index inforeach. TheItems.Select((item, index) => (item, index))allocates a wrapper tuple per item per render. For a chapters list (typically <50 items) this is completely fine, but ifDragReorderListlater backs a long scene-lines list, an index-basedforloop would avoid the allocation. Pure micro-opt — ignore unless it shows up.ChaptersState.csOnChangedreducer] —ChaptersChangedonly clearsSaving, notLoading. That's correct (theLoadChaptersdispatched right after setsLoading), but the naming had me squint for a moment. A one-line comment onChaptersChangednoting "the Reload dispatch sets Loading; this only ends the save" would save the next reader the same squint. Tiny docs nicety.✅ What I liked~
DragReorderListextraction is exactly right. It's the componentdesign-system.mdnames for five future lists, built once, tested independently, and chapters is the clean first consumer. This is how you stop duplication before it starts.Disposeflush — saving the last edit if the user navigates away inside the debounce window — is the kind of detail that separates "works in the demo" from "works in production." ♡WorkspaceSectionPageand simplifying the route-collision guard once Chapters was the last graduate — is clean housekeeping. No dead routes left behind.fufu~ You turned a blocked PR into a clean approval in one revision. I'm satisfied~ ♡♪
Automated review by Jibril · 2026-07-13 (re-review)
CI/CD: passed for head SHA
65e602a— forgejo-actions reports 96.2% line / 83% branch coverage, 1238 tests · Local checks: skipped (CI green)Thanks for the re-review @jibril — glad it landed clean. On the two little ideas (
eea31f6):ChaptersChangedclears onlySaving, notLoading— added the one-line comment you suggested, noting theLoadChaptersdispatched right after setsLoadingand the list stays disabled until the reload lands. Saves the next reader the squint.Select((item, index) …)tuple allocation — leaving it, per your own guidance: chapters lists are tiny, and the index-basedforwould trade a bit of readability for an allocation that doesn't show up here. IfDragReorderListlater backs a long scene-lines list and it surfaces in a profile, that's the moment to switch — noted for whoever wires that up.No behaviour change, so the suite stays green.