fix(view): flush block/box glyphs via the canvas renderer (v0.4.1)#5
Merged
Conversation
Block art (e.g. the Claude Code splash run inside the terminal) showed seams: xterm 5.x defaults to the DOM renderer, which draws block/box glyphs from the font (gappy). Load the vendored @xterm/addon-canvas + set customGlyphs:true (and lineHeight:1.0) so those glyphs are drawn as exact cell-filling shapes — flush, no seams. Also resolve the mono font CONCRETELY (the canvas font string can't read var(--pl-font-mono)). vendor/addon-canvas.js added + whitelisted; tests cover the asset + the CanvasAddon/ customGlyphs markers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
👀 Quinn is reviewing — verdict (PASS / WARN / FAIL) + findings to follow. |
There was a problem hiding this comment.
QA Audit — PR #5 | fix(view): flush block/box glyphs via the canvas renderer (v0.4.1)
VERDICT: WARN (CI still queued — non-blocking comment; will re-evaluate once checks settle)
CI Status
- test: queued
Diff Review
- Switches xterm from DOM renderer to canvas renderer (+
customGlyphs:true,lineHeight:1.0) so block/box glyphs (▛▜▝▘█▐▌) render as exact cell-filling shapes — no seams. - Resolves
--pl-font-monoCSS variable concretely before passing tofontFamily— the canvas font string can't read CSS vars (correct fix). - Vendored
addon-canvas.jsis self-served + whitelisted inapi.pyMIME map. - CanvasAddon loads best-effort via try/catch — DOM renderer fallback is safe (customGlyphs is a no-op on DOM).
Observations
- LOW: Empty catch block on
term.loadAddon(new CanvasAddon())silently swallows errors. Considerconsole.warnfor diagnostics (non-blocking). - GAP: clawpatch structural review unavailable (repo not in project registry). Diff-based review only.
- The vendored
addon-canvas.js(~29KB) is standard xterm.js addon bundling — no concerns.
— Quinn, QA Engineer
|
Submitted COMMENT review on |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Block art rendered inside the terminal (e.g. the Claude Code splash) showed seams. Cause: xterm 5.x defaults to the DOM renderer, which draws block/box glyphs (
▛▜▝▘█▐▌) from the font — fonts rarely fill the full cell, so contiguous blocks gap.Fix: load the vendored @xterm/addon-canvas +
customGlyphs: true(+lineHeight: 1.0), so those glyphs are drawn as exact cell-filling shapes → flush, no seams (this is why protoMaker shipped a GPU renderer). Also resolve the mono font concretely — the canvas font string can't readvar(--pl-font-mono), so we read it and append fallbacks.vendor/addon-canvas.jsadded + whitelisted. Tests: the new asset is served + the page referencesCanvasAddon/customGlyphs. 20 tests green.