Repository navigation
Conversation
The main E2E run went red on the save-validity tests: `vm.swunorm()` reported invalid SWU while the Save button still read aria-disabled="false". Two causes, both about font readiness: useFontStore seeded `ready` from document.fonts.check(). Chromium answers true for a face that is still downloading (status 'loading'), so `ready` was already true at the first render and ensureSignWritingFonts() took its early return — the false→true flip that tells consumers to recompute never fired. Seed it false and always run the load; cached and locally-installed fonts resolve within a frame, so returning visitors still see no placeholder. Saveability is derived from glyph sizes, which font-ttf can only measure once those fonts are available — until then signNormalize falls back to the un-normalized sign and an oversized sign measures as fitting. The Save buttons subscribed only to the sign store, so they kept that first wrong answer. They now go through useSaveable(), which also subscribes to font readiness, matching the existing useGlyph pattern. This never reproduces on a machine with the SignWriting fonts installed: local() resolves with no download and the first render already has real measurements. The new test strips local() and delays the woff2 to force the cold path — it fails on the previous build and passes on this one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three cleanups on the previous commit, no behavior change: - useSaveable lived in the store module; every other hook lives in src/hooks, and having it there meant signStore imported fontStore purely to serve a component concern. - The `!ready ||` short-circuit ran after the store selector had already computed saveable(). That is the one window where the computation is expensive: pre-fonts, font-ttf's symbolSize misses its cache, scans a blank 152x152 canvas per symbol, and returns undefined without caching, so every call repeats the scan. Folding the gate into the selector skips it. - The comment claimed cached fonts resolve with no placeholder; document.fonts.load() always resolves asynchronously, so there is one painted frame either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Fixes the red E2E run on
main(run 30279252538), from #26.What broke
The save-validity tests failed with
vm.swunorm()reporting invalid SWU while the Save button still readaria-disabled="false"— the button had the right data available and was showing the wrong state.Two causes, both about font readiness:
1.
readynever flipped.useFontStoreseededreadyfromdocument.fonts.check(). Chromium answerstruefor a face that is still downloading (document.fonts.status === 'loading'), soreadywas alreadytrueat the first render andensureSignWritingFonts()took its early return. The false→true transition that tells consumers to recompute never fired. Seededfalsenow, and the load always runs; cached and locally-installed fonts resolve within a frame, so returning visitors still see no placeholder.2. The Save buttons didn't listen for it. Saveability is derived from glyph sizes, which font-ttf can only measure once those fonts are available — until then
signNormalizethrows andfswnormfalls back to the un-normalized sign, so an oversized sign measures as fitting. The buttons subscribed only to the sign store, which does not change when fonts arrive, so they kept that first wrong answer. They now go throughuseSaveable(), which also subscribes to font readiness — the same patternuseGlyphalready uses.Why it passed review and CI on the fork
It never reproduces on a machine with the SignWriting fonts installed:
local(SuttonSignWritingLine)resolves with zero.woff2requests, so the very first render already has real measurements. Cold CI runners download the fonts and lose the race.Timeline from a forced cold load, before the fix:
The normalized sign goes out of lane at ~2.5s, and nothing re-rendered.
Testing
New test strips
local()from the served CSS and delays the.woff2by 1s to force the cold path on any machine. Sampling the button againstswunorm()validity every 150ms across the font load:The test fails on the previous build and passes on this one. Full suite: 93 passing.
🤖 Generated with Claude Code