Skip to content

refactor(web): collapse duplicated focus-retry loops and pointer hint lists - #3079

Merged
tyler-dane merged 1 commit into
mainfrom
claude/eager-mayer-7p6eom
Sep 2, 2026
Merged

refactor(web): collapse duplicated focus-retry loops and pointer hint lists#3079
tyler-dane merged 1 commit into
mainfrom
claude/eager-mayer-7p6eom

Conversation

@tyler-dane

Copy link
Copy Markdown
Contributor

Summary

Technical-debt sweep over changes merged in the last 7 days (#3017#3078). Three pieces of duplication that recent work left behind, one of which had already drifted into a user-visible bug.

1. Three copies of the same animation-frame retry loop (packages/web/src/common/utils/event/event.util.ts)

focusCalendarEventElement, focusCalendarEventElementAfterDiscard, and refocusEventElement each carried their own tryFocus closure, their own copy of the magic 30, and their own off-by-one accounting (++attempts < 30, attempts < 30, attemptsLeft-- > 0) — so two of them retried 30 times and the third retried 31. They now share one focusAcrossFrames(findTarget, { startOnNextFrame }) helper with a single named FOCUS_RETRY_FRAMES budget; each caller supplies only the predicate that finds its target. The 31st retry is gone, which is the only behavior change.

2. A pointer-action list that had to be maintained in two places (PointerHint.tsx)

isContextualAttempt enumerated every entry of POINTER_ACTIONS plus both grid kinds — i.e. "everything except unknown" spelled out longhand. It grew by one line per PR (startTrial in #3050, reconnectGoogle in #3064, upNextDismiss in #3074); forgetting to extend it makes a new action's hint flash for 400ms instead of 2500ms. It now derives from the attempt directly.

3. Hardcoded shortcut letters that had drifted from the constants that own them (PointerHint.tsx)

The start-trial hint hardcoded S rather than START_TRIAL_SHORTCUT_KEY, and the event-open hint advertised Press S to reveal event shortcuts — but the binding is h (KEYMAP.eventJump), and S is the Saturday/Sunday jump prefix, so following the hint did something else entirely. That copy came in with #3017 and had no test. Both letters now read from the constants, and the corrected copy has a regression test.

Simplicity

Net line count is roughly flat, but three retry loops become one and two parallel lists become zero: adding a pointer action or remapping h no longer requires editing a second place that nothing enforces. The test file's rAF-stubbing harness was also written out three times and is now built once.

Automated validation

  • bun test:web — 1537 pass, 0 fail (182 files)
  • bun run lint — exit 0 (same 23 pre-existing warnings as main)
  • bun run type-check — clean

Behavior-preserving apart from the two intended corrections: the 31st retry in refocusEventElement, and the event-open hint now naming H instead of S.

Independent review

Not run — this is a self-contained refactor with existing coverage on all three focus helpers and on every PointerHint message branch. The diff is 4 files / 243 lines, inside the autofix-merge-guard.sh rails (≤8 files, ≤250 lines, no denylisted path).

Test plan

bun test:web
bun run lint
bun run type-check

Added: focusCalendarEventElement gives up once the retry budget is spent; PointerHint teaches the event-jump key when no sequence is assigned yet.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DCRGg7GX4J57nPqVVDvsFs


Generated by Claude Code

… lists

Three cleanups to duplication that recent work left behind.

`event.util.ts` carried three near-identical animation-frame retry loops
(`focusCalendarEventElement`, `focusCalendarEventElementAfterDiscard`,
`refocusEventElement`), each with its own copy of the magic 30 and its own
off-by-one accounting: two tried 30 times, the third tried 31. Collapse them
onto one `focusAcrossFrames` helper with a single named budget; each caller
now supplies only the predicate that finds its target. Its test file repeated
the same rAF-stubbing harness three times; that is now built once.

`PointerHint`'s `isContextualAttempt` enumerated every pointer action id, so
each new action had to be registered twice (here and in `pointerHintMessage`)
or the hint would flash for 400ms instead of 2500ms. It now derives from the
attempt: anything that is not `unknown` is contextual.

The same file hardcoded its shortcut letters instead of reading the shared
constants, and one had already drifted: the event-open hint advertised `S` to
reveal event shortcuts, but the binding is `h` (`KEYMAP.eventJump`), and `S`
is the Saturday/Sunday jump prefix. Both letters now come from the constants
that own them, with a regression test on the corrected copy.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DCRGg7GX4J57nPqVVDvsFs
@tyler-dane
tyler-dane enabled auto-merge (squash) September 2, 2026 10:09

Copy link
Copy Markdown
Contributor Author

unit (scripts) is red on this head, in the push-triggered run (33617934369). It is not this PR's failure, and I'm re-running it rather than changing the diff.

The same unit (scripts) job passed on this exact commit in the pull_request run (33617976233) — same SHA d006587, opposite result.

What the failing job actually did: every test it ran passed, the last one (purge-user (db) > normalizes the email before matching) finishing at 10:09:24. Nothing was logged for the next five minutes, then The action 'Run scripts tests' has timed out after 5 minutes at 10:14:32, and cleanup terminated orphaned bun and mongod-x64-ubuntu-6.0.14 processes. That is the runner hanging after the suite finished — a teardown hang in the DB-backed scripts tests, not a failing assertion.

It also cannot be this diff: the change is four files under packages/web/src (event.util.ts, PointerHint.tsx, and their tests). Nothing in packages/scripts imports them, and that suite spins up its own in-memory MongoDB. unit (scripts) is green on the last eight test-unit runs on main, so the base branch is not broken either.

Re-running the failed jobs in that run once. Every other check on this head is green: unit (web/core/backend/sync), e2e, lint, type-check, knip, lighthouse, and CodeQL. If the re-run fails again I will treat it as real and root-cause it instead.


Generated by Claude Code

@tyler-dane
tyler-dane merged commit 6bbc652 into main Sep 2, 2026
35 of 36 checks passed
@tyler-dane
tyler-dane deleted the claude/eager-mayer-7p6eom branch September 2, 2026 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants