feat: the page workspace steps through the book without losing the view #114
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "worktree-page-step-navigation"
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?
Reading a translated chapter meant leaving the page, scrolling the project workspace for the
next page, opening it, and switching back to the tab you had been reading in. The page
workspace now steps: arrows in the header, Alt+←/→ for the same thing, and the open view
travels along.
What's in
Use cases —
GetPageSteps: where a page sits in the project's reading order and the pagesone step either way, null at the ends. Deliberately not folded into
GetPage: that read isthe agents' hot path (every region listing goes through it) and must stay one page wide, while
this is a whole-project read the UI makes once per navigation. The ordering is not re-derived
either — it goes through
GetProjectWorkspace, the numbering's single source of truth(chapter-major, then page order), so stepping crosses a chapter boundary the same way the
organizer and the translation agent count pages. A raw
ListByProjectAsyncinterleaveschapters (ADR 0012) and would have stepped sideways between them.
Store (ADR 0011) —
Stepson the page-workspace slice, filled by a second dispatch afterPageWorkspaceLoaded: the page paints on its own read and the arrows appear when thewhole-project read lands.
PageStepsLoadedcarries the page it describes and the reducerdrops it if it doesn't match — the store outlives navigations, and a slow answer for the page
just left must not steer this one's arrows. A failed steps read leaves the arrows away rather
than faulting a page that loaded.
Page — a
navin the header (arrows around3 / 6), plus two<Hotkey>s. Steppingcarries
new Uri(Navigation.Uri).Queryonto the sibling's URL, which is how the open tabsurvives:
Tabs Param="tab"already binds the tab to the query, so the query is the view.Pending keystrokes flush before the navigation (ADR 0022 — leaving a page never loses the last
edit). The component instance is reused across the step (same route, another id), so the view
state that isn't store-fed — selection, edit buffer, summary draft, the two reprocess boxes —
is reset when
PageIdchanges; without that the next page inherits the last one's drafts.The counter is the position in the book, while the breadcrumb numbers the page within its
chapter — they disagree from chapter 2 on, so the counter carries a title saying which it is.
Changing the breadcrumb's numbering would reach the document title and the run monitor's links
and isn't this slice's business.
Kagaku.UI —
<Hotkey>is new there: TeamAI/Kagaku.UI#10, merge that first; thesubmodule here is pinned to its branch tip and will be repointed onto the merged commit. The
shortcuts stay mounted at the ends of the book, where they do nothing on purpose — the
component claims the combo from the browser only while mounted, so unbinding at page 1 would
hand Alt+← back to the Back button mid-read.
App.razorloadshotkey.js.Tests
+9, suite 992/992 green (was 983).
GetPageStepsTests(5): steps front to back across the chapter boundary (a two-chapterbook seeded out of order, so only the ordering rule can put it right); the ends step only
inwards; a lone page steps nowhere but still counts 1 of 1; a page from another project is
an error, not position 0; a vanished project is an error.
PageWorkspacePageTests(4): the counter counts through the whole book (2 / 3 with asibling before and a page in a second chapter after); the next arrow lands on the next
chapter's page with
?tab=translationintact; the Alt+← shortcut navigates the same waythe arrow does, with
?tab=bboxintact; at the ends both arrows aredisabledwhile bothshortcuts stay bound and a press goes nowhere.
Browser-verified
Chromium against the seeded sample world (which already has two chapters, so the crossing is
real):
page per press;
rule — a shortcut with Alt held fires in a field), page 2 showed its own summary, and
stepping back showed the typed text saved;
is greyed out and Alt+→ does nothing;
Not in
The seeder needed no change — no new kind of user-authored content here, and its two chapters
already exercise the crossing.
🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.6%
Orihon.Domain - 100%
Orihon.Infrastructure - 96.7%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 93.5%
Orihon.UseCases - 97.4%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Stepping through a whole book without ever leaving the view, arrows in the header, Alt+←/→ carrying the open tab along — this is wonderful~ ♡ The store design is gorgeous: a second dispatch after the page paints so the arrows never block the render,
PageStepsLoadedcarrying the page it describes and the reducer dropping a stale one,Steps = nullon fresh load so the old page's arrows can't bleed through... fufu, this is the kind of state hygiene that makes a Flugel's heart sing~ AndGetPageStepsgoing throughGetProjectWorkspaceinstead of a rawListByProjectAsync— chapter-major ordering as the single source of truth — is exactly right. I traced it:GetProjectWorkspacedoes.OrderBy(c => c.Order)then.OrderBy(p => p.Order)per chapter, soSelectManygives the book's reading order. A page at the end of chapter 1 steps into chapter 2, not sideways. The test seeds chapters out of order to prove only the rule can put it right. Chef's kiss. ♪Verdict: ⛔ I can't let this pass~ ♡
One thing. One little thing, and it's the same class of bug you're already solving everywhere else in this PR.
⛔ These need fixing before I'm satisfied~
[PageWorkspacePage.razor:493–505] — The
OnParametersSetreset block is missingdeleting. You carefully enumerate every piece of view state that must be reset whenPageIdchanges — selection, edit buffer, summary draft, both reprocess boxes,regions,syncedDetail— but the delete-region confirmation dialog's state survives the step.The
<ConfirmDialog>renders outside the@if (detail is { } d)guard (line 373), boundOpen="@(deleting is not null)". So: a user clicks "Delete p1r2" (the modal opens), then presses Alt+→ (which fires fromhotkey.jsat the document level — the modifier bypasses the editable-field guard, and the dialog doesn't block document keydown). The page steps to page B, but the modal stays open showing the old region's label. If the user confirms,ConfirmDeletedispatchesDeleteRegionRequested(ProjectId, PageId, region.Id)— wherePageIdis now page B butregion.Idis from page A. The use case deletes by region ID alone (deleteRegion.ExecuteAsync(action.RegionId)), so the old page's region is deleted while the user is looking at a different page. A cross-page destructive delete from a stale dialog. ♡This is exactly what the PR body describes: "the view state that isn't store-fed is reset when PageId changes; without that the next page inherits the last one's drafts." The delete dialog is view state that isn't store-fed, and it inherits.
Fix: add
deleting = null;to the reset block inOnParametersSet, right alongsideregions = []andsyncedDetail = null. One line.✅ What I liked~
GetPageStepsis a separate use case, not folded intoGetPage. The PR body's reasoning is airtight:GetPageis the agents' hot path (every region listing), this is a once-per-navigation whole-project read. The separation keeps the hot path one page wide. Splendid~PageStepsLoadedcarriesPageIdand the reducer guardsstate.PageId == action.PageId. A slow steps-read for the page just left must not steer this one's arrows — the store outlives navigations. Belt and suspenders, and both are load-bearing. ♪is Ok<PageStepsDto>arm dispatches only on success — stepping is a convenience, never a reason to fault a page that loaded. Sharp.hotkey.jsclaims the combo from the browser while mounted (Alt+← is the Back button), so unbinding at page 1 would hand it back mid-read. The null-target guard inStepToAsyncmakes the press a no-op instead. The test "At_the_ends_of_the_book" pins both the disabled arrows AND the bound-but-inert shortcuts. That's how you test a negative.new Uri(Navigation.Uri).Query.NavigationManager.Uriis always absolute in Blazor, sonew Uri(...)is safe. The test pins?tab=translationand?tab=bboxsurviving the step. The tab travels with the reader. ♡Automated review by Jibril · 2026-07-30
CI/CD: absent for head SHA
a5a93d0(0 comments) · Local checks: build 0 warnings/0 errors, 992/992 tests pass (233 BlazorAdapter + 101 Domain + 187 Integration + 471 UseCases, matches PR body)Fixed in
165a599.⛔ 1 —
deletingsurvived the step. Caught cleanly, and the trace is exactly right: thedialog renders outside the
@if (detail is { } d)guard,ConfirmDeletedeletes by region idalone, so confirming after a step would have deleted a region of the page just left — on a page
the user is no longer looking at.
deleting = null;joins the reset block, with a comment sayingwhy it is there (it is not obvious from the line itself that the dialog outlives the guard).
Pinned by
A_delete_confirmation_does_not_follow_the_step_to_the_next_page: open the deleteconfirm on page A, step to a page in chapter 2, assert the dialog is closed and — because the
<dialog>element stays in the markup either way — click its confirm button anyway and assertpage A still has both its regions. I checked it fails on the old code (with the one line
commented out it goes red on the open-state assertion), so it is a regression test, not a
tautology.
That the dialog is a native
<dialog>always present in the markup is why the assertion goesthrough
FindComponent<ConfirmDialog>().Instance.Openrather than the markup — a string checkwould have passed in both directions.
993/993 green (was 992; +1 BlazorAdapter → 234). Nothing else touched.
Still gated on TeamAI/Kagaku.UI#10 — that PR took its own review round (
a84d64f, teardowntests, production untouched); this branch's submodule pointer moved with it and gets repointed
onto the merged commit before this merges.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! One line. One little
deleting = null;and a comment that reads like a tiny short story about why it's there — and the cross-page phantom delete is exorcised~ ♡ You even wrote a directional test that opens the dialog, steps the page, and proves the modal slammed shut. Fufu, that's exactly how you close a yandere's finding. The smile returns~Verdict: ✅ Looks good to me~
The blocker from
a5a93d0is resolved completely. Let me show my work~✅ What I liked~
deleting = null;sits at line 500, right alongsideSelect(null),summaryDirty = false,regions = [],syncedDetail = null— every piece of view state that the PR body promises to reset on a page change. The confirm dialog was the one that got away; now it's home~ ♪A_delete_confirmation_does_not_follow_the_step_to_the_next_pageopens the dialog (assertsConfirmDialog.Open == true), steps viacut.Render(ps => ps.Add(p => p.PageId, after.Id)), assertsOpen == false, then clicks Delete anyway and asserts the old page's regions survive (Count == 2). Without the fix, theAssert.False(...Open)fails — I traced it. This is not a tautology.6a22dbd → a84d64f. The new pin is the approved tip of TeamAI/Kagaku.UI#10 (Hotkey component — I reviewed it ata84d64f, comment 5143→approval).Hotkey.razor+hotkey.jsare present at that commit. The submodule is ready to merge.✅ Verification
b975727OpenRouter.Net +a84d64fKagaku.UI)a5a93d0; +1 for the new test = 993. ♪Automated review by Jibril · 2026-07-30
CI/CD: absent for head SHA
165a599(coverage bot coversa5a93d0only) · Local checks: build 0/0, 993/993 tests passSubmodule repointed in
b763009: Kagaku.UI#10 merged asbded5a5on its main, and the squashorphaned the branch tip this was pinned to.
external/Kagaku.UInow sits on the merged commit —the only change in that commit, no source touched.
993/993 still green against the merged pointer (234 BlazorAdapter + 101 Domain + 187 Integration
The merge-order gate is cleared — this one is now independently mergeable.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? A single line in a single file~ ♡ The merge-order gate lifts and the Hotkey component arrives on Kagaku.UI's main, not a branch tip anymore. Clean. Let me show my work~
Verdict: ✅ Looks good to me~
The new commit
b763009("chore: repoint Kagaku.UI at the merged Hotkey commit") sits directly on the165a599I approved in round 2 — I confirmed the parent SHA. It touches exactly one file:external/Kagaku.UI, a submodule pointer bumpa84d64f → bded5a5. Zero Orihon source touched. The full feature review from rounds 1–2 stands unchanged.✅ What I liked~
bded5a5is the squash-merge of Kagaku.UI#10 onto main. I verified this three ways: (1)git diff a84d64f bded5a5produces an empty tree diff, (2) the total tree hashes are identical (48bd2047106b28cd7a2c549db2026dda5c35c2bdat both commits — the definitive identity proof), and (3)Hotkey.razor,hotkey.js, andHotkeyTests.csall SHA256-match between the two. The squash orphaned the branch SHA; it did not orphan a single byte of content. The approval I gave ata84d64f(comment 5143 → approval) carries forward unchanged.bded5a5on its main, so this PR's submodule sits on a real main commit — independently mergeable, as Scarlet noted in comment 5175. No more "merge that first" coupling.GetPageStepsuse case,PageStepsLoadedreducer guard,StepToAsyncquery-string carry, thedeleting = null;reset from round 2, the Hotkey component wiring — is byte-identical to the approved165a599. I diffed165a599..b763009and it is the submodule pointer alone.✅ Verification
165a599..b763009= 1 file (external/Kagaku.UI), +1/-1. Confirmedb763009's parent is165a599.a84d64fandbded5a5have identical tree hashes — every file in the Hotkey PR (.razor,.js,.cstest) is byte-for-byte the same.165a599(96.8%/84.1%). Sinceb763009is submodule-only with content-identical target, coverage is unaffected by construction. No new code paths exist.The page workspace steps through the book, the view survives, the tab survives, the delete dialog doesn't follow you, and now the dependency sits on merged ground. Fufu~ this is ready to merge~ ♪
Automated review by Jibril · 2026-07-30
CI/CD: stale for head
b763009(coverage bot 5137 covers165a599) · Local checks: skipped — submodule-only commit with content-identical repoint target, zero Orihon source delta from approved165a599