Skip to content

fix(events): render debug bar restore button outside the container it un-hides (#3345) - #3370

Merged
bpamiri merged 2 commits into
developfrom
fix/bot-3345-debugbar-restore
Aug 5, 2026
Merged

fix(events): render debug bar restore button outside the container it un-hides (#3345)#3370
bpamiri merged 2 commits into
developfrom
fix/bot-3345-debugbar-restore

Conversation

@bpamiri

@bpamiri bpamiri commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

What / Why

The dev-mode debug bar's minimized "Debug" restore button (#wdb-minimized) could never render: it was nested INSIDE the #wheels-debugbar container, and wdbMinimize() (in public/assets/js/debugbar.js) hides that container with display:none. A descendant of a display:none element is never rendered regardless of its own display value, so once a developer clicked the X the bar was gone for the entire browser session (sessionStorage['wdb-hidden'] re-minimizes on every page load; manual sessionStorage cleanup was the only recovery).

Approach

  • vendor/wheels/events/onrequestend/debug.cfm: moved the #wdb-minimized block to AFTER the #wheels-debugbar closing </div>, still inside the cfsavecontent so the whitespace-collapse ReReplace continues to apply. The button is independently position:fixed, so as a sibling it stays visible while the container is hidden.
  • The debugbar.js script include also moved below both elements: its load-time wdbMinimize() re-invocation calls getElementById('wdb-minimized'), which must already exist in the DOM (previously the button preceded the script; keeping that ordering as siblings avoids a load-time TypeError when the minimized state is restored).
  • New structural spec vendor/wheels/tests/specs/events/DebugBarMinimizedButtonSpec.cfc (modeled on debugBarEncodingSpec.cfc): renders debug.cfm and walks the <div/</div balance from the container's opening tag, asserting id="wdb-minimized" appears only after the balance for #wheels-debugbar returns to zero (sibling, not descendant). Written TDD-first: failed on the old markup, passes after the move.
  • Changelog fragment changelog.d/debugbar-restore-button.fixed.md.

Test evidence

  • Failing first (TDD red): events scope ran 65 passed / 1 failed — the new spec failed with the sibling assertion.
  • After the fix: events scope 66 passed / 0 failed / 0 errors.
  • Full core suite (Lucee 7 + SQLite, local): 4759 passed / 0 failed / 0 errors.

Residual scope

Dev-mode UI change — the structural spec is green, but a live browser hand-test (click X, see the Debug button bottom-right, click it to restore, minimized state persists via sessionStorage across navigations) is pending and will be done by the maintainer session before merge.

Fixes #3345

🤖 Generated with Claude Code

… un-hides (#3345)

The minimized 'Debug' restore button (##wdb-minimized) was nested inside
the ##wheels-debugbar container, which wdbMinimize() hides with
display:none — a descendant of a display:none element never renders, so
the button could not appear and the bar stayed hidden for the whole
browser session (sessionStorage re-minimizes on every load).

Move the ##wdb-minimized block after the container's closing div, still
inside the cfsavecontent so the whitespace-collapse ReReplace applies.
It is independently position:fixed, so as a sibling it stays visible
when the container is hidden. The debugbar.js script include moves
below both elements because its load-time wdbMinimize() re-invocation
does getElementById('wdb-minimized') and must find it in the DOM.

New structural spec asserts id="wdb-minimized" appears only after the
div balance for ##wheels-debugbar returns to zero (sibling, not
descendant).

Signed-off-by: Peter Amiri <petera@pai.com>

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer

TL;DR — This PR fixes #3345: the debug bar's minimized "Debug" restore button (#wdb-minimized) was nested inside #wheels-debugbar, which wdbMinimize() hides via display:none, making the restore button permanently unreachable for the browser session. The fix moves the button to be a sibling of the container (still inside cfsavecontent), reorders the script include below both elements, adds a TDD structural spec, and ships a changelog fragment. I verified the fix against the actual JS and the div balance — it is correct and well-tested at the structural level. Verdict: comment (code is clean; the author-acknowledged live browser hand-test is still pending, so this is a non-blocking endorsement rather than an approval).

Correctness

The root-cause analysis holds up. vendor/wheels/public/assets/js/debugbar.js:27-28 sets #wheels-debugbar to display:none and #wdb-minimized to display:block — a descendant of a display:none ancestor never renders regardless of its own display, so the pre-fix nesting made the restore button unreachable. Moving it to a sibling (debug.cfm:492 now closes the container before the button at :496) resolves this; the button is independently position:fixed so its layout does not depend on the container.

Div balance is preserved: the diff adds one </div> at line 492 and removes the trailing one that previously followed the <script>, so the container's open at debug.cfm:87 still closes exactly once. All three references to wdb-minimized (debugbar.js:28, :33, debug.cfm:496) use getElementById — no descendant/child CSS selectors that the re-parenting would break — and the load-time wdbMinimize() at debugbar.js:44 still runs after both elements exist in the DOM (<script> at debug.cfm:503 follows both), so no load-time TypeError.

Cross-engine

No concerns. The new spec uses try/finally with no loops inside finally (invariant 12 N/A), no catch-with-local writes (invariant 11 N/A), standard arrow-function describe/it, and the application.wo.$includeAndReturnOutput($template=...) call carries an argument — the verified-safe boundary of cross-engine invariant 16b, and identical to five other passing specs in vendor/wheels/tests/specs/events/.

Tests

vendor/wheels/tests/specs/events/DebugBarMinimizedButtonSpec.cfc is a well-modeled BDD spec (extends="wheels.WheelsTest") that directly asserts the bug's mechanism: it walks the <div>/</div> balance from the container's opening tag and requires id="wdb-minimized" to appear only after the balance returns to zero (sibling, not descendant). The <div vs </div token search correctly distinguishes the two (<div is not a substring of </div), and the request-state setup / url.format handling is copied faithfully from debugBarEncodingSpec.cfc. PR reports TDD red→green (65/1 → 66/0) plus a full-suite green run.

One residual, author-acknowledged in the PR body: the live browser hand-test (click X → button visible bottom-right → click restores → sessionStorage persistence) is still pending. CLAUDE.md notes green tests do not verify feature-correctness for UI changes; the structural spec covers the actual defect (nesting) well, and the remaining visual behavior is standard unchanged CSS/JS, so this is a reasonable maintainer-before-merge step rather than a gap in this PR.

Docs

Changelog fragment changelog.d/debugbar-restore-button.fixed.md follows the <slug>.<type>.md convention with a valid fixed type — no direct CHANGELOG.md edit. The inline comment at debug.cfm:495 documenting why the sibling placement is deliberate is a nice touch for future maintainers.

Commits

fix(events): render debug bar restore button outside the container it un-hides (#3345) — valid conventional-commit header, 86 chars (≤100), type fix from the allowlist, scope events. Reflects the "why."

Nice, tightly-scoped fix with a correct root-cause diagnosis and a spec that tests the actual mechanism.

As a sibling of ##wheels-debugbar the ##wdb-minimized button no longer sits
under the container's all:initial isolation or its font stack, so the host
app's global CSS bled in (font-family:inherit resolved to the app's body
font — verified live: the demo app's Lato replaced the system stack).
Mirror the container's guard on the sibling: all:initial first, then its
own declarations plus the same font-family. The JS display toggling still
wins because style.display updates in place after the all shorthand.

Live-verified on Lucee 7: minimize/restore cycle, sessionStorage persistence
across reload, button at bottom-right 8px/8px in the system font. Full core
suite 4759 passed / 0 failed / 0 errors.

Signed-off-by: Peter Amiri <petera@pai.com>

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer

TL;DR — Re-review of PR #3370 (fixes #3345) at the new head. The substantive fix (relocating #wdb-minimized to a sibling of #wheels-debugbar so wdbMinimize()'s display:none no longer makes the restore button unreachable) was already verified correct in the prior pass. The only delta since then is one commit adding all:initial plus an explicit system font stack to the button's inline style, isolating it from host-app CSS now that it no longer inherits the container's all:initial guard. That change is correct, minimal, mirrors the container's own pattern, and closes the last residual from the prior review. Verdict: approve.

Correctness

The all:initial addition at vendor/wheels/events/onrequestend/debug.cfm:497 is sound. The container at debug.cfm:87 already opens with style="all:initial;position:fixed;...;font-family:-apple-system,BlinkMacSystemFont,...;"; once the button became a sibling (this PR's core fix) it fell outside that isolation, and the author observed the host app's body font (Lato) bleeding in via the button's child font-family:inherit. The fix applies the identical guard to the sibling — all:initial first, then the same explicit stack — so font-family:inherit on the inner <button> again resolves to the system stack rather than the app's.

Declaration order is correct: all:initial;display:none;position:fixed;bottom:8px;right:8px;z-index:99999;font-family:.... Every functional longhand follows all:initial, so each wins by source order for the same specificity — display:none still hides the button on load, position:fixed/z-index still pin it bottom-right. The JS toggle is unaffected: debugbar.js flips visibility with document.getElementById('wdb-minimized').style.display = 'block' (minimize, :28) and = 'none' (restore, :33), which set the display longhand in place — the all shorthand in the parsed inline block does not interfere, exactly as the commit message states.

Cross-engine

No concerns. The delta is a pure CSS string literal inside the existing cfsavecontent; no CFML code paths, no new # characters requiring ## escaping in the changed style (the font stack contains none), and no new <div> tokens.

Tests

vendor/wheels/tests/specs/events/DebugBarMinimizedButtonSpec.cfc still holds — it asserts id="wdb-minimized" renders after the #wheels-debugbar div-balance returns to zero, and the style change adds no divs. CSS isolation is not structurally testable, and the author reports a live-verified minimize/restore + sessionStorage-persistence cycle on Lucee 7, which resolves the pending hand-test residual noted in the prior review. Full core suite reported 4759 passed / 0 failed / 0 errors.

Commits

fix(events): isolate the relocated restore button from host-page CSS — valid conventional-commit header, fix type, events scope, within length, signed off, and the body explains the "why" (font bleed after re-parenting) rather than just the "what".

Clean, tightly-scoped follow-up that closes the prior review's only open item.

@bpamiri
bpamiri merged commit 9e8a8b6 into develop Aug 5, 2026
10 checks passed
@bpamiri
bpamiri deleted the fix/bot-3345-debugbar-restore branch August 5, 2026 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unhiding debug bar

1 participant