fix: code-review batch — sysmon sampler liveness, stale fetch race, a11y names, interaction polish - #17
Merged
Conversation
The self-chained 16ms tick returned early when the window was invisible WITHOUT rescheduling, so one minimized tick killed the sampling chain permanently — frame-time p50/max froze forever even after restore. Reschedule before any early return, skip only the push while hidden, and advance the timestamp every tick so the first sample after restore measures one interval instead of the whole minimized duration. Also corrects the comment: the chain is a timer approximation of frame cadence, not real rAF.
refreshWindows() destroyed and recreated every window-row entity each
1s tick — GC churn, pressed/hover state loss, and a rebuilt a11y
subtree under screen readers. Reconcile against wm.list() keyed by
windowId instead: closed windows drop their rows, survivors are reused
and only relabeled when focus/minimize state changes, and the empty
placeholder is created once.
Also gives the ✕ close button a textual accessible name ("Close
<title>") while keeping the glyph visual.
DesktopClickCatcher set dragging on pointerdown but only cleared it on pointerup, so a gesture released over another mirror, cancelled by the browser, or leaving the canvas left dragging/moved latched — later buttonless pointermove kept drawing marquees. Mirror ThemedButton's defensive handlers: abandon the drag on pointercancel and pointerleave, collapsing any latched marquee via a final zero-size rect.
The emoji fallback has been populated for all 10 icons since the SVG migration but never read anywhere. Remove the field, its factory parameter, and the 10 literal arguments.
Ctrl+C with the calculator focused cleared the display and Ctrl+Backspace backspaced: the global keydown handler read every key as a keypad press. Early-return on ctrlKey/metaKey/altKey, matching the terminal's guard.
render() awaited the proxy fetch with no generation guard, so Back or re-navigation during an in-flight fetch let the old response overwrite the newer page's title/body/status (address bar then disagreed with the page). Capture the requested URL before the await and bail unless it is still history[historyIndex] after both the fetch and the JSON parse — including the network-error path.
The usage hint was anchored at absolute x=364 with clipChildren=true: fully outside the client area at the declared 360px minimum and its tail clipped even at the default 600px width. Derive its position from the actual swatch row and hide it when the window cannot fit it (IRenderer has no measureText, so the fit budget is ~5.5px per glyph). Also removes the unreachable currentColor === '' branch that highlighted the Clear button — the color starts as #000000 and only changes via swatch clicks, so it is never empty.
ThemedButton now projects an optional a11y name in place of the visual label (setA11yName + getA11yAttributes override; btn/themedButton take an optional 4th argument). Decorative emoji stay visible on the canvas while screen readers hear plain words: - Files nav buttons: Root / /docs / /notes / Refresh / Seed Samples - Files entry rows: '<name> (<size> B)' without the kind glyph - Notes toolbar: Save / Reload / Clear
The dial used the full half-height for its radius, pushing the toLocaleTimeString readout past the bottom edge at the declared 240x220 minimum. Reserve 24px under the dial when computing the radius.
The minimum-geometry gate exercised clock at 260x220 while clock.ts declares minWidth 240 — the test was looser than the contract it guards.
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.
Implements accepted findings from the WEB-0017 review. Rebased onto main after #16 (the deps commit is intentionally absent here).
Deferred with notes: notes unsaved-edit confirm (needs dialog pattern, PX-0040), settings active-preset indicator (PX-0040).
(*rebase rewrote SHAs; see branch tip e90311c)
Validation
bun run checkclean ·bun test38/38 (260 expects) · build verified pre-rebase (361.00 kB / gzip 108.79 kB)CarryCtx: WEB-0019 (findings WEB-0017 PX-0026..0036)