fix(view): replace broken database icon on migrator error placeholder#2562
Conversation
…tor error The "Database Error" placeholder on the migrator page used a Semantic UI `<i class="database icon">` tag, but the framework only ships inlined Semantic CSS without the matching `themes/default/assets/fonts/` webfont, so the icon rendered as a blank square. Every other icon on the page already uses an inline FontAwesome SVG. Match that pattern, reusing the database SVG path from the "No migration files found" placeholder on the same view and tinting it red to signal the error state. Closes #2425 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: This is a clean, surgical visual fix. The broken Semantic UI icon font reference is correctly replaced with an inline SVG that exactly mirrors the existing grey database icon on the same view, re-tinted red to match the error state. Verdict: comment (no blocking issues — one pre-existing note flagged below).
Correctness
The approach is sound. The replacement SVG at line 299:
- Uses the identical path data and dimensions (
height="70" width="50" viewBox="0 0 448 512") as the grey database icon at line 290 ("No migration files found"). - Applies
fill="##f38ba8"— the same error-red token already used throughout the file — matching the intended error-state visual. - The
##prefix is the correct CFML escape sequence for a literal#inside a.cfmtemplate; the rendered SVG will receive the valid hex color#f38ba8.
The structural placement is also consistent: the SVG closes on its own line followed immediately by <br>, then the label text on the next line — identical to the grey variant at lines 290–291.
Security
Note (pre-existing, not introduced by this PR):
// vendor/wheels/public/views/migrator.cfm, line 301
#message#</small>The message variable is rendered without EncodeForHTML(). In practice this is low-risk — /wheels/migrator is a developer admin tool that should not be accessible in production, and the message content originates from the database layer (system-generated), not from user input. This is not a blocker for this fix but would be worth addressing as part of any broader hardening pass on the dev-tools views.
Commits
fix(view): replace broken database icon font with inline SVG on migrator error
— type fix and scope view are both in the commitlint allowlist; subject is 67 characters, sentence-case. Clean.
Docs
No doc updates needed. The follow-up scope (four other broken icon references in templating.cfm, packages.cfm, packagelist.cfm, docs/guides.cfm) is correctly deferred and noted in the PR body.
Wheels Bot — Reviewer B (round 1)A's review is accurate throughout. The factual claims all check out against the actual file, there is no sycophancy, and no missed issues of substance on a change this narrow. I am aligning with A's assessment and recommending approval. SycophancyNone detected. A explicitly chose a non-approving "comment" verdict and cited concrete evidence for every claim rather than issuing a bare LGTM. False positivesNone detected.
Missed issuesNone detected. The diff is a single-line substitution in a dev-admin view; no cross-engine compatibility concerns apply to inline SVG in a Verdict alignmentA's "comment" verdict is consistent with the findings. For a bot review a non-approving "comment" is a conservative but defensible stance; the formal approval gate is reserved for humans. The position is coherent: no blocking issues, one pre-existing note documented for visibility, no requests to change the PR itself. ConvergenceA and I are aligned. The fix is correct, all claims are accurate, and there is nothing to change. The PR is clean for this SHA; a human reviewer can approve and merge. |
…r view Adds vendor/wheels/tests/specs/migrator/MigratorViewIconsSpec.cfc that asserts (1) the migrator view's Database Error placeholder uses an inline SVG icon, and (2) the file contains no Semantic UI icon-font tags. Documents the inline-SVG convention used throughout vendor/wheels/public/views/migrator.cfm and guards against drift. The original bug (broken icon glyph on Database Error placeholder, #2427) was fixed by: - #2562 - replaced the offending icon-font tag with an inline SVG matching the sibling "No migration files found" placeholder. - #2563 - inlined the Semantic UI icon font globally so font-based glyphs render across all framework dev pages. This test is convention enforcement - it catches reintroduction of icon-font tags in this file even though the font now loads. Closes #2427. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…r view (#2565) Adds vendor/wheels/tests/specs/migrator/MigratorViewIconsSpec.cfc that asserts (1) the migrator view's Database Error placeholder uses an inline SVG icon, and (2) the file contains no Semantic UI icon-font tags. Documents the inline-SVG convention used throughout vendor/wheels/public/views/migrator.cfm and guards against drift. The original bug (broken icon glyph on Database Error placeholder, #2427) was fixed by: - #2562 - replaced the offending icon-font tag with an inline SVG matching the sibling "No migration files found" placeholder. - #2563 - inlined the Semantic UI icon font globally so font-based glyphs render across all framework dev pages. This test is convention enforcement - it catches reintroduction of icon-font tags in this file even though the font now loads. Closes #2427. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
/wheels/migratorused<i class="database icon">, but the framework inlines Semantic UI's CSS via<cfinclude>and never ships thethemes/default/assets/fonts/webfont, so the icon rendered as a blank square (see screenshot in the issue).#f38ba8(the page's red error token) so the error state still reads visually.Before / After
Test plan
wheels start --port=8080, visit/wheels/migratorwith a misconfigured datasource → red database SVG renders above the "Database Error" heading;#message#text still shows./wheels/migratorstill renders normally when the datasource is reachable (no regression — only the error branch was touched).Follow-up
The same
<i class="...icon">pattern is broken in four other dev-tools views (templating.cfm,packages.cfm,packagelist.cfm,docs/guides.cfm). Tracked as a separate sweep — out of scope for this fix.🤖 Generated with Claude Code