Theme: implement Bootstrap 5 (14 components, both template sets) - #29
Conversation
All 14 components in both template sets, replacing the PLANNED.md stubs.
`bootstrap` is now accepted by CF_UI_THEME and install_cf_ui(theme=...);
_CDN_CSS, _DEFAULTS and assets.jinja already carried it at 5.3.3.
The theme loads no Bootstrap JavaScript, and that is the whole design.
Bootstrap's data-bs-* API is a second state owner for the modal, the tabs
and the accordion, and cf_ui_alpine.js is already the first. Loading both
would make Alpine.store('cf').modal.open(id) behave differently under this
theme than under every other one -- the cross-theme guarantee the theme
work exists to protect. So the templates take Bootstrap's classes and
markup structure and wire state through the existing Alpine components.
Two places where "use Bootstrap's classes" is not enough on its own, both
because the missing piece was in the bundle we are not loading:
* `.modal` is display:none and `.show` only sets opacity -- Bootstrap's
JS is what writes style.display = "block". The reveal rides on
`d-block`, a real utility Alpine can toggle, and the E2E tier asserts
visibility rather than the class list so a reveal that changes classes
and nothing else cannot pass. The backdrop is the same story: Bootstrap
appends one to <body> at z-index 1050, under the modal's 1055, and we
have no JS to do that. It lives inside the modal at a negative z-index
instead, which keeps it under the dialog rather than eating every
click; both halves are covered by a test.
* The panel body deliberately carries no `.collapse`. That class is
display:none without `.show`, which would hide a server-open panel
permanently once Alpine is off -- exactly the bug #21 fixed. x-show
owns display, seeded from data-cf-open. The navbar does use
`.collapse`, where the pure-CSS `.collapse:not(.show)` /
`.navbar-expand-lg .navbar-collapse` pair is correct with or without
JS.
Accessibility parity is enforced rather than asserted in prose:
tests/unit/test_accessibility.py now parametrizes over three themes, so
dialog semantics, the label fallback, the tabs' server-rendered active
state with roving tabindex, and the panel's aria-expanded/aria-controls
toggle all have to hold here too.
"bootstrap" is added to THEMES last, so a half-finished theme was never
selectable at any commit in this branch.
Closes #22
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
) Issue #22 asks to "confirm assets.jinja has the Bootstrap CDN entry to match _CDN_CSS". Confirming it by reading both files is exactly the kind of check that rots: `_CDN_CSS` serves Django and the assets.jinja macro serves Jinja2 apps, holding the same URLs in two places, and a theme wired into one and not the other renders unstyled on half the supported frameworks with no error at all. So the agreement is now asserted, parametrized over all five theme names including the two that are still stubs, plus a guard that every entry in THEMES has both a CDN URL and a pinned version -- a theme selectable at startup but unstyled at render is worse than one rejected at startup. Verified non-vacuous: bumping the bootstrap version in assets.jinja alone turns the new parity test red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adversarial self-reviewI went looking for places this diff is wrong rather than places it is right, and tried to settle each suspicion by running something rather than by re-reading the template. Below is the one real gap I found (now closed in c972a2b), the things that turned out to be fine, and the evidence for each. The three Bootstrap-specific decisions, and why I distrusted themThis theme is unusual in the repo because it deliberately omits the framework's own JavaScript. That makes three places where Bootstrap's CSS expects its JS to have written something load-bearing and effectively invisible to diff review. I verified all three against the actual Modal reveal uses The backdrop lives inside the modal at The panel body carries no Mutation testing: nine deliberate breakages, nine redsA green suite proves nothing unless removing the behaviour turns it red. Each mutation was reverted immediately after.
A gap the unit suite structurally cannot seeUnit tests render these templates through plain Jinja2, where The one thing that was actually wrongThe issue asked me to "confirm Things I checked and deliberately left alone
Environment caveat, unrelated to this diffEarly in this run |
…trap-theme # Conflicts: # CHANGELOG.md
) Two things #29 needed before merge. #33 — the CSS-only, Alpine-driven stance was decided during #22 and recorded nowhere a consumer or maintainer would look. `docs/bootstrap.md` states it, gives the three reasons in the order they mattered, and answers the question that was actually unanswered: what to do about a Bootstrap component cf-ui does not ship. Bootstrap 5.3.3 ships 12 JS-driven components (`base-component.js` is the shared base, not a component — the earlier "13" in #33's body counted it). cf-ui replaces four of those plugins across five of its own components, so eight are uncovered. The page names them, gives three ways forward, and states the one hard rule: never put a `data-bs-*` attribute on a cf-ui component, because Bootstrap's JS and `cf_ui_alpine.js` would then own the same state and the failure is load-order dependent and intermittent. The behaviour-driver abstraction is recorded as considered-and-deferred rather than left unmentioned, so it is not relitigated from scratch. `tests/unit/test_bootstrap_version_pin.py` turns "monitor for Bootstrap 6" into something mechanical. `_DEFAULTS["bootstrap"]` is the single place the major version is stated; the test fails the moment it leaves the `5.x` line and its failure message is the checklist of what to re-evaluate. Same pattern #17 established for Tailwind. Verified non-vacuous: bumping the pin to 6.0.0 fails with that checklist, and the pin was restored. What it points at, read off `v6-dev` directly rather than from release notes — several secondary sources claim v6 already adopted native `<dialog>` citing twbs#41751, which is closed and was never merged: `_modal.scss` is replaced by `_dialog.scss` with no `.modal*` selector left, `modal.js` by `dialog.ts` on `HTMLDialogElement.showModal()`, and the JS surface is growing. cf-ui's bootstrap modal templates carry eight `modal-*` references each, so the markup breaks at v6 on the CSS alone — which is why deferring the JS question costs nothing. #32 — the four Alpine bindings on each tab now read `$el.dataset.cfTab` instead of an interpolated `'{{ tab.id }}'`. Applied here rather than waiting, so this theme does not land with a known injection and get patched twice; the tree-wide guard arrives with #32 itself and will hold whichever of the two merges second. 743 unit + integration pass, 129 E2E pass, 65 node pass, ruff and prek clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NhqNRBg83czKfr8L6FF5xf
|
Patched to get this over the line — #33 — #33 — What it points at, read off #32 — the Alpine injection fix, applied here too. The four bindings on each tab read Merge order does not matter. #35 fixes bulma + daisy and adds the guard; this PR fixes bootstrap. Whichever lands second merges
|
…on-injection CHANGELOG only. Kept both sides, and folded master's bootstrap-only #32 note into this branch's full #32 section rather than leaving two entries for the same fix — #29 carried the fix in on its own branch, so the section now names all the themes it covers. The tree-wide guard from this branch now also scans master's bootstrap templates. They pass: #29 landed with the fixed pattern, so the guard finds nothing, which is the merge order working as intended. 872 unit + integration pass, ruff clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NhqNRBg83czKfr8L6FF5xf
Foundation (#23) landed on master, which moved the tree under this branch. Only CHANGELOG.md conflicted, and only as an additive union: both sides appended a new section to the top of [Unreleased]. Both kept, newest-first per the file's existing order — Security (#32) above Foundation (#23). The branch's deletion of the one-bullet "Fixed — tab ids no longer reach Alpine as expression source (#32)" section that Bootstrap (#29) carried in merged cleanly and stands: the full Security section supersedes it. Foundation's equivalent note stays a bullet inside its own theme section, where it describes what that theme shipped rather than duplicating a heading. One line went stale in the merge and was corrected: the Security section said "Foundation and Fomantic follow the same way" about themes carrying the fix in on their own branches. Foundation has now landed with it, so the count and tense move — eight sites each in bootstrap and foundation, branches #29 and #30, with only Fomantic still to come. No source or test file needed an edit. Both guards derive their theme list from cf_ui.themes.THEMES, so tests/unit/test_alpine_expression_safety.py and tests/e2e/test_alpine_injection.py picked Foundation up on the merge and pass against it — which is the whole point of writing them tree-wide rather than per-theme. Gates: 1066 unit+integration passed (903 on master before the merge), 182 E2E passed / 26 skipped, 65 node --test, ruff check and ruff format clean over src and tests, prek clean on two consecutive runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NhqNRBg83czKfr8L6FF5xf
1 participant
Implements the Bootstrap 5 theme: 14 components in both template sets, replacing the
PLANNED.mdstubs.Closes #22
Scope checklist
templates/jinja/bootstrap/— 14 JinjaX components (*.jinja) — Breadcrumb, Card, CheckboxGroup, FormField, Modal, Navbar, Notification, Pagination, Panel, Progress, Select, Table, Tabs, Textarea. All render under plain Jinja2 withStrictUndefined, so every optional prop carries itsis definedguard.templates/cotton/_themes/bootstrap/— 14 cotton partials (*.html) — no<c-vars>anywhere; the prop contract stays on the untouched wrappers incotton/cf/. BothPLANNED.mdstubs are deleted, including the legacycotton/bootstrap/directory that predates the_themes/dispatch from DaisyUI theme — implement component set #6."bootstrap"toTHEMESinthemes.py— last — it is the final source change in the branch; the theme was unreachable, and rejected loudly at startup, until every template existed.assets.jinjahas the Bootstrap CDN entry to match_CDN_CSS— verified, not assumed: both carrybootstrapat5.3.3and both point atbootstrap@{v}/dist/css/bootstrap.min.css._DEFAULTSlikewise. No change needed. Steps 4, 5 and 6 of CLAUDE.md's Adding a New Theme were already done (thebootstrap = []extras entry exists too).tests/unit/jinja/andtests/unit/cotton/—test_bootstrap.pyin each, mirroring the shape of the daisy suites.Verification checklist
just checkgreen —ruff check src tests,ruff format --check src tests, andpytest tests/unit tests/integration(732 passed).node --test65 passed.prek run --all-filesrun twice, clean both times with no rewrites.pytest tests/unit/test_theme_dispatch.py -vcovers bootstrap —IMPLEMENTED_THEMESnow has three entries, so every parametrized case picks it up automatically.tests/e2e/test_bootstrap.py, 48 passed / 6 skipped, on new fixtures at ports 8775 (JinjaX) and 8776 (cotton). Fullpytest tests/e2ealso green: 129 passed, 19 skipped.tests/unit/test_accessibility.pynow parametrizes over three themes instead of two, so every case in that module had to pass against Bootstrap markup unchanged.The architectural constraint: CSS only, Alpine drives behaviour
No
bootstrap.bundle.js, nodata-bs-*. Bootstrap's own API is a second state owner for the modal, the tabs and the accordion, andcf_ui_alpine.jsis already the first — loading both would makeAlpine.store('cf').modal.open(id)mean something different under this theme than under every other one. This is asserted rather than left to prose: every component is checked fordata-bs-in both the rendered output and the source, and the E2E suite reads backscript[src]on the gallery page to prove no Bootstrap script is loaded at all.Two places where "use Bootstrap's classes" was not sufficient on its own, both because the missing piece lived in the bundle we are not loading:
The modal reveal.
.modalisdisplay: noneand Bootstrap's.showonly sets opacity — its JS is what writesstyle.display = "block". A theme that toggled just.showwould change the class list and never become visible. The reveal rides ond-block, a real Bootstrap utility, and the E2E test assertsto_be_visible()rather than a class so that failure mode cannot pass.The modal backdrop. Bootstrap appends one to
<body>at z-index 1050, below the modal's 1055. cf-ui has no JS to do that, so the backdrop has to live inside the modal — where the same 1050 would paint it over the dialog, since.modalopens a stacking context, and it would swallow every click. A negative z-index puts it back underneath. Because that is the least obvious decision in the diff, it has its own E2E test asserting both halves: a control inside the dialog is still clickable, and a backdrop click still closes.The panel body carries no
.collapse. That class isdisplay: nonewithout.show, which would hide a server-open panel permanently once Alpine is off — exactly the bug #21 fixed.x-showowns display, seeded fromdata-cf-open. The navbar does use.collapse, where the pure-CSS.collapse:not(.show)/.navbar-expand-lg .navbar-collapsepair is correct with or without JS; the E2E test asserts the computeddisplayrather than the class list, following the lesson from the DaisyUI navbar fix.Every class name was confirmed against the actual Bootstrap 5.3.3 stylesheet from the CDN — not from memory — including the
.modal,.modal-backdrop,.collapse:not(.show),.navbar-expand-lg .navbar-collapse,.invalid-feedbackand.is-invalid ~ .invalid-feedbackrules that drove the decisions above.Two smaller notes worth flagging
invalid-feedback d-block. Bootstrap's.invalid-feedbackisdisplay: noneuntil an.is-invalidsibling precedes it. That works for the input in FormField/Select/Textarea, but a checkbox group has no such sibling, so the message would render invisibly. Every error slot in this theme carriesd-blockfor consistency.Progress is a div, not a
<progress>. Bootstrap sizes the bar in percent, so the width has to be computed fromvalue/maxrather than emitted verbatim —{% widthratio %}on the Django side, a filter chain on the Jinja side. A bar hard-coded tovalue%would be wrong for anymax != 100, so there is a test formax=4and one formax=0.One shared test I had to change, and why
test_switching_the_setting_switches_the_rendered_markupasserts that each theme renders differently. Bootstrap and DaisyUI genuinely spell a plain non-dismissible info alert the same way —alert alert-info— so the notification case collided. Rather than weaken the assertion to something pairwise-optional, the props bag now passesdismissible="true". That restates a<c-vars>default whichrender_to_stringdoes not apply, so it makes the fixture more representative of a real render, and the themes diverge where they actually differ (btn-closeversus DaisyUI'sbtn btn-sm btn-circle btn-ghost). The comment in the test records the reason.Two tests in
test_theme_dispatch.pyusedbootstrapas their example of a rejected stub theme; they now usefoundationandfomantic, which are still stubs.Merge conflict note
As the issue predicted, this touches exactly one shared source line —
THEMESinthemes.py. It also touches three shared test surfaces the Foundation and Fomantic phases will want:IMPLEMENTED_THEMESintest_theme_dispatch.py,THEMES+ACTIVE_TAB_CLASSintest_accessibility.py, and the_THEME_CSS/_THEME_EXTRA_HEADmaps plus E2E fixtures. All are one-line-per-theme additions.tests/unit/test_tailwind_content.pyneeded no change: it is scoped to an explicitDAISY_TEMPLATESlist rather than parametrized overTHEMES, so Bootstrap — which ships prebuilt CSS and has no tree-shaking hazard — is correctly out of its scope, and daisy's assertions are untouched.Follow-ups deliberately not folded in
docs/bootstrap.md. DaisyUI needed one because of the Tailwind content-glob trap; Bootstrap has no build step, so the CSS-only constraint is documented in the README themes section and the CHANGELOG instead. A dedicated page can follow if the theme grows options.aria-labeland:aria-expandedbut noaria-controls, matching the Bulma and DaisyUI navbars. Adding it properly needs anidprop on the wrapper's<c-vars>, which is a public prop change and out of scope here.