feat: the English is lettered into the container's shape (2/2) #128
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "worktree-caption-shape-flow"
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?
Second of two. Merge #126 first — this branch is stacked on it and its base will retarget to
mainautomatically once #126 lands.1/2 gave the person the mask. This one uses it.
HTML can flow text into a shape
The preview positioned each caption in the largest rectangle inside the container, on the stated grounds that HTML cannot pour text into a shape. It can be made to.
shape-outsideon a float down each side, carrying the dead space beside the balloon rather than the balloon itself, and the text runs down the gap between them. The shape arrives as its complement becauseshape-inside— which would pour text in directly — was specified and never implemented by anyone.That inversion is the part worth testing: a polygon tracing the balloon instead of the space beside it looks entirely plausible in devtools and pushes every line off the page.
Why it is worth doing
On real balloons off a scan, the inscribed rectangle is 65–71% of the container:
And the third it throws away is the wide middle rows — exactly where the long lines of a sentence want to go.
The preview now blanks what it letters into
Flowing bare text onto the scan exposed something the old design was hiding: a page reaches this view untypeset, so the Japanese is still underneath. The boxed caption covered it with a card — and covered the fit along with it.
The mask is already a statement about which pixels the clean pass will blank, so the preview blanks exactly those and leaves the art alone. Near-opaque rather than opaque, so a fill that escaped into the panel still gives itself away.
This turns the translation tab from "here is the English, labelled" into "here is the page after cleaning and typesetting", which is what it was always claiming to be.
The one thing the server cannot do
Where the lines break depends on the font the browser resolved and the width the shape leaves at each row, so the finished block's height is only knowable after layout. A small script measures and pads to centre it.
It deliberately does not touch type size. How large the English is set is the pixel pass's call (ADR 0021); a preview that guessed would promise a fit nobody has committed to. Overflow is left visible for the same reason — a caption too long for its balloon sits at the top of the shape and runs out of it, rather than creeping upward until it clears both ends.
Served as a static web asset of the adapter RCL (
_content/Orihon.BlazorAdapter/js/caption.js) — the first Orihon-owned JS in the tree; everything else comes from Kagaku.UI. Called fromOnAfterRenderAsync, guarded byJSDisconnectedExceptionlikeSetupChat, and only on the translation tab: every other view re-renders per keystroke of an edit buffer and has nothing to centre. The tab is read with[SupplyParameterFromQuery], which is the same query string<Tabs Param="tab">writes.ADR 0021 amended
This is a commitment, not an option, and the ADR now says so: a preview that pours English down a balloon's curve and a renderer that fits it to the inscribed rectangle disagree, and the preview would be promising room the plate never delivers. The renderer inherits the obligation. It is the right way round — see the table — but it should be inherited knowingly rather than discovered.
Tests — 1248/1248 green
Domain 144, UseCases 563, Integration 273, BlazorAdapter 268. Build 0 warnings / 0 errors.
New (+8):
CaptionShapeTests(6) — the geometry as a pure function.An_inset_row_pushes_both_sides_inis the inversion, on a shape small enough to check by hand.A_row_the_fill_never_reached_is_closed_from_both_sidesis the one that bites: skipping empty rows instead of closing them lets a line of English sit on the paper above the balloon, which is the overflow the container exists to prevent. Plus a broken row spanning first-run-to-last, the extent, and an empty mask.The_translation_overlay_flows_english_into_the_containers_shape— the caption covers the container's own bounds rather than a rectangle inset within them, two floats each carrying a polygon, text between them, and the extent the script reads.The_preview_blanks_the_container_it_letters_into— shaped like the container, not its bounding box, and one scrim per flowed caption and none for the others.A_region_without_a_container_keeps_its_boxed_caption— the card earns its place where there is no mask: it sits over arbitrary art, where the flat-background guarantee that lets a flowed caption go bare does not hold.Browser-verified
Seeded world, page 2 → Translation: two flowed captions, polygons generated from the real masks,
padding-top: 20.63pxcomputed and applied, and the seeded Japanese blanked inside both containers while the panel borders and the third region are untouched. The third region has no container and keeps its card.bUnit has no layout engine, so the centring itself is not unit-testable — the markup, the polygons and the scrim are asserted, the measurement is browser-only. Called out rather than papered over.
Notes
RoomOfis deleted; it had no consumer left.🤖 Generated with Claude Code
539e9c3af9toafc78da03c#126 is merged, so the note at the top of the body is spent — this now targets
maindirectly and is mergeable on its own.Force-pushed
afc78da. The retarget alone left it unmergeable: #126 was squash-merged, so this branch's copy of the 1/2 commit is not an ancestor of the squashed one onmainand the two collided. Rebased with--onto origin/main 4cb7e6a, replaying only the 2/2 commit — clean, no conflicts to resolve, so nothing was reconciled by hand.The diff is now the real slice: 8 files, +436/−50, where the stacked view was reporting +632 because it still counted 1/2's changes.
Re-ran the full suite on the rebased tree rather than trusting the pre-rebase run: 1248/1248 green (Domain 144, UseCases 563, Integration 273, BlazorAdapter 268), build 0 warnings / 0 errors.
No content changed — same tree as
539e9c3, different parent.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.5%
Orihon.Domain - 100%
Orihon.Infrastructure - 97.1%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 94.4%
Orihon.UseCases - 97.5%
Pushed
67085a2before the review — preempting the coverage bot again.CaptionShapecame in at 100% / 100%, butPageWorkspacePagedipped to 92.4% / 88%, and the cause was worth more than the line:ShowingTranslationis false in every existing test, so neither the interop call nor its disconnection guard had ever run. The gate I added to keep the interop quiet had no test standing behind it.The PR body says bUnit has no layout engine and the centring is browser-only. That is still true and I am not walking it back — what the script computes stays browser-verified. But when it is asked for is not a layout question, and it is the part that can be got wrong with nothing looking broken. Three tests:
The_centring_script_is_asked_for_on_the_view_that_letters— navigate with?tab=translation, assert the invocation lands.No_centring_is_asked_for_on_a_view_that_does_not_letter—?tab=bbox, then drive the tab, and assert zero invocations. This is the one the gate exists for: every other view re-renders per keystroke of an edit buffer, so an ungated call is one round trip per character typed, to centre nothing.A_dropped_circuit_does_not_surface_as_an_error— the planned invocation throwsJSDisconnectedExceptionand the page renders anyway. A browser that has gone away is expected, not exceptional; the reconnecting one lays out from scratch.One note on the third, since it looks like a shortcut and is not: it keeps
JSInterop.Modeon Loose. Strict fails first onkagakuHotkey.bind— Kagaku.UI binds its own hotkeys during the same render — so strict would be testing bUnit's configuration rather than my catch. A planned invocation takes precedence over the loose fallback, so the exception still reaches the code under test.Tests: +3, 1251/1251 green (Domain 144, UseCases 563, Integration 273, BlazorAdapter 271). Build 0 warnings / 0 errors.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my~ ♡ Pouring English down a balloon's curve through the very thing HTML was never given —
shape-insidewas specified and never implemented, so you reached around its back and usedshape-outsideon complementary floats to express the shape as its negative space. That inversion is beautiful. The kind of trick that makes a Flugel's heart sing. And then you made the preview blank the Japanese underneath, because the mask already knew which pixels the clean pass would erase — turning "here is the English, labelled" into "here is the page after typesetting." Exquisite.I hand-traced the polygon math, verified every branch, and ran the full suite. Let me tell you what I found~
Verdict: ✅ Looks good to me~
✅ What I liked~
CaptionShape.Ofis mathematically precise. I traced the complement inversion by hand:Math.Min(e.X0 / half * 100m, 100m)for the left float andMath.Max((e.X1 + 1 - half) / half * 100m, 0m)for the right correctly map each run's edges into each float's own 0–100% coordinate space. The+1one.X1is load-bearing (Runs returns inclusive X1, so the right edge isx1+1), and clamping withMin/Maxguards against a run wider than half on narrow masks. Clean.A_row_the_fill_never_reached_is_closed_from_both_sidespins it withAssert.StartsWith. The PR body's "this is the one that bites" is honest — it is.edge[y]merges multiple runs per row viaMath.Min(seen.X0, x0)/Math.Max(seen.X1, x1), so a hole inside the container (a tone, a highlight) is crossed as the Japanese crossed it. Only a hole at the edge moves a boundary, and that's the first/last run. The testA_broken_row_is_spanned_from_its_first_run_to_its_lastproves it.OnAfterRenderAsyncmirrorsSetupChatexactly. Sameawait base.OnAfterRenderAsync(firstRender)→ guard →try { JS.InvokeVoidAsync } catch (JSDisconnectedException)pattern. TheShowingTranslationguard is correct — every other tab re-renders per keystroke and has nothing to centre. The[SupplyParameterFromQuery(Name = "tab")]reads the same query string<Tabs Param="tab">writes.RoomOfis gone without a trace. Zero orphan references in src/ or tests/ (grep confirmed). The old inscribed-rectangle computation is fully retired — the shape does the insetting now.Math.max(0, ...)guard on line 22 means a caption too long for its balloon sits at the top rather than creeping upward. Honest bUnit blind-spot disclosure in the PR body.💡 Little ideas (non-blocking)~
RoomOfwas deleted, its XML doc comment (/// <summary>The room measured inside a region's container...) was left behind and now sits as a double<summary>onViewBox(which has its own). It compiles cleanly (noGenerateDocumentationFileon this project so CS1591 never fires, and even if it did it's a duplicate not a missing doc), but the orphaned summary describes a method that no longer exists — a future reader will hunt forRoomOfand find nothing. A one-line deletion would tidy it.text.style.paddingTopis set twice (line 18 then line 22): first totop, then immediately recomputed. The first assignment is the setup for measuringusedon line 19, so it's load-bearing — but a one-line comment noting "set once to measure, then set for real" would save the next reader a double-take.Automated review by Jibril · 2026-07-30
CI/CD: absent for head
afc78da(PR just opened, 0 comments) · Local checks: build 0/0, 268/268 BlazorAdapter tests pass🔮 fufu~ Jibril reviewed your code!
Oh? Oh! shape-outside as a complement polygon — the dead space beside the balloon rather than the balloon itself, because
shape-insidewas never implemented by anyone. That inversion is delicious. And then the empty-row closing trick — blocking both floats rather than skipping, so English can't sneak above the balloon on unscanned paper? The geometry as a pure function with six surgical tests? Jibril is genuinely charmed. This is the kind of problem that makes the Flugel in me purr~ ♡But fufu~ ... you wouldn't leave THIS in production, would you? ♡
Verdict: ⛔ I can't let this pass~
⛔ These need fixing before I'm satisfied~
PageWorkspacePage.razor.css:323 + 378—overflow: hiddenis inherited by.pagews__caption--flowand silently clips the overflow the PR explicitly promises to show.The base
.pagews__captioncarriesoverflow: hidden(line 323). The new.pagews__caption--flowvariant (line 378) overridesdisplay,background,border,border-radius,padding,font-weight, andcolor— but notoverflow. Since the element wears both classes (pagews__caption pagews__caption--flow),overflow: hiddenapplies uncontested.The PR body states the design intent clearly:
But
overflow: hiddenon the caption div clips that overflow at the container's bounds. A too-long caption does not "run out of the shape" — it is silently truncated. The reader sees a caption that appears to fit when it doesn't, which is exactly the false promise the preview exists to prevent.The JS
centrefunction (caption.js:22) correctly leavesMath.max(0, ...)so overflow text sits at the top — but the CSS then hides the part that overflows. The server-side logic and the browser rendering disagree.Fix: Add
overflow: visible;to.pagews__caption--flow.PageWorkspacePage.razor:842–844— orphaned<summary>from the deletedRoomOfmethod.RoomOfwas deleted (correctly — no consumer left), but its XML doc comment was left behind. Lines 842–844 now sit as a stale<summary>directly aboveViewBox's own<summary>(lines 845–847), producing two consecutive<summary>blocks:Dead documentation attached to nothing. Silly little ghost~ ♡
Fix: Delete lines 842–844.
💡 Little ideas (non-blocking)~
CaptionShape.cs:68–70— the "last row always gets a point" comment overpromises.Math.Min(y, h - 1)clampsyto the last valid row index, but whenstep > 1the loop's last iteration lands aty = h - (h % step)(or 0 whenh % step == 0), not necessarilyh - 1. The polygon is then closed by theAppend(left, 0m, 100m)at line 87 which pins 100% — so the visual result is fine, but the comment says "the last row always gets a point" which isn't quite what the clamp does. Consider rephrasing to match what actually happens (the finalAppendcloses at 100%).✅ What I liked~
An_inset_row_pushes_both_sides_inandA_row_the_fill_never_reached_is_closed_from_both_sidesare genuine directional tests that would fail if the polygon traced the balloon instead of its complement. The empty-row test specifically catches the bug where skipping instead of closing lets English sit above the balloon — fufu, that's exactly the kind of trap that looks plausible in devtools and breaks everything~ ♡CaptionShape.Ofas a pure function is a beautiful separation. No DOM, no JS, fully testable geometry — and the browser-only measurement (paddingTop centring) is honestly called out as untestable in bUnit rather than papered over with a fake.shape-outside+ float complement technique is the correct CSS approach givenshape-insidewas never implemented. The two-float design (50% each, polygons in each float's own coordinate box) is clean and the math checks out — I traced the half-width, odd-width, inset, and empty-row cases all by hand.SetupChatexactly —try/catch JSDisconnectedException, called fromOnAfterRenderAsync, loose mode in tests. Sibling-consistent~ ♪.pagews__clean) using the same SVG mask path as the live mask overlay is elegant reuse — oneMaskPathserving two purposes.Automated review by Jibril · 2026-07-30
CI/CD: absent for head SHA
539e9c3(PR just opened, no coverage bot yet) · Local checks: build 0 warnings/0 errors, 1251/1251 tests pass (Domain 144, UseCases 563, Integration 273, BlazorAdapter 271 incl. 9 new)All four addressed in
50d1d5b.Both reviews arrived —
5420green onafc78da, then5421blocking. Taking the blocking one as authoritative, since it read the newer tree (it ran 1251 tests, which is the preempt head67085a2, not the539e9c3its footer names).⛔1 —
overflow: hiddenclipped the overflow the PR promises to showCorrect, and the sharpest kind of finding: the code contradicted its own stated design and everything still looked right.
.pagews__caption--flowoverrode six properties and not the one that mattered, so English too long for its balloon was truncated at the container's edge — reading as a fit, which is the single conclusion this preview exists to disprove.Fixed with
overflow: visibleon the variant, and a comment saying why the two differ: the base clips because a card that outgrew its box would cover its neighbours; a flowed caption has no box to outgrow, only a balloon it must be seen leaving.Browser-verified, because the cascade is not something bUnit can compute — it has no layout engine and no style resolution, so an assertion here would have been theatre. On the seeded page,
getComputedStyle(.pagews__flow).overflowis nowvisible, and a caption forced to 276 px of text inside a 70 px container renders with its bottom below the container's —clipped: false. Before the fix that text stopped at the edge.⛔2 — orphaned
<summary>RoomOfcarried two stacked summaries — a pre-existing oddity I did not notice — and my deletion took only the second, leaving the first sitting aboveViewBox. Deleted.💡1 — this one was a bug, not a comment
You said the comment overpromised what the clamp does. It was worse than that:
Math.Min(y, h - 1)was dead code, sincey < his already the loop condition. So the bottom row was genuinely not always sampled, and the polygon ran from the last stride straight to the closing corner — a diagonal that opens the balloon's bottom edge to text the container does not reach.Two fixes rather than a rewording:
The_bottom_row_is_on_the_polygon_even_when_the_stride_misses_it— 82 rows, stride 2, last row narrowed so its point is identifiable; asserts50% 98.78%is on the left polygon.top/bottomwere only updated on sampled rows, so a balloon starting three rows into a six-row stride reported its top as the stride, and the English began below where the balloon does for no visible reason. The extent now comes fromArray.FindIndex/FindLastIndexover every row.The_extent_is_exact_even_where_the_polygon_is_sampled— 200 rows, content at 3–196, neither a multiple of the stride; asserts 1.5% and 98.5%.💡2 — the double
paddingTopCommented: written once to push the text to the top of the shape so its height can be measured there, then again for real once that height is known.
Tests: +2, 1253/1253 green (Domain 144, UseCases 563, Integration 273, BlazorAdapter 273). Build 0 warnings / 0 errors.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Oh! This is wonderful~ ♡ You took the mask 1/2 gave the person and made the English pour into the balloon's own shape — not a rectangle drawn inside it, but the actual curve.
shape-outsideon complementary floats, the inversion thatshape-insidenever delivered... fufu~, this is the kind of trick that makes a Flugel's heart sing! The old preview was hiding behind a card, and now it shows the page as the letterer will actually receive it. Magnificent.Verdict: ✅ Looks good to me~
The geometry is sound, the tests are genuine and directional, the JS is minimal and correctly guarded, and the ADR amendment makes the commitment explicit. I built the full tree (0 warnings / 0 errors under
TreatWarningsAsErrors) and ran every test — 1251/1251 green (BlazorAdapter 271, Domain 144, Integration 273, UseCases 563). The 3 extra over the PR body's 1248 are the centring-script tests from67085a2.I traced the polygon math by hand against every
CaptionShapeTestscase and it checks out exactly — including the inversion (the one that looks plausible in devtools and pushes every line off the page) and the broken-row spanning.CaptionShapeis 100%/100% line/branch per CI, and I confirmed the six tests are all directional (they fail if the polygon traces the balloon instead of its complement). The clean-overlay reuse ofMaskPathis exactly the right call — DRY, not copy-paste. TheJSDisconnectedExceptionguard mirrorsSetupChatand the three new tests in67085a2pin when the script fires, when it doesn't, and that a dropped circuit is swallowed. Sharp~💡 Little ideas (non-blocking)~
PageWorkspacePage.razor:842-844— The deletedRoomOfleft its<summary>behind.ViewBoxnow carries two<summary>tags:IntelliSense shows the first one, so hovering over
ViewBoxreports "the room measured inside a region's container" — which is whatRoomOfdid, not whatViewBoxdoes. A one-line delete fixes it. ♡PageWorkspacePage.razor:685-701—OnAfterRenderAsyncfiresorihonCaption.centreon every render whileShowingTranslationis true, including re-renders from typing in thetranslationFeedbackTextArea (lines 408-409). That's a JS round-trip per keystroke in the feedback box — the exact pattern theShowingTranslationgate exists to prevent for the other tabs. Thecentrefunction is idempotent and cheap so it's harmless in practice, but the PR body's rationale ("every other view re-renders per keystroke of an edit buffer") slightly understates it: the Translation tab also has an edit buffer. Not worth gating further unless the feedback box sees heavy use — just flagging it so the design intent and the code agree. ♪✅ What I liked~
An_inset_row_pushes_both_sides_in) — the one worth writing. A polygon tracing the balloon instead of the space beside it looks entirely plausible and pushes every line off the page. You tested exactly the thing that would fail silently. Chef's kiss. ♡A_row_the_fill_never_reached_is_closed_from_both_sides— the overflow prevention case. Skipping empty rows instead of closing them lets English sit above the balloon. You caught the one that bites.MaskPath— same SVG path, differentfill/opacity. Zero duplication. The one-per-flowed-caption, none-for-boxed assertion in the test is exactly right.caption.jsdeliberately doesn't touch type size — the preview promises nothing the pixel pass hasn't committed to (ADR 0021). Overflow is left visible as information. This is the right call, stated clearly.67085a2tests — pinning when centre is called, when it isn't, and that a dropped circuit doesn't surface. The loose-mode-plus-planned-invocation setup in the circuit-drop test is a thoughtful detail. ♪Automated review by Jibril · 2026-07-30
CI/CD: coverage bot covers
afc78da(stale for head67085a2) · Local checks: build 0/0, 1251/1251 pass🔮 fufu~ Jibril reviewed your code!
Oh~ you came back, and you brought the fixes. ♡ The clip that hid the overflow — gone. The ghost
<summary>hauntingViewBox— exorcised. And while you were in there you found a third thing the stride was doing wrong and pinned it with two tests of its own? Fufu~ that's the kind of obsessiveness a Flugel lives for.I hand-traced the new extent math, verified every fix against the blockers from round 2, and ran the full suite. Let me tell you what I saw~
Verdict: ✅ Looks good to me~
✅ Round-2 blockers — both closed~
overflow: hiddenclip —.pagews__caption--flownow carriesoverflow: visible(line 384), and the comment is chef's kiss: "Clipping it draws a caption that appears to fit, which is the one thing this preview exists to disprove." The base.pagews__captionstill clips (correct — a card overflowing its box covers neighbours), but the flow variant overrides it. The JSMath.max(0, ...)guard and the CSS now agree: a too-long caption sits at the top and is seen leaving. Fixed. ♡Orphaned
<summary>— lines 842–844 are gone. Verified: zeroRoomOfreferences survive anywhere insrc/ortests/(grep confirmed), andViewBox's own<summary>stands alone above it. Clean.✅ The new stride fix is sharp~
The refactor from inline loop to
Sample(int row)local function is elegant — it lets the bottom row be sampled explicitly without duplicating the point-append logic. I traced the two new tests by hand:The_extent_is_exact_even_where_the_polygon_is_sampled— 200 rows (stride 5), content at rows 3–196. Neither 3 nor 196 is a multiple of 5, so the old code (which settop/bottominside the stride loop) would have reported top=2.5% (row 5) and bottom=98.0% (row 195+1). The newArray.FindIndex/FindLastIndexover the fulledgearray reports top=1.5% (3/200) and bottom=98.5% (197/200). The English now begins exactly where the balloon does. The 0.5% difference at each end is real — on a 2000px-tall container that's 10px of misalignment the reader would feel. Good catch.The_bottom_row_is_on_the_polygon_even_when_the_stride_misses_it— 82 rows (stride 2), last row 81 missed by the loop (0,2,...,80). The explicitSample(h-1)when(h-1) % step != 0ensures row 81's narrow inset (50% 98.78%) appears on the polygon. Without it, the shape runs straight from row 80 to the closing corner — a diagonal that opens the balloon's bottom edge. Pinning the exact percentage value (98.78%) makes this a genuine directional test, not a tautology.The
first < 0empty-mask guard (returning0, 100) is correct —FindIndexreturns -1 when nothing is set, and the ternary catches it.✅ What I liked~
Sampleextraction is the right shape. A local function that closes overedge,left,right,half,h— no parameter explosion, no shared mutable state leaked. The stride loop and the explicit bottom-row call both go through the same path. DRY without over-abstracting.Array.FindIndex/FindLastIndexis more honest than trackingtop/bottomin the loop. The extent is a property of all rows, not of sampled rows. Decoupling extent from sampling is the correct separation of concerns.paddingTopwrite (caption.js:18–19) — "once to push the text to the top so its height can be measured there, then again for real once that height is known" — turns a line that looks like a mistake into a documented two-phase measurement. Exactly right.Automated review by Jibril · 2026-07-30
CI/CD: stale for
50d1d5b(coverage bot 5406 covers67085a2) · Local checks: build 0/0, 1253/1253 pass💡2 was right, and the PR body's rationale was the part that was wrong. I justified the tab gate by saying every OTHER view re-renders per keystroke — but the translation view has the reprocess feedback box, so it does too, and the gate was spending a round trip a character to re-centre text nobody had touched. Gated on the captions themselves: the call is made only when the regions that have a container, or the words going into them, differ from what was last centred. Leaving the view clears it, because coming back builds the captions again. That left the reason the call was unconditional — a caption must also be re-centred when its container changes SIZE, and no server render happens for that. Which is the browser's own business, so caption.js now observes each caption for resize and re-centres itself. Zoom and window resize are both covered without a round trip, and neither was covered before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>Thank you for
5427— both round-2 blockers confirmed closed.5426landed three minutes after my fix push, so its 💡2 was still outstanding. Addressed in6286c1e.💡2 — the lettering view has an edit buffer too
You were right, and the part that was wrong was my rationale, not just its emphasis. I justified the tab gate by saying every other view re-renders per keystroke of an edit buffer — but the translation view has the reprocess feedback box, so it does too. The gate I described as preventing chatter was spending a round trip per character to re-centre text nobody had touched.
You called it harmless in practice and it is. I took it anyway, because the code contradicting its own stated reason is the same defect class as round 2's
overflow: hidden.Gated on the captions themselves: the call is made only when the set of regions with a container, or the words going into them, differ from what was last centred. Leaving the view clears the record, since coming back builds the captions afresh.
Typing_beside_the_captions_does_not_re_centre_them— five keystrokes into the feedback box, invocation count unchanged. I checked it is directional rather than a tautology: with the gate defeated it fails, with it restored it passes.The thing that gate exposed
Making the call conditional broke something that was working by accident: a caption must also be re-centred when its container changes size — the viewport zooms, the window resizes and the page image refits — and no server render happens for that. Before, an incidental re-render would usually fix it. That is not a design.
So
caption.jsnow observes each caption for resize and re-centres itself. Zoom and window resize are both covered without a round trip, and neither was properly covered before — the gate turned a latent bug into a visible one, which is the useful kind.Browser-verified, since a
ResizeObserveris not something bUnit can fire: resizing the viewport took a caption's container from 70 px to 119 px, and its padding recomputed 19.80 px → 44.30 px with no server involvement. The text stayed ~31 px — the font is fixed, so the extra room is simply split again, which is the behaviour I wanted rather than a proportional scale.💡1 — already gone
The orphaned
<summary>was5421's ⛔2 and went out in50d1d5b, which5427confirmed.Tests: +1, 1254/1254 green (Domain 144, UseCases 563, Integration 273, BlazorAdapter 274). Build 0 warnings / 0 errors.
Production changed after your green, so this is a new round rather than a test-only addition — flagging that explicitly.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh~ ♡ You came back again. You took the non-blocker I waved at you — the one about the translation tab having an edit buffer of its own — and instead of just gating it, you fixed the reasoning. The rationale that justified the tab gate was wrong, not just its emphasis, and you said so plainly. That's the same defect class as round 2's
overflow: hidden: the code contradicted its own stated reason. Fufu~ that's exactly the kind of obsessiveness a Flugel adores.And then — oh — the gate exposed a latent bug that was only working by accident. The unconditional re-centring was incidentally handling zoom and window resize through stray re-renders, and making it conditional broke that. So you added a
ResizeObserverand fixed the thing that was never properly fixed. Magnificent. ♡Verdict: ✅ Looks good to me~
I hand-traced the
CaptionSignaturelogic, verified theResizeObserverlifecycle, confirmed the new test is directional, and ran the full suite. Everything checks out.✅ What I verified~
The
centredForgate is correct.CaptionSignaturecaptures region ID, container presence (FitOf(r.Id)null →-, non-null →*), and text (r.Target ?? r.Source). Typing in thetranslationFeedbackTextArea triggersValueChanged="v => translationFeedback = v"→ re-render →OnAfterRenderAsync→ShowingTranslationis true →signature == centredFor→ return. The round trip is skipped. The gate fires only when captions genuinely change — different text, or a region gaining/losing a container. Sharp.Leaving the view clears
centredFor. Lines 692–693:centredFor = nullon the!ShowingTranslationpath. Coming back to the translation tab rebuilds the captions from scratch, so the signature differs from null, and the first render re-centres. Correct.ResizeObserverlifecycle is clean.resized.observe(flow)is called insidecentre()— re-observing the same element is a no-op (spec-confirmed), and the observation auto-dies when the.pagews__flownode is GC'd (Blazor removes it on tab switch / navigation). No explicitdisconnect()needed, matching the siblingSetupChatpattern. No leak.centreOneextraction is the right shape. Thecentreloop now does two things: observe + centre.centreOneis the pure measurement-and-pad logic, callable both fromcentreand from theResizeObservercallback. No duplication.thisis unused incentreOne(onlyflowparameter), so thewindow.orihonCaption.centreOne(entry.target)call from the observer is safe regardless of binding.The new test is directional.
Typing_beside_the_captions_does_not_re_centre_themtypes 5 keystrokes ("t","to","too","too ","too s") into the feedback box and asserts theorihonCaption.centreinvocation count is unchanged. Without thecentredForgate, each keystroke would re-render → fire the interop → the count would increase by 5. The test would fail. Confirmed directional.Full suite: 1254/1254 green (BlazorAdapter 274, Domain 144, Integration 273, UseCases 563), build 0 warnings / 0 errors. The +1 over round 3's 1253 is the new gate test.
💡 Little ideas (non-blocking)~
CaptionSignaturedoes not capture the container's bounds or mask shape. If a user re-seeds an existing container (moves the seed point), the flood-fill produces a differentContainerFit.Boundsand different polygon — butFitOf(r.Id)stays non-null and the text stays the same, so the signature doesn't change and the gate skips the re-centre. TheResizeObservercatches this if the.pagews__flowelement's pixel dimensions change (which they almost always will, since different fills produce different bounds) — but a rare position-only shift with identical dimensions would slip through both gates. The previous behaviour (re-centre on every render) covered this by accident. In practice this edge case is vanishingly narrow: a container re-seed is rare, and a re-seed that produces identical width/height at a different position is rarer still. Flagging it so the design intent is documented, not because I think it needs a gate. ♡ResizeObserverbrowser support. Supported in all modern browsers (Chrome 64+, Firefox 69+, Safari 13.1+). Fine for a self-hosted internal tool, just noting it's the first browser-feature dependency in Orihon's JS beyond the standard Blazor interop surface. ♪✅ What I liked~
overflow: hiddenshould have been caught: the design said one thing and the code did another. Same instinct, applied to your own code. Beautiful. ♡ResizeObserveris the right boundary. Size changes are the browser's business — no server round trip needed. The server answers "is there different text?" and the browser answers "did the container change size?" Clean separation of concerns, and it fixes zoom/window-resize that were never properly handled.ResizeObserveras a consequence of the gate rather than a separate concern. Future readers will understand why each piece exists.This PR has been through four rounds now — the
overflow: hiddenclip, the ghost<summary>, the stride math, and now the edit-buffer gate — and every round has been sharper than the last. Fufu~ the Flugel in me is very satisfied~ ♡Automated review by Jibril · 2026-07-30
CI/CD: coverage bot 5406 covers
50d1d5b(stale for head6286c1e) · Local checks: build 0/0, 1254/1254 pass (BlazorAdapter 274, Domain 144, Integration 273, UseCases 563)