feat: the bible's five sections each get a tab of their own #25
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/bible-tabs"
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?
What
The bible was one long scroll — five cards stacked, the section you wanted always somewhere below the fold. Each section now lives behind its own icon tab, using the same Kagaku.UI
Tabscomponent settings and the page workspace already use:menu_bookgroupmaptimelinenotesParam="tab"makes every section deep-linkable (?tab=summaries), matching the settings page.SaveIndicatorstay above the tab bar, so save state is visible from any section.Tests
Sections mount one at a time now, so the all-sections test walks the tabs, the summary tests open their tab first, and a new test pins the deep link. All 73 BlazorAdapter tests pass.
Verified in the browser against the seeded dev world — tab switching, the
?tab=URL, and the missing-summary badges all behave.Was stacked on #24 (shell width); rebased onto main after it merged.
🤖 Generated with Claude Code
95dfe89c40to7b9aad6e397b9aad6e39to78a9428f3eSummary
Summary
Coverage
Orihon.BlazorAdapter - 81.9%
Orihon.Domain - 100%
Orihon.Infrastructure - 93.7%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 93.3%
Orihon.UseCases - 96.2%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh~ A tab bar for the bible! Five sections, each with their own little icon home, deep-linkable like the settings page... fufu~ this is exactly the kind of tidy knowledge-architecture that makes my heart flutter ♡ Let me look closer~
Verdict: ✅ Looks good to me~
This is a textbook surgical UX refactor. +48/−6 across 2 files, one clear purpose, and it follows the established sibling pattern exactly. I checked every seam~
✅ What I liked~
<Tabs AriaLabel="Bible sections" Param="tab">mirrors SettingsPage's<Tabs AriaLabel="Settings" Param="tab">attribute-for-attribute. Every<Tab>carries the sameTitle+Id+Icontriple. No drift, no copy-paste mutation. The deep-link contract (Slug(tab) = tab.Id) means?tab=summariesresolves to the exact tab the PR body promises — I traced it throughTabs.razor'sActivegetter and the slug matchesId="summaries"cleanly.Each_section_lives_behind_its_own_tabopens withAssert.DoesNotContain("ヒロイン", cut.Markup)— that's the load-bearing assertion. It proves the Characters panel isn't in the DOM until its tab is clicked. Then walking through Lore → Story beats → Page summaries, eachAssert.Containsfires only after the click. That's how you test a tab component that only mounts the active panel (the@if (ReferenceEquals(tab, Active))contract inTabs.razor). ♪NavigateTo($"projects/{project.Id}/bible?tab=summaries")before render, thenAssert.Contains("Context.")+Assert.DoesNotContain("先輩")— proves the URL parameter drives the initial tab AND that the non-active panel stays unmounted. Matches the SettingsPage sibling test shape.Typing_a_summary_into_the_gap_creates_it,Blanking_a_summary_deletes_it_making_the_page_blocked_again) got theOpenTab(cut, "Page summaries")prepend they need — without it they'd break, because the summary textarea no longer mounts by default. The author caught every test that depended on the old all-panels-rendered shape. Nothing fell through the cracks.menu_book,group,map,timeline,notes— all present inIconCatalog.csat the pinned86d8b22. No ghost-icon risk (the exact bug Kagaku.UI#1 was merged to fix).Icon.razorwould render an empty<svg>for a missing name, but I verified each one has a real<path>. ♡SaveIndicatorsit in the<header>above<Tabs>, exactly as the PR body claims — so auto-save feedback is visible from any section. TheConfirmDialogalso stays page-level (outside the tabs), correct for a modal.<Card Title="Glossary">inside<Tab Title="Glossary">was redundant — the tab IS the label now. (Contrast with SettingsPage, where<Tab Title="Provider">+<Card Title="OpenRouter">carry different information, so the card title stays. Here they were identical, so dropping is the right call.)73/73 BlazorAdapter tests pass locally (8.8s), matching the PR body exactly. Build: 0 warnings, 0 errors.
No blockers, no suggestions — this one's clean~ fufu ♡
Automated review by Jibril · 2025-07-25
CI/CD: absent for head
95dfe89(PR just opened, no bot comment) · Local checks: build 0/0, 73/73 BlazorAdapter tests pass🔮 fufu~ Jibril reviewed your code!
Oh? Oh! tabs! The bible was one long scroll of five cards piled on top of each other, and now each section gets its own little home behind an icon~ How wonderful! ♡ I was so ready to be possessive about this one, but you've made the beds before I even arrived.
Verdict: ✅ Looks good to me~
I crawled through every line of this diff and ran the suite myself, and I couldn't find a single thing that would make me hold the knife up. Let me show you what I checked~
🔍 What I verified obsessively
menu_book,group,map,timeline,notes— I grepped each one againstIconCatalog.csat the pinned submodule tip (86d8b22), all five present. No ghost glyphs hiding in this PR~ ♪IconCatalogCompletenessTests.The_scan_actually_sees_the_source_it_claims_to_guardnow listsmenu_bookamong its well-known names, so the moment a future catalog rename orphans one of these tabs, the test fails loudly. Self-guarding guard. chef's kissParam="tab"is the sibling contract. Byte-for-byte the same deep-link shapeSettingsPage.razor:31uses (?tab=…, replace-not-push, slug falls back to title).PageWorkspacePagedeliberately omitsParambecause its tabs are transient view modes — you correctly chose the deep-linkable variant for content the user would actually share. Good judgment~Card Title="…"is safe.Card.Titleisstring?with@if (Title is not null)around the header — passing nothing simply suppresses the<h3>. The tab label is the label now, no duplication. The PR body says exactly this and the code honours it.Tabs.razoronly renders@if (ReferenceEquals(tab, Active))— andEach_section_lives_behind_its_own_tabproves it directionally:Assert.DoesNotContain("ヒロイン", cut.Markup)on first render, thenOpenTab(cut, "Characters")flips it to Contains. That's a genuine mount/unmount assertion, not a tautology. ♡A_section_tab_is_deep_linkabledoesNavigateTo($"…/bible?tab=summaries")before render, then asserts the summaries panel mounts and the glossary panel doesn't. That pins both theQueryValuereader and theSlug(t.Id)matcher — the two new code paths this PR opts into.78a9428(I ran them locally — CI bot hasn't posted for this head yet). Build is clean: 0 warnings, 0 errors.💡 Little ideas (non-blocking)~
BiblePage.razor:48-50— the<Tabs>/<Tab>/<Card>trio is flat-indented at 4 spaces, whileSettingsPage.razor:31-33nests each one level deeper (8/12/16 spaces). Razor doesn't care, but the next reader scanning for "what's inside what" will. A quick re-indent to match the sibling rhythm would be lovely — purely cosmetic, no behaviour. ♡That's it. That's the whole list. Fufu~ I'm almost disappointed there's nothing to sharpen my claws on~
✅ What I liked~
Shows_every_table_and_flags_the_missing_summary→Each_section_lives_behind_its_own_taband walking the tabs with a realOpenTabhelper isn't a cosmetic rename — it pins the new mount-one-at-a-time invariant. The two summary tests correctly grewOpenTab(cut, "Page summaries")because they'd otherwise poke at an unmounted panel. That's the work, done properly.Paramwiring without being asked.Cardtitle drops are the consequence of the tab providing the label (not drive-by cleanup), and the stacked-on-#24 note in the PR body is upfront about the rebase plan.Mmm~ clean, well-tested, architecturally consistent. This is how you do a UI refactor, scarlet~ ♡
Automated review by Jibril · 2026-07-25
CI/CD: absent for head SHA
78a9428(no coverage bot comment yet) · Local checks: build 0 warnings/0 errors, 73/73 BlazorAdapter tests pass (incl. 7 BiblePageTests, 2/2 IconCatalogCompletenessTests)🔮 fufu~ Jibril reviewed your code!
Oh? Oh my~ ♡ The bible was one long scroll, was it? Five cards stacked like a loaf of bread — and now each slice has its own little tab home, complete with a deep-linkable address? That's elegant, scarlet. Knowledge organized is knowledge treasured~ ♪
Verdict: ✅ Looks good to me~
I dug in with my claws and found nothing to block on. Let me show you what I checked~
The pattern match is letter-perfect. I compared your
Tabs/Tabusage against the two siblings:SettingsPage.razor:31—<Tabs AriaLabel="Settings" Param="tab">+<Tab Title="..." Id="..." Icon="...">PageWorkspacePage.razor:52—<Tabs AriaLabel="Page views">(noParam) +<Tab Title="..." Icon="...">(noId)You correctly chose the SettingsPage shape (with
Param="tab"for deep-linking +Idon each tab), not the workspace shape. The fiveIdvalues (glossary,characters,lore,beats,summaries) are all URL-safe lowercase — and they're better than the slug fallbacks (story-beats,page-summaries), so setting them explicitly was the right call even thoughTabs.Slug()would have computed something. fufu~ you even knew that, didn't you? ♡The ghost-icon tripwire passes. All five icons (
menu_book,group,map,timeline,notes) are catalogued inKagaku.UI/Icons/IconCatalog.cs. Orihon's ownIconCatalogCompletenessTeststripwire — the one ported from PR #1 — would catch a missing one and render invisible clickable air. Clean.Cards drop titles correctly. I read
Card.razor: whenTitleis null andHeaderContentis null, the entire.kg-card__headerblock is omitted (@if (Title is not null || HeaderContent is not null)). So droppingTitle="Glossary"etc. doesn't leave an empty header bar — the tab label genuinely becomes the only label. No visual artifact.Inactive tabs genuinely don't mount. The
Tabscomponent renders only@if (ReferenceEquals(tab, Active))panels. YourEach_section_lives_behind_its_own_tabtest proves this withAssert.DoesNotContain("ヒロイン", cut.Markup)on initial render — the Characters content isn't in the DOM until you click. That's the load-bearing assertion, and it's the right shape.Deep-link works.
A_section_tab_is_deep_linkablenavigates to?tab=summariesbefore render, then asserts the summaries content shows and the glossary doesn't. I tracedTabs.Active:Paramis set →QueryValue("tab")returns"summaries"→ matches theId="summaries"tab → only that panel renders. Correct.✅ What I liked~
OpenTabhelper is chef's kiss DRY — one little static method and three existing tests get a one-line prefix instead of copy-pasted tab-walking boilerplate.cut.FindAll("[role=tab]").First(b => b.TextContent.Contains(title)).Click()— clean and readable.Shows_every_table_and_flags_the_missing_summary→Each_section_lives_behind_its_own_tabhonestly reflects the new behavior. No misleading names.BiblePage.razor.cssconflict was the right call — and the CSS already carriesflex: 1 1 autofrom that PR, so the bible fills viewport height from any tab.// Glossary is the first tab; only the active panel mounts.teaches the reader why theDoesNotContainassertion matters. Good pedagogy~Verification: Build 0 warnings/0 errors (submodules at
86d8b22/9544ff2). BlazorAdapter suite 73/73 pass — ran it 10 times consecutively after an initial cold-start flake (parallel test discovery timing on the very first run; not reproducible). BiblePageTests 7/7 in isolation and in-class. CI absent for head7b9aad6(no coverage bot comment yet) — local verification used.fufu~ ♡ Approved. The bible breathes properly now — each section in its own room, but all reachable from a single shared URL. Wonderful~
Automated review by Jibril · 2026-07-25
CI/CD: absent for head SHA · Local checks: build 0/0, 73/73 BlazorAdapter tests pass (10 consecutive runs)