refactor(ui): adopt the Kagaku.UI layout vocabulary project-wide #150
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/layout-adopt"
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?
Phase 3 for Orihon: adopt the Kagaku.UI layout vocabulary (Kagaku.UI #23 + #24) project-wide.
Submodule bump
48a5e8b→479100e.The sweep — every layout-only scoped rule in the project (46 measured): 34 migrated across 13 files, −310 CSS / +136 markup. Bible, PageWorkspace (viewport + page), ProjectList/Wizard/Workspace, SetupChat, RulingBar, Settings, ProjectMetadataCard.
Spacing policy: exact token matches migrate 1:1; off-scale gaps snap to the nearest step. This PR snaps 0.4rem → Small (
.pagews__sample,.trow__head) — the only two; every other migrated gap was exact.Classes on migrated elements are gone (maintainer policy — unconditional). Dependents were rewritten, not the classes:
article button, text-matched buttons (Single(b => b.TextContent.Contains("Start the conversation"))),aside select,.pagews__samples > div,[role="alertdialog"] button,.pagews__inkrow input.pagews__panel h2, barep, bare::deep .kg-textarea-style rules.setupchat__answerstays a literal div — its::deep :first-childflex rule can only anchor on the scoped class (the KUI #24 survival rule)The 12 keeps (each reasoned): 3×
justify-content:space-between(noBetweenmember), sizing leftovers (flex:1 1 auto+min-height:0,flex:1.6,min-widthfloors in grid parents), 3×::deepchild overrides,display:contents, one inert-without-::deepflex rule (migrating would newly apply dormant styling).Verification: build 0 errors; BlazorAdapter 263, Integration 266, Domain 128, UseCases 597 — all green.
Same checklist as Kagura #281 (merged) and Tsumugi #22 (open). Kagura's dead-class purge follow-up is in flight.
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.5%
Orihon.Domain - 100%
Orihon.Infrastructure - 97.2%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 94.4%
Orihon.UseCases - 97.6%
🔮 fufu~ Jibril reviewed your code!
Ohhh, a whole-project vocabulary adoption~ Thirty-some flex rules dissolved into
Rows/Columnsand −174 net lines of CSS-adjacent weight gone. I read every hunk of the 1444-line diff, every changed file at head, and traced each migration against the library's ownLayoutCssmaps. The discipline here is delicious — but one little rule slipped through the purge, and you know how I feel about those~ ♡Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
.wizard__body p { margin: 0 }is dead: this same PR deletedwizard__bodyfrom ProjectWizardPage.razor (all three steps now use<Rows Gap="Spacing.Large">), so no element carries the class ever again — yet the rule survived the CSS sweep. Mechanically proven: the rule ships in the compiled scoped bundle (.wizard__body p[b-1hhlphvds4] { margin: 0 }, adapter bundle line 614) but can match nothing — and there is no globalpmargin reset anywhere (kagaku-ui.css resets onlyh1–h4; app.css none). At base those intro paragraphs hadmargin: 0via the.wizard__bodyancestor; at head they fall back to the UA default1emblock margins, which in a flex container don't collapse — they stack on top ofGap=Large(1rem). Net effect: the name/upload/setup intro paragraphs sit2rem off their controls instead of 1rem. A real visual regression, invisible to green tests.fufu~ you wouldn't leave THIS in the bundle, would you? ♡Fix: the sibling pattern from this very PR — ProjectWorkspacePage re-anchored
.workspace__setupchat pto a bare scopedp { margin: 0 }(that component'sp's are all authored by its own markup, so the scoped rule stays precise). Do the same here: replace.wizard__body pwithp { margin: 0 }— it covers the three intro paragraphs; the upload-countpkeeps its own.wizard__step-footer prule (that anchor survives in markup) with the identical value, so no conflict. And per the standing maintainer policy from today's twin PR: a refactor leaving dead selectors behind is a blocker, not a nit💡 Little ideas (non-blocking)~
cut.FindAll("button").Where(b => b.TextContent.Contains("Sample the box")).First(b => b.TextContent.Contains("Sample the box"))— theWhereandFirstpredicates are identical; one of them is pure decoration..First(b => b.TextContent.Contains(...))alone says the same thing.691a176), #22 nav tree (0025d3e), #23 (b4e4548), #24 (479100e). Harmless — I verified all four main-side commits are tree-identical to the approved PR heads (#21 r1, #22 r3, #23 r1, #24 r5), so zero unreviewed library code rides in — but the credit line undersells the bump. (Same nit I left on Tsumugi #22.)[role="alertdialog"] buttonindexed[0]/[1]is order-coupled; a text match like the siblings would survive a button reorder. Fine as-is — the dialog is unique in the page today (verified).✅ What I liked~
30 migrations against♪LayoutCss:.pv__toolsand.pagews__editoractionsboth keep theirjustify-content: flex-endasJustify="Alignment.End";workspace__newchapter'salign-items: end→Align="Alignment.End"; the bare-justifyrows correctly inheritStart. The missing-Justifybug I blocked Tsumugi #22 for did not recur here — someone read the checklist::deepsurvival audit is clean. All nine surviving::deeprules anchor on classes still authored by their component's markup (.wizard__drop,.setupchat__answer,.chapter__upload, …) — the KUI #24 ScopedRuleSurvival trap, dodged completely. And the one documented revert (.setupchat__answerstaying a literal div) is correctly reasoned: its> ::deep :first-childrule can only anchor on the scoped class..pagews__sample,.trow__headat 0.4rem) are exactly the two snaps the body documents.aside selectholds because KUITabsunmounts inactive panels (only one panel's aside exists in the DOM at a time); the structural.pagews__samples > divmatches the surviving wrapper; the alertdialog queries are unique. 263 adapter tests pin the new shape.Element="header"/Element="nav", androle="toolbar"/aria-labelsplat throughColumns' unmatched-attribute capture onto the root (verified in the library source).Automated review by Jibril · 2026-09-24
CI/CD: absent for head
5937f19(PR minutes old, no bot comment) · Local checks: build 0w/0e Release; full suite 1254/1254 (Domain 128 + UseCases 597 + Adapter 263 + Integration 266 — matches the PR body digit-for-digit); scoped-bundle inspection + dead-anchor sweep over all ten changed componentsFixed in
91bfe2b.The dead rule now anchors bare scoped
p— the same sibling pattern this PR already used forworkspace__setupchat: all six paragraphs in ProjectWizardPage are the component's own, so the barep { margin: 0 }hits exactly the ones that lost their margin rule with.wizard__body..wizard__step-footer p(line 37) is untouched and keeps winning the cascade where it applies (later rule, both properties).Also: the two redundant
Whereclauses before an identicalFirstpredicate in PageWorkspacePageTests are collapsed toFirst(predicate).263 green, head
91bfe2b.🔮 fufu~ Jibril reviewed your code!
Back so soon~? One little commit, riding in under the wire — exactly the shape I asked for. Let me unwrap it properly~ ♪
Verdict: ✅ Looks good to me~
The increment (
5937f19→91bfe2b, single commit, +3/−3 across exactly 2 files, zero scope creep):ProjectWizardPage.razor.css:59—.wizard__body p→ bare scopedp { margin: 0 }, the exact sibling pattern from this same PR'sProjectWorkspacePagere-anchor. Verified in the freshly compiled bundle: the dead rule is gone (0 occurrences ofwizard__bodyanywhere — source or bundle) and the live rule ships asp[b-1hhlphvds4] { margin: 0 }at adapter bundle line 614. The three intro paragraphs get their margin back.PageWorkspacePageTests.cs:178, :875— the decorativeWhere( identical-predicate )beforeFirstdropped. Same semantics, same failure mode; both tests green.I also audited the anchor-widening before accepting it — a bare scoped
preaches every authored<p>in the component, so: all four paragraphs in the markup are the wizard's own; no child component roots at<p>(Card → div, InlineAlert → div, SetupChat → Rows/div, PageOrganizer → div — and child-internal<p>s would carry the child's scope attribute anyway); and.wizard__step-footer p[b](0,2,1) keeps beatingp[b](0,1,1) in the cascade, with the identicalmargin: 0value regardless. Airtight~✅ What I liked~
step-footerwinning where it applies) instead of just flipping the selector — that's the reasoning the sibling pattern deserves.On CI (build ✅ / test ❌ at
91bfe2b): I chased the red before blessing anything. The failure isRunMonitorTests.A_burst_of_engine_changes_coalesces_into_one_re_read— Expected 1, Actual 2 — a debounce-coalescing test asserting exactly oneLoadMonitorRunafter a fixedTask.Delay(500). It is untouched by this PR (last modified in #99, its source in #132, zero RunMonitor-adjacent files in the diff), CI was fully green at5937f19with the identical submodule tree, and the failure signature is the classic window-straddle under load. I could not reproduce: 6 local runs at91bfe2b(3× full suite, 3× filtered — the last two mimicking CI's exact concurrent four-DLL invocation) all green, 1254/1254. Verdict: pre-existing timing flake, unrelated to this PR~ The 500ms-fixed-delay shape is the fragile part — if it bites again, the durable fix is subscribing before the burst and asserting on monotonic coalescing rather than a wall-clock quiesce, but that's a follow-up for whoever owns RunMonitor, not a condition for this merge.(One tiny smile: the commit message says "all six paragraphs in the component are its own" — I count four authored
<p>s (three intro + the step-footer one). The claim's direction is what matters and it holds; no action needed~)Round-1 architectural review stands in full — alignments exact,
::deepaudit clean, tokens 1:1, test rewrites sound. With the dead selector exorcised, I'm satisfied~Automated review by Jibril · 2026-09-24
CI/CD: failed at head
91bfe2b— single unrelated pre-existing flake (RunMonitorTests debounce, evidence above; 6/6 local reproductions of CI's exact invocation green) · Local checks: build 0w/0e Release −warnaserror; suite 1254/1254 ×3 (Domain 128 + UseCases 597 + Adapter 263 + Integration 266); RunMonitorTests 15/15 ×3; scoped-bundle dead-rule sweep clean