Skip to content

Redraw on every key press in the interactive viewer - #96

Merged
bjmorgan merged 3 commits into
mainfrom
fix-key-redraw-throttle
Sep 1, 2026
Merged

Redraw on every key press in the interactive viewer#96
bjmorgan merged 3 commits into
mainfrom
fix-key-redraw-throttle

Conversation

@bjmorgan

@bjmorgan bjmorgan commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Stacked on #95.

Key presses in the interactive viewer were dispatched through the same 30 ms throttle as drag motion. A press made within that window of the previous draw (straight after releasing a drag, or two toggles in quick succession) mutated the view or style but dropped the redraw, and nothing was scheduled to catch up, so the display stayed stale until the next interaction.

A key press is a discrete action with no follow-up event, so it now always redraws. The throttle is reserved for drag motion, where the release handler guarantees a final draw. Style toggles need only a repaint rather than a full recompute: precompute never reads the toggled flags, the painter gates on them at draw time.

Also tidies the _apply_key_action docstring so "view" and "full" describe what the kinds actually mean, and drops a comment in the fidelity test that explained why the throttle happened not to bite there.

@bjmorgan bjmorgan added the bug Something isn't working label Sep 1, 2026
Base automatically changed from fix-interactive-nonblocking to main September 1, 2026 20:15
Key presses were dispatched through the drag throttle, so a press
within 30 ms of the previous draw mutated the view or style but
dropped the redraw with nothing scheduled to catch up. A key press is
a discrete action with no follow-up event, so it now always redraws;
the throttle is reserved for drag motion, where release guarantees a
final draw.
@bjmorgan
bjmorgan force-pushed the fix-key-redraw-throttle branch from f06d60d to 2c2c81a Compare September 1, 2026 20:18
Copilot AI lite review requested due to automatic review settings September 1, 2026 20:18

This comment was marked as resolved.

Style toggles are now view redraws, so the _full_redraw docstring and
the _apply_key_action return-kind docstring described a split that no
longer exists; state the rule instead. Reword the key-press comment to
cover auto-repeat, and return "none" for keys swallowed during number
input, which change nothing and previously triggered a wasted repaint.
Copilot AI review requested due to automatic review settings September 1, 2026 20:28

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The behavior change is narrowly scoped, aligns with the described failure mode, and is covered by a targeted regression test.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

The test relied on both presses arriving within the throttle window by
wall-clock luck, so on a slow runner a re-throttled key handler would
still draw and the test would pass. Stub the module's time source so
every press is inside the window and a throttled redraw is always
dropped.
Copilot AI review requested due to automatic review settings September 1, 2026 21:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The change is narrowly scoped, matches the described bug, and is backed by a deterministic regression test covering the previously dropped-redraw scenario.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@bjmorgan
bjmorgan merged commit 1f213d9 into main Sep 1, 2026
6 checks passed
@bjmorgan
bjmorgan deleted the fix-key-redraw-throttle branch September 1, 2026 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants