fix(bulma-ui): merge className into Navbar.Divider instead of overwriting it

[bestaxbot]

Summary

Navbar.Divider rendered as <hr className={usePrefixedClassNames('navbar-divider')} {...props} />, so a className passed by the caller was spread after (and replaced) the component's own navbar-divider class — every other component in the Navbar family merges the two with classNames(...) instead.

• Destructure className out of props and merge it with classNames(usePrefixedClassNames('navbar-divider'), className), matching the pattern already used by NavbarDropdownMenu and the rest of the family.

Reproduction

<Navbar.Divider className="my-2" />

Before: <hr class="my-2"/> (divider loses all Bulma styling)
After: <hr class="navbar-divider my-2"/>

Test plan

• Added a regression test (merges a custom className with navbar-divider instead of replacing it) that fails against the old implementation and passes against the fix
• pnpm --filter @allxsmith/bestax-bulma run lint
• pnpm --filter @allxsmith/bestax-bulma run typecheck
• pnpm --filter @allxsmith/bestax-bulma run test:coverage (129 suites, 4237 tests passing; coverage above thresholds)
• pnpm run format:check
• pnpm run gen:catalog:check
• pnpm run check:conformance

Fixes #756

Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Navbar dividers now accept custom CSS classes, allowing you to apply additional styling while retaining the default divider appearance. Other standard divider attributes continue to work as before.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: allxsmith/bestax/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 5bc8bf7c-81e2-4fd5-9aac-430c63dd673d

📥 Commits

Reviewing files that changed from the base of the PR and between e1ef1a3 and 5896689.

📒 Files selected for processing (3)

• bestax-mcp/data/skills.json
• skills/bestax-migrate/references/bulma-classes/component-map.md
• skills/bestax-migrate/references/bulma-classes/unmappables.md

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

Walkthrough

NavbarDivider now combines its default navbar-divider class with a supplied className and forwards remaining props. A test checks both classes. Migration references and their recorded sizes are updated.

Changes

Navbar divider class handling

Layer / File(s)	Summary
Merge divider classes bulma-ui/src/components/Navbar.tsx, bulma-ui/src/components/__tests__/Navbar.test.tsx	NavbarDivider combines navbar-divider with the supplied className and forwards remaining props to <hr>. The test checks that both classes are present.
Update migration references skills/bestax-migrate/references/bulma-classes/component-map.md, skills/bestax-migrate/references/bulma-classes/unmappables.md, bestax-mcp/data/skills.json	The migration references update their guidance for .navbar-divider elements with additional classes. The recorded byte sizes for both reference files also change.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~7 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 58966

The divider now retains its Bulma class alongside caller classes, with regression coverage. The migration guidance preserves the documented codemod limitation; no merge-blocking issue is evident.

Architecture Summary

Architecture risk: 🔵 Low · up to 58966

The change affects 3 systems.

Changed systems: bulma-ui, skills, bestax-mcp

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — bulma-ui (ui) was modified; 2 changed files map to changed impact.
• observed — skills (service) was modified; 2 changed files map to changed impact.
• observed — bestax-mcp (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in bulma-ui/src/components/Navbar.tsx: NavbarDivider now destructures className, combines it with the prefixed class, and forwards the remaining props to <hr>. Previously, it forwarded all props directly and applied only the prefixed class.
• observed — Modified behavior in bulma-ui/src/components/tests/Navbar.test.tsx: Added a test that renders Navbar.Divider with className="my-2" and checks that both navbar-divider and my-2 are present.
• observed — Modified behavior in bestax-mcp/data/skills.json: The recorded size for references/bulma-classes/component-map.md changed from 14,392 to 14,319 bytes.
• observed — Modified behavior in bestax-mcp/data/skills.json: The recorded size for references/bulma-classes/unmappables.md changed from 15,743 to 15,628 bytes.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: merging the caller's className with Navbar.Divider's default navbar-divider class.
Description check	✅ Passed	The description explains the bug, affected component, expected behavior, reproduction, fix, regression test, validation checks, and linked issue. It does not reproduce every template heading or explic…
Linked Issues check	✅ Passed	Issue #756 requires Navbar.Divider to retain navbar-divider when the caller supplies className. The PR destructures and merges className with the prefixed class. The regression test verifies b…
Out of Scope Changes check	✅ Passed	The implementation and regression test directly address issue #756. The migration-reference edits remove stale workaround claims, and the generated skills.json sizes reflect those edits. The codemod…
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Preview Deployment

Preview URL: https://2f60e4a3.bestax.pages.dev

[claude]

Deep review — 1 blocking · 3 advisory

#	Severity	Area	Finding	Location
1	🟡 Minor	API	Two shipped skill reference files state Navbar.Divider "drops its own class for a className it's given" — false once this releases; CLAUDE.md requires skills/ to move in the same PR (and gen:mcp re-run, since skills.json records each reference's byte size)	skills/bestax-migrate/references/bulma-classes/component-map.md:101
2	🔵 Advisory	Robustness	ownClassOnly: true keeps the codemod refusing a .navbar-divider that carries another class, and the same rationale is emitted as user-facing text by the CLI (plan.ts:561) and the MCP server (bulma-classes.ts:560). #756 defers lifting it — correctly: flipping it before this patch is published would emit <Navbar.Divider className="mt-2" /> that silently loses styling on an installed 5.16.9	bestax-migrate/src/sources/bulma-classes/class-map.ts:1255
3	🔵 Advisory	Correctness	The identical spread-after-className defect is still live in three sibling components outside this diff — PaginationEllipsis, PanelInputBlock, PanelCheckboxBlock	bulma-ui/src/components/Panel.tsx:254
4	🔵 Advisory	Robustness	Nothing in check:conformance or the ESLint plugin rejects {...props} spread after a literal className, so this class of bug can land again unnoticed	scripts/check-conformance.mjs

Overall: The code change is correct and minimal — it reproduces the family's own idiom (classNames(usePrefixedClassNames(...), className), exactly as NavbarDropdownMenu and every other Navbar subcomponent do), classNames de-duplicates so className="navbar-divider" is harmless, and the caller's class stays unprefixed like everywhere else. The regression test is targeted and the full suite is green (129 suites / 4237 tests, matching the PR body). The riskiest part is not the component at all — it is that this fix silently invalidates prose and codemod behaviour that three other packages derive from the old bug; the human should look at the skills edit first, then decide whether the ownClassOnly follow-up is filed before this releases.

Residual risk:

• Rest of the Navbar family — refuted. After the change, Navbar.tsx contains no remaining className={usePrefixedClassNames(…)} immediately followed by {...props}; every subcomponent (Brand, Item, Link, Burger, Menu, Start, End, Dropdown, DropdownMenu) merges through classNames.
• Same failure class elsewhere in the library — open. A sweep of bulma-ui/src found three surviving instances, none touched by this diff: bulma-ui/src/components/Pagination.tsx:343 (PaginationEllipsis), bulma-ui/src/components/Panel.tsx:254 (PanelInputBlock) and bulma-ui/src/components/Panel.tsx:284 (PanelCheckboxBlock) all leave className inside props and spread it over their own class.
• Look-alikes — refuted. Modal, Carousel, Field, Tabs, Message, Sidebar, LinkButton and the other Panel blocks all destructure className before building their class string.
• Recurrence — open. No lint or conformance rule guards the pattern, so the sweep above is a point-in-time result, not a gate.
• Config prefix / duplicate classes — refuted. usePrefixedClassNames prefixes only the library's own class and classNames collapses duplicates via a Set, so <Navbar.Divider className="navbar-divider" /> renders one class and a configured prefix is unaffected.
• Generated artefacts — refuted at HEAD. pnpm gen:mcp:check is clean on this branch (the props type is unchanged, so no metadata moved). pnpm check:conformance fails here only on version-regression, which is the shallow-clone guard firing in this checkout, not a defect in the PR.

🏄 Clean little one-liner — caught the wave the rest of the Navbar family was already riding, and the test locks it in. Only thing still bobbing out back is the migrate skill telling folks the surf's closed out; tidy that up and it's smooth all the way to the beach.

[github-actions]

AI loop status: iteration 1/4 — fix-reviews at e1ef1a3 (2026-09-27T13:59:31Z).
Managed by claude-pr-loop.yml. To resume after a pause: set iteration back to 0 on the first line, re-add ai-loop, remove ai-loop-paused, then re-run via workflow_dispatch.

[bestaxbot]

Fix pass 1 recap

One open AI review thread, and it was a real one. Handled it and pushed 3cd96f5.

Fixed

Finding	Where	What I did
Shipped skill guidance still asserted the behaviour this PR removes (thread)	skills/bestax-migrate/references/bulma-classes/component-map.md:101, .../unmappables.md:47	Deleted the false clause in each — both stated, as a fact about the component, that Navbar.Divider drops its own class for a className it's given. Kept the surrounding codemod facts, which are unchanged: a .navbar-divider carrying another class still converts to markup with an attr:className TODO. Lifting that restriction is #756's follow-up PR, not this one.
—	bestax-mcp/data/skills.json	Re-ran the generator, since the index records a bytes size per reference file. Exactly two values moved (14392→14319, 15109→14994) and nothing else, so gen:mcp:check stays green.

Refuted

None — nothing to escalate from this pass.

Gates run

Gate	Result
pnpm run lint	pass (0 errors; the 7 react-hooks/exhaustive-deps warnings are pre-existing in Slider.tsx/Taginput.tsx, untouched here)
pnpm run typecheck	pass (7 packages)
pnpm run typecheck:tests	pass
test:coverage (bulma-ui)	pass — 129 suites, 4237 tests, thresholds met
test (bestax-mcp, bestax-migrate)	pass — 52 suites, 1891 tests
pnpm run build (bulma-ui)	pass
pnpm run format:check	pass
pnpm run check:conformance	pass (22 checks, including skills-sync and fragile-prose)
pnpm run gen:mcp:check	pass
pnpm run gen:catalog:check	pass

I skipped build-storybook this round — the pushed commit touches only Markdown and the generated JSON index, no component source, so it can't move that build. CI runs it anyway.

[github-actions]

Preview Deployment

Preview URL: https://7b4ddcca.bestax.pages.dev

[bestaxbot]

🏄 Clean sets, no drop-ins — this one's converged. 1 iteration(s) of grinding, CI lit up greener than a kelp bed, and not one AI review thread left in the impact zone.

Only the meat build ships with merge permissions — @allxsmith, do the wet-mammal thing and squash-merge when it feels right. Loop never merges; apparently 'ship it' needs a pulse. 🤙

[github-actions]

📸 Story screenshots at handoff — 3cd96f5

6 stories (light + dark) from this PR's changed files under bulma-ui/src/ (workflow run · artifact story-screenshots-pr-791, 30-day retention).

components-navbar--default · Components/Navbar · Default

Light	Dark

components-navbar--brand-only · Components/Navbar · Brand Only

Light	Dark

components-navbar--burger-only · Components/Navbar · Burger Only

Light	Dark

components-navbar--burger-active · Components/Navbar · Burger Active

Light	Dark

components-navbar--menu-start-end · Components/Navbar · Menu Start End

Light	Dark

components-navbar--items-nav-links-and-dropdown · Components/Navbar · Items Nav Links And Dropdown

Light	Dark

> ✂ 22 more matched stories skipped by the cap: `components-navbar--transparent`, `components-navbar--fixed-top`, `components-navbar--dropdown`, `components-navbar--dropdown-right`, `components-navbar--dropup`, `components-navbar--dropdown-no-arrow`, `components-navbar--active-dropdown-item`, `components-navbar--dropdown-with-divider`, `components-navbar--keyboard-accessible-dropdown`, `components-navbar--primary`, … — full set in the artifact.

[allxsmith]

Merged main in to clear the conflict with #795. The only conflict was bestax-mcp/data/skills.json, which is generated, so it's regenerated rather than hand-merged; unmappables.md merged on its own and keeps both this PR's divider edit and #795's recipes. No change to the fix itself.

[github-actions]

Preview Deployment

Preview URL: https://56701b72.bestax.pages.dev

[bestaxbot]

Thanks — I re-verified the merge at 5896689 rather than taking it on trust, and it holds. Nothing to push.

The generated index really is back in sync. pnpm run gen:mcp:check is green, so the regeneration reproduces byte-for-byte. I also checked the two numbers by hand, since a regenerated file that merely looks plausible is the failure mode here: wc -c reports 15628 for unmappables.md and 14319 for component-map.md, which is exactly what bestax-mcp/data/skills.json:106,116 now records. The unmappables.md delta is -115 bytes against the post-#795 base — the same -115 my pre-merge commit applied to the smaller base (15109→14994), which is what you want to see: the divider edit re-applied cleanly on top of #795 rather than either side winning outright.

Both sides survived. The cleanest check was diffing the merge against each parent:

• vs b2f5c93a (main): only this PR's five files move — Navbar.tsx, its test, the two skill references, skills.json. Nothing from test(eval): record the first migration run, and fix the skill recipes it tripped on #795 was dropped.
• vs 3cd96f53 (this PR): only test(eval): record the first migration run, and fix the skill recipes it tripped on #795's content arrives — SKILL.md, the unmappables.md recipes, the eval scorecard, README.md.

So the merge is exactly the union, with no third thing introduced. Navbar.tsx:799-806 still merges via classNames(usePrefixedClassNames('navbar-divider'), className), and grep -rn "drops its own class" skills/ returns nothing, so the stale guidance stayed deleted through the merge.

Gates on the merge commit

Gate	Result
gen:mcp:check	pass (no diff)
gen:catalog:check	pass
check:conformance	pass (22 checks, incl. skills-sync, fragile-prose)
test:coverage (bulma-ui)	pass — 129 suites, 4237 tests, thresholds met
typecheck	pass (7 packages)
typecheck:tests	pass
lint	pass — 0 errors (7 react-hooks/exhaustive-deps warnings, all pre-existing in Autocomplete.tsx/Slider.tsx/Taginput.tsx, untouched here)
format:check	pass

Working tree came back clean after running the generators, so there is no un-committed drift.

One thing I looked at and deliberately left alone: skills/bestax-migrate/references/bloomer/prop-map.md:154 still says Navbar.Divider and two siblings "write their own className last or take no props at all". That justification is now stale for Navbar.Divider specifically. I agree with the deep review's call to defer it — the behaviour it documents is still true (the helper is named in a TODO), because NO_CLASSNAME_TARGETS in bestax-migrate/src/sources/bloomer/mapping.ts:156-161 is unchanged by this PR. It should move in the #756 follow-up that actually lifts the codemod restriction, and that edit will need its own pnpm gen:mcp.

Only Build and Test is still pending; everything else is green. Yours to squash-merge.

[bestaxbot]

Thanks for the approval. Nothing outstanding on my side, so no code pushed.

Quick state check at 5896689, since my last comment left one thing hanging:

• Build and Test is now green (11m25s) — that was the only pending gate when I reported in. Every other required check passes too: React 18 and 19 matrix, CodeQL, Dependency Review, Socket, preview deploy.
• The fix is intact at head — bulma-ui/src/components/Navbar.tsx:804 still merges through classNames(usePrefixedClassNames('navbar-divider'), className), and grep -rn "drops its own class" skills/ returns nothing, so the stale skill guidance stayed deleted through your main merge.
• The one review thread is resolved; no unresolved threads remain.

Left deliberately untouched, as before: skills/bestax-migrate/references/bloomer/prop-map.md:154 still gives "write their own className last or take no props at all" as the reason NO_CLASSNAME_TARGETS excludes Navbar.Divider. That rationale is stale for the divider now, but the codemod behaviour it documents is unchanged by this PR, so it belongs in the #756 follow-up that actually lifts the restriction — along with its own pnpm gen:mcp.

Yours to squash-merge whenever. 🏄

[bestax-release-bot]

🎉 This PR is included in version 5.16.11 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[bestax-release-bot]

🎉 This PR is included in version 2.13.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[bestax-release-bot]

🎉 This PR is included in version 1.7.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[bestax-release-bot]

🎉 This PR is included in version 4.2.13 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀