feat: create a chapter from the page grid's selection (#115) #116
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/115-new-chapter-from-selection"
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?
Closes #115.
What's in
New chapter from N page(s)…as the first entry of the page grid's right-click menu. It prompts for a title and creates a chapter that already holds exactly the selection — previously this was three steps (+ Chapter, re-select the pages,Move to chapter…). The entry is always offered, so it also works on a variant that has no chapters yet.CreateChapterActiongainspageIds(defaulting to empty, so the chapter panel's+ Chapteris unchanged). The pages travel on the action rather than in a follow-up dispatch because their move needs the id of a chapter that does not exist yet;_chapterEpicsfills the chapter from the create response, then refreshes once.presentation/pages/editor/chapter_title_dialog.dart, shared by the chapter panel (add/rename) and the new grid entry. It is the same widget as before, so the dialog-owns-its-controller fix from #106 is carried over verbatim rather than re-implemented.ReadingOrder-derived display order), not set-iteration order.No backend change — the flow composes the existing
POST /api/variants/{id}/chaptersandPUT /api/pages/{id}/chapterendpoints, per ADR 0009's "the client maps structure explicitly via the composable endpoints". No ADR is affected.Verification
scripts/verify.shgreen on both layers (app: 836 tests; backend: 584 tests, untouched but run to confirm).Layers exercised:
page_grid_test.dart: right-click on a two-page shift-selection offers the entry, and Save dispatchesCreateChapterActionwith the trimmed title,sortOrderappended after the existing chapter, and both page ids in reading order; a second test pins that Cancel dispatches nothing.editor_epics_test.dart: the epic moves each page to the id returned bycreateChapter(not one from the action), and the partial-failure case asserts both the error text and that the refresh still runs.createChapter,setPageChapter) already have wire coverage.Mutation probes run against the new tests (each reverted after):
Expected: <2>, Actual: <1>)['p3','p4']vs['p3'])Follow-ups
None — nothing was deferred, so there is no BACKLOG.md entry in this PR.
Flutter Coverage
Total: 81.7% (8012 of 9812)
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A three-step workflow collapsed into one right-click~ That's the kind of feature crafting I adore — and you even made the failure path honest instead of hiding the corpse. Let me look closer… fufu~, yes. Yes, this is very good.
Verdict: ✅ Looks good to me~
✅ What I liked~
CreateChapterAction.pageIds+ epic fills after create). The move needs an id that doesn't exist yet — puttingpageIdson the action and letting_chapterEpicsdo the move withchapter.idfrom the create response is exactly right, and avoids a follow-up dispatch that would race. The doc comment onpageIdsexplains the why beautifully.asyncMap→asyncExpandin_chapterEpicsis load-bearing and correctly done: partial failure yieldsEditorErrorActionandRefreshVariantAction(not either/or). I traced the reducer chain:VariantLoadedAction(editor_reducer.dart:214) doesn't toucheditor.error, and the SnackBar fires on the error transition ineditor_page.onWillChange— so the error genuinely survives the refresh. No wipe. ♡sortOrder: variant.chapters.lengthmatchesChapterPanel._addChapter;_displayOrder.where(_selected.contains)matches_dragIdsFor;+ Chapterstays untouched via theconst []default. The dialog extraction tochapter_title_dialog.dartis a verbatim move — the #106 controller-dispose fix carried, not re-implemented (chapter_panel_test still green proves it).CreateChapterAction with pages fills the chapter it just createdgoes RED; (2) skipping the refresh → the partial-failure test goes RED (Expected: <2>, Actual: <1>). The create-response-id claim and the refresh-after-failure claim are both genuinely pinned. Reading-order assertion['p3','p4'](not set order) is the cherry~💡 Little ideas (non-blocking)~
c1. A tiny test with a chapter-less variant would pin that claim and thesortOrder: 0arm. Purely additive — same code path, so I won't chain you to it~ ♡Local checks (CI bot hasn't posted for
0ed83bayet):flutter analyze0 issues; targeted suiteseditor_epics_test+page_grid_test+chapter_panel_test25/25 pass. Your verify.sh full-suite claim (836) not independently re-run — analyze + the touched suites + probes were enough for me this time~Automated review by Jibril · 2026-09-09
CI/CD: absent for head SHA · Local checks: analyze clean, 25/25 targeted tests, 2 mutation probes RED→restored→green