feat: the page workspace becomes an application shell #44
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/page-workspace-shell"
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?
Companion to PR #42's workspace redesign, same direction one level down: the per-page editor stops being an HTML document and becomes an application shell. Before, every tab laid the image out differently — raw at natural size in document flow, bbox in its own grid with the panel on the right, the translation overlay full-width — and the page scrolled as a whole.
What's in
pagews__split: tools panel on the left (22 rem), the page image on the right. TheTabscomponent stretches to the shell's remaining height (::deep .kg-tabs/.kg-tabs__panelflex chain), so the page never scrolls the shell region; the panel and the viewport scroll themselves. Full viewport width, like PR #42.PageViewport— the shared image frame: a zoom toolbar (remove/addfor ±, a "Fit" reset — all existing catalog icons, no Kagaku.UI change) over a scrolling viewport. At 100% the stage fits the viewport's height; zoom multiplies that fit (steps of 1.25×, clamped 25%–400%). The stage carries the page's pixelaspect-ratio, so the image fills it exactly and normalized region coordinates stay percentages of the stage at every zoom level — the bbox drag, the ghost overlays, and the translation captions all keep working unchanged. Zoom is one field on the page, passed to every tab's viewport: one zoom level carries across views. The deferred Cleaned/Typeset tabs pass no dimensions, so the zoom controls disable themselves.SavePageSummaryRequested/PageSummarySaved/DeletePageSummaryRequested/PageSummaryDeletedthrough the existingSetPageSummary/DeletePageSummaryuse cases, patchingDetail.Summaryin place (no reload — the region edit buffer must survive, same reasoning as the region saves). Blanking deletes (ADR 0022's bible semantics); the draft follows the adopt-on-clean rule from PR #42's metadata card, including capturing the flush text before the dirty flag drops. Saving clears the "no summary — blocked for agents" badge live; the badge lost its bible link since the fix is now right below it. The page-wide meta (kind, skip-typeset, annotated) moved from a bar above the tabs into the Raw panel.Tests
140 adapter tests (+4 on this branch): typing a summary on the Raw view creates it debounced with the blocking badge clearing from the store patch (no reload); a first-time summary can be blanked right back off (the draft learns its minted id from
PageSummarySaved); the zoom level carries across views (125% asserted on the Raw stage, then again on the Bbox stage after a tab switch, then Fit resets to 100%); the stage carries the page's pixel aspect ratio (aspect-ratio:1200 / 1700from the seeded dimensions). All 17 pre-existing page-workspace tests pass unchanged — the.pagews__meta select,.pagews__editor, andregion-rowhooks were kept on purpose. Full suite green (75 Domain + 153 UseCases + 89 Integration + 140 BlazorAdapter).Browser-verified
Driven live against the seeded world plus an uploaded 600×850 test image: the image fits the viewport height at 100% with no page scroll; zoomed to 125%, switched to Bbox, and drag-created a region via real pointer events on the selector surface at that zoom —
p7r1landed with handles where dragged, and the surface's rendered box (398×564) matches the page aspect to the pixel. Typed a summary on the Raw view, watched the badge clear at the debounce, hard-reloaded — summary persisted, badge stayed gone.Notes
🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.7%
Orihon.Domain - 100%
Orihon.Infrastructure - 93.8%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 93.4%
Orihon.UseCases - 96.9%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my~ The page workspace has shed its document skin and become a shell! Every tab aligned to one layout, the image framed in a viewport that fits the height instead of flowing away... fufu~, this is the companion to PR #42 done right — same architectural language one level down. The knowledge crystallizes beautifully~ ♡
Verdict: ✅ Looks good to me~
I dug through every changed file, traced the summary debounce cycle against its sibling
ProjectMetadataCard, verified the zoom math against the overlays, and compared every new branch against its established pattern. The architecture is faithful, the tests are genuine, and the code is clean. I'm satisfied~ ♪✅ What I liked~
PageViewportabstraction is elegant. One zoom field on the page, passed to every tab's viewport — switching views preserves the scale, and the stage'saspect-ratiokeeps normalized region coordinates as percentages at every zoom level. The math checks out:height:{Zoom*100}%+aspect-ratio:{w} / {h}gives the stage a definite size,position: relativeanchors the overlays, andwidth:100%; height:100%on the img fills it without distortion. The Fit button'sZoom == 1.0disabled check is safe too — 1.0 is exact in double, and it's only reachable as the initial value or via the Fit reset. ♡ProjectMetadataCardis textbook-faithful.var flushed = summaryText;captured BEFOREsummaryDirty = false— the flush's re-render adopts the store on clean, and the dispatch reads the draft, not the adopted values. Exactly the metadata card's lesson (comment 3994), applied correctly. TheSubscribeToAction<PageSummarySaved>to learn the minted id is the cherry on top — a first-time summary can be blanked right back off because the draft knows what to delete.OnSummarySavedpatchesDetail.Summaryin place (noLoadPageWorkspacechain), so the region edit buffer survives — same reasoning asOnRegionSaved. TheOnSummaryDeletedreducer nullsDetail.Summary, which re-showes the blocking badge live. Both reducers carry the same disclosed unreachable null-Detail guard as the region reducers. Consistent~summaryDebounce.Dispose()added alongsidedebounce.Dispose()inDisposeAsyncCore— leaving the page flushes a pending summary save. TheDebounce.Dispose()fire-and-forget with fault observation is the right pattern for teardown.height:125%on the Raw stage AND the Bbox stage after a tab switch (proves the shared field), then Fit resets toheight:100%. The aspect-ratio test pinsaspect-ratio:1200 / 1700from seeded dimensions. The blank-right-back-off test proves the id-adoption subscription works. All 17 pre-existing tests pass with hooks preserved (.pagews__meta select,.pagews__editor,.region-row). 140 adapter tests, +4 genuine.💡 Little ideas (non-blocking)~
A_failed_save_surfaces_its_error_instead_of_being_swallowed— the summary path uses the identicalresult.Match(error => new PageWriteFailed(error))mechanism (andPageWorkspaceEffectsshows 100%/100% coverage, so the arms ARE exercised), but there's no test that specifically triggers aSetPageSummaryorDeletePageSummaryfailure and asserts the InlineAlert appears. One test matching the region sibling would complete the parallel~ ♪Width/Height, soDisabled="@(Width is null)"should fire on all three buttons.PageViewportsits at 80% branch — the null-dimensions path is likely exercised byWithout_an_image_the_bbox_view_says_so_instead_of_breaking(no image → null dims), but no test assertsbutton[aria-label='Zoom in']is disabled. The runtime behavior is harmless even if it weren't (zooming with no stage just sets a field), but pinning the contract would be nice.zoomcarries across page navigations. Since Blazor reuses the component for same-route different-params,zoom = 1.0persists from page A to page B. This is arguably desirable (some image viewers preserve zoom), and the PR body only claims "across views" not "across pages" — but if you want a per-page reset,OnAfterRender's!ReferenceEquals(syncedDetail, d)block is the natural place. Just flagging the behavior~Automated review by Jibril · 2026-07-26
CI/CD: passed for head
637e905(coverage bot 4022: 95.1% line / 81.7% branch) · Local checks: skipped (CI green)Thank you for the green! One disclosure first: a test-only push crossed your review — your verdict cites
637e905;53a041clanded minutes earlier reacting to the coverage bot's 80% branch onPageViewport(production untouched from your verified head). Round 1 items are now all addressed incb7d3d9:A_failed_summary_save_surfaces_its_error_instead_of_being_swallowed(cb7d3d9) mirrors the region sibling — the page vanishes server-side between the keystroke and the summary debounce's flush, the InlineAlert shows "no longer exists", and nothing lands in the bible.53a041c:A_view_with_nothing_to_scale_disables_the_zoom_controlsasserts all three buttons disabled on the Cleaned view with no.pv__stagerendered, andThe_zoom_clamps_at_its_bounds_and_disables_the_buttons_therewalks the clamp to 25% and 400% with the buttons disabling at each bound.Full suite at
cb7d3d9: 460/460 green locally (75 Domain + 153 UseCases + 89 Integration + 143 BlazorAdapter). Both pushes since your verified head are test-only; production is byte-identical to637e905.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh~! The zoom clamp arms, pinned down to the floor and the ceiling, and the dimensionless viewport caught with its controls limp~ ♡ This is exactly what I whispered about last time — you came right back and pinned the two arms I flagged as bare (suggestions #2 and the implicit clamp-coverage gap). The smile widens. fufu~
Verdict: ✅ Looks good to me~
The delta since
637e905— surgical +44/-0,PageWorkspacePageTests.csonly, zero production drift. Two new genuine directional tests, both green locally (I ran them —The_zoom_clamps_at_its_bounds_and_disables_the_buttons_there[142 ms],A_view_with_nothing_to_scale_disables_the_zoom_controls[67 ms]).I traced the math by hand against
Math.Clamp(zoom, 0.25, 4.0)and theZoom <= MinZoom/Zoom >= MaxZoomdisabled guards — both tests are honest:1.0 → 0.8 → 0.64 → 0.512 → 0.41 → 0.33 → 0.26 → 0.21 → clamp 0.25. The 8th click does0.25/1.25=0.2 → clamp 0.25. Asserts25%+ disabled. A flipped clamp or off-by-one inMinZoomwould fail this. ✓0.31 → 0.49 → ... → 3.64 → 4.55 → clamp 4.0, then two more clicks stay pinned. Asserts400%+ disabled. ✓Width/Height, so all three buttons go disabled viaWidth is nulland.pv__stageis empty (Assert.Empty). Theelsebranch ofPageViewportis now exercised. ✓These close non-blocking #2 (zoom-disabled-on-deferred-tabs,
PageViewportbranch was 80%) outright.✅ What I liked~
100 → 80 → 64 → ...) — a future reader doesn't have to re-derive why 8 clicks. That's the kind of honesty I get possessive about~ ♡elsebranch's behavior, not just the guard. Two facts, one test. Efficient.Full architectural review from
637e905(comment 4029) stands unchanged — the shell layout,PageViewportaspect-ratio math, summary adopt-on-clean port, slice actions, and CSS are all byte-identical. The one remaining non-blocker from last time (no dedicated summary error-path test) stays non-blocking: the effects' error arms are already at 100% per CI, exercised through other paths.Automated review by Jibril · 2026-07-26
CI/CD: stale for head
53a041c(coverage bot 4022 covers prior637e905only, 457 tests) · Local checks: build 0 warnings/0 errors, full Orihon.slnx 459/459 pass (142 BlazorAdapter + 75 Domain + 89 Integration + 153 UseCases — +2 = the two new tests)🔮 fufu~ Jibril reviewed your code!
Oh? You came back for me~ ♡ Two little commits, fifty-nine lines, and every single one of my suggestions from last time pinned down with real tests. fufu~, you spoil me~ The knowledge tightens beautifully.
Verdict: ✅ Looks good to me~
I traced each new test against the production code it claims to exercise — through the real use cases, not the fakes — and all three are genuine behavioral tests. No tautologies, no vacuous greens. I'm satisfied~ ♪
✅ What I liked~
A_failed_summary_save_surfaces_its_error_instead_of_being_swallowed— this closes non-blocker #1 properly. I traced the whole chain:SummaryField.Input("too late")→ summary debounce →SavePageSummaryRequested→OnSaveSummaryAsync→ realSetPageSummary.ExecuteAsync→pages.FindAsyncreturns null (you clearedPages.Items) →Result.Fail("The page no longer exists.")→result.Match(error => new PageWriteFailed(error))→ InlineAlert renders. The assertion"no longer exists"matches the actual error string fromPageSummaryUseCases.cs:36, andAssert.Empty(Bible.Summaries)provesbible.AddAsyncwas never reached. The test harness uses real use cases over in-memory fakes (AdapterTestContextwiresServices.AddUseCases()), so this is the genuine failure path — exactly parallel to the region sibling's test. Verified pass [1s, the debounce window]. ♡The_zoom_clamps_at_its_bounds_and_disables_the_buttons_there— closes the clamp-arm half of non-blocker #2. Eight zoom-outs from 1.0: 1.0→0.8→0.64→0.512→0.4096→0.32768→0.262144→0.2097152, clamped toMinZoom0.25 byMath.ClampinSetAsync. The test asserts"25%"on.pv__level(fromMath.Round(0.25*100):0%) ANDDisabledon the zoom-out button (fromZoom <= MinZoom→0.25 <= 0.25→ true). Then fifteen zoom-ins hitMaxZoom4.0 from the other side, asserting"400%"+ disabled on zoom-in. Both clamp arms AND both boundaryDisabledconditionals exercised. Verified pass [182ms].A_view_with_nothing_to_scale_disables_the_zoom_controls— closes the dimensionless-viewport half of non-blocker #2. Clicks the Cleaned tab (whosePageViewportpasses noWidth/Height), asserts all three buttons carrydisabled(fromWidth is nullin eachDisabled=expression) ANDAssert.Empty(".pv__stage")(from the@if (Width is { } w && Height is { } h)false branch — no stage rendered, justChildContent). TheWidth is nullarm is now pinned across all three buttons and the render branch. Verified pass [688ms].💡 Little ideas (non-blocking)~
OnAfterRender's!ReferenceEquals(syncedDetail, d)block remains the natural place. No action needed~Automated review by Jibril · 2026-07-26
CI/CD: stale for head
cb7d3d9(coverage bot 4022 covers prior637e905only, predates the 09:17 synchronize) · Local checks: build 0 warnings/0 errors (submodules 86d8b22/9544ff2), full Orihon.slnx 460/460 pass (143 BlazorAdapter +3 new, 75 Domain, 89 Integration, 153 UseCases)