diff --git a/packages/web/src/common/utils/event/event.util.test.ts b/packages/web/src/common/utils/event/event.util.test.ts index ab7b091fc..23f375bd6 100644 --- a/packages/web/src/common/utils/event/event.util.test.ts +++ b/packages/web/src/common/utils/event/event.util.test.ts @@ -267,24 +267,17 @@ describe("addId", () => { }); }); -describe("focusCalendarEventElement", () => { - const EVENT_ID = "507f1f77bcf86cd799439011"; - let pendingFrames: FrameRequestCallback[]; +const EVENT_ID = "507f1f77bcf86cd799439011"; + +/** + * The three focus helpers all retry across animation frames, so every suite + * below needs the same seam: a stubbed `requestAnimationFrame` whose queue the + * test drains by hand, plus a way to mount an event card mid-retry. + */ +const setupFocusRetryHarness = () => { + let pendingFrames: FrameRequestCallback[] = []; let originalRequestAnimationFrame: typeof requestAnimationFrame; - const addEventElement = () => { - const element = document.createElement("div"); - element.setAttribute(WEEK_INTERACTION_EVENT_ID_ATTRIBUTE, EVENT_ID); - element.tabIndex = 0; - document.body.appendChild(element); - return element; - }; - - const flushFrame = () => { - const frames = pendingFrames.splice(0); - frames.forEach((frame) => frame(performance.now())); - }; - beforeEach(() => { pendingFrames = []; originalRequestAnimationFrame = globalThis.requestAnimationFrame; @@ -297,13 +290,33 @@ describe("focusCalendarEventElement", () => { document.body.innerHTML = ""; }); + return { + addEventElement: () => { + const element = document.createElement("div"); + element.setAttribute(WEEK_INTERACTION_EVENT_ID_ATTRIBUTE, EVENT_ID); + element.tabIndex = 0; + document.body.appendChild(element); + return element; + }, + flushFrame: () => { + const frames = pendingFrames.splice(0); + frames.forEach((frame) => frame(performance.now())); + }, + pendingFrameCount: () => pendingFrames.length, + }; +}; + +describe("focusCalendarEventElement", () => { + const { addEventElement, flushFrame, pendingFrameCount } = + setupFocusRetryHarness(); + it("focuses the event element when it already exists", () => { const element = addEventElement(); focusCalendarEventElement(EVENT_ID); expect(document.activeElement).toBe(element); - expect(pendingFrames).toHaveLength(0); + expect(pendingFrameCount()).toBe(0); }); it("retries until the event element appears", () => { @@ -317,37 +330,25 @@ describe("focusCalendarEventElement", () => { expect(document.activeElement).toBe(element); }); -}); - -describe("focusCalendarEventElementAfterDiscard", () => { - const EVENT_ID = "507f1f77bcf86cd799439011"; - let pendingFrames: FrameRequestCallback[]; - let originalRequestAnimationFrame: typeof requestAnimationFrame; - const addEventElement = () => { - const element = document.createElement("div"); - element.setAttribute(WEEK_INTERACTION_EVENT_ID_ATTRIBUTE, EVENT_ID); - element.tabIndex = 0; - document.body.appendChild(element); - return element; - }; + it("gives up once the retry budget is spent", () => { + focusCalendarEventElement(EVENT_ID); - const flushFrame = () => { - const frames = pendingFrames.splice(0); - frames.forEach((frame) => frame(performance.now())); - }; + let flushes = 0; + while (pendingFrameCount() > 0 && flushes < 100) { + flushFrame(); + flushes += 1; + } - beforeEach(() => { - pendingFrames = []; - originalRequestAnimationFrame = globalThis.requestAnimationFrame; - globalThis.requestAnimationFrame = ((frame: FrameRequestCallback) => - pendingFrames.push(frame)) as typeof requestAnimationFrame; + expect(flushes).toBeLessThan(100); + const element = addEventElement(); + flushFrame(); + expect(document.activeElement).not.toBe(element); }); +}); - afterEach(() => { - globalThis.requestAnimationFrame = originalRequestAnimationFrame; - document.body.innerHTML = ""; - }); +describe("focusCalendarEventElementAfterDiscard", () => { + const { addEventElement, flushFrame } = setupFocusRetryHarness(); it("skips a draft portal and focuses the saved card", () => { const draftPortal = addEventElement(); @@ -378,34 +379,8 @@ describe("focusCalendarEventElementAfterDiscard", () => { }); describe("refocusEventElement", () => { - const EVENT_ID = "507f1f77bcf86cd799439011"; - let pendingFrames: FrameRequestCallback[]; - let originalRequestAnimationFrame: typeof requestAnimationFrame; - - const addEventElement = () => { - const element = document.createElement("div"); - element.setAttribute(WEEK_INTERACTION_EVENT_ID_ATTRIBUTE, EVENT_ID); - element.tabIndex = 0; - document.body.appendChild(element); - return element; - }; - - const flushFrame = () => { - const frames = pendingFrames.splice(0); - frames.forEach((frame) => frame(performance.now())); - }; - - beforeEach(() => { - pendingFrames = []; - originalRequestAnimationFrame = globalThis.requestAnimationFrame; - globalThis.requestAnimationFrame = ((frame: FrameRequestCallback) => - pendingFrames.push(frame)) as typeof requestAnimationFrame; - }); - - afterEach(() => { - globalThis.requestAnimationFrame = originalRequestAnimationFrame; - document.body.innerHTML = ""; - }); + const { addEventElement, flushFrame, pendingFrameCount } = + setupFocusRetryHarness(); it("focuses the event's element once it is replaced", () => { const staleElement = addEventElement(); @@ -430,7 +405,7 @@ describe("refocusEventElement", () => { refocusEventElement(EVENT_ID); let flushes = 0; - while (pendingFrames.length > 0 && flushes < 100) { + while (pendingFrameCount() > 0 && flushes < 100) { flushFrame(); flushes += 1; } diff --git a/packages/web/src/common/utils/event/event.util.ts b/packages/web/src/common/utils/event/event.util.ts index 184c5b511..2701fd75e 100644 --- a/packages/web/src/common/utils/event/event.util.ts +++ b/packages/web/src/common/utils/event/event.util.ts @@ -116,27 +116,51 @@ export const getCalendarEventIdFromElement = (element: HTMLElement) => readCalendarEventIdFromElement(element); /** - * Focuses a calendar event's DOM node as soon as it exists. Retries across - * animation frames when the card is not mounted yet (e.g. after form close or - * undo restore). Unlike `refocusEventElement`, this focuses an in-place node - * and does not wait for React to replace it. + * How many frames a focus retry waits for its target. Roughly half a second + * at 60fps: long enough for React to commit a remount, short enough that a + * card which never arrives stops spinning. */ -export const focusCalendarEventElement = (eventId: string) => { - const selector = calendarEventIdValueSelector(eventId); - let attempts = 0; +const FOCUS_RETRY_FRAMES = 30; + +/** + * Focuses the element `findTarget` returns, retrying once per animation frame + * until it appears or the budget runs out. `startOnNextFrame` defers the first + * look by a frame, for callers that must let a pending unmount commit first. + */ +const focusAcrossFrames = ( + findTarget: () => HTMLElement | null | undefined, + { startOnNextFrame = false }: { startOnNextFrame?: boolean } = {}, +) => { + let attemptsLeft = FOCUS_RETRY_FRAMES; const tryFocus = () => { - const element = document.querySelector(selector); + const element = findTarget(); if (element) { element.focus(); return; } - if (++attempts < 30) requestAnimationFrame(tryFocus); + attemptsLeft -= 1; + if (attemptsLeft > 0) requestAnimationFrame(tryFocus); }; + if (startOnNextFrame) { + requestAnimationFrame(tryFocus); + return; + } tryFocus(); }; +/** + * Focuses a calendar event's DOM node as soon as it exists. Retries across + * animation frames when the card is not mounted yet (e.g. after form close or + * undo restore). Unlike `refocusEventElement`, this focuses an in-place node + * and does not wait for React to replace it. + */ +export const focusCalendarEventElement = (eventId: string) => { + const selector = calendarEventIdValueSelector(eventId); + focusAcrossFrames(() => document.querySelector(selector)); +}; + const isGridDraftEventSurface = (element: HTMLElement) => element.getAttribute("data-grid-event-surface") === "draft"; @@ -147,21 +171,13 @@ const isGridDraftEventSurface = (element: HTMLElement) => */ export const focusCalendarEventElementAfterDiscard = (eventId: string) => { const selector = calendarEventIdValueSelector(eventId); - let attempts = 0; - - const tryFocus = () => { - attempts += 1; - const element = [...document.querySelectorAll(selector)].find( - (candidate) => !isGridDraftEventSurface(candidate), - ); - if (element) { - element.focus(); - return; - } - if (attempts < 30) requestAnimationFrame(tryFocus); - }; - - requestAnimationFrame(tryFocus); + focusAcrossFrames( + () => + [...document.querySelectorAll(selector)].find( + (candidate) => !isGridDraftEventSurface(candidate), + ), + { startOnNextFrame: true }, + ); }; /** @@ -171,18 +187,10 @@ export const focusCalendarEventElementAfterDiscard = (eventId: string) => { export const refocusEventElement = (eventId: string) => { const selector = calendarEventIdValueSelector(eventId); const staleElement = document.querySelector(selector); - let attemptsLeft = 30; - - const tryFocus = () => { + focusAcrossFrames(() => { const element = document.querySelector(selector); - if (element && element !== staleElement) { - element.focus(); - } else if (attemptsLeft-- > 0) { - requestAnimationFrame(tryFocus); - } - }; - - tryFocus(); + return element && element !== staleElement ? element : null; + }); }; export const getWeekDayLabel = (day: Dayjs | Date) => { diff --git a/packages/web/src/components/PointerHint/PointerHint.test.tsx b/packages/web/src/components/PointerHint/PointerHint.test.tsx index 12555638b..35d97e0ee 100644 --- a/packages/web/src/components/PointerHint/PointerHint.test.tsx +++ b/packages/web/src/components/PointerHint/PointerHint.test.tsx @@ -12,6 +12,7 @@ import { pointerBlockActions, usePointerBlockStore, } from "@web/shortcuts/keyboard-only/pointer-block.store"; +import { KEYMAP } from "@web/shortcuts/keymap"; import { eventJumpActions, initialEventJumpState, @@ -254,6 +255,21 @@ describe("PointerHint", () => { ); }); + it("teaches the event-jump key when no sequence is assigned yet", () => { + render(); + + act(() => { + pointerBlockActions.pulseBlockedClick({ + actionId: POINTER_ACTIONS.eventOpen, + eventId: "event-1", + }); + }); + + expect(screen.getByRole("status")).toHaveTextContent( + `Press ${KEYMAP.eventJump.keycaps.join("")} to reveal event shortcuts.`, + ); + }); + it("shortens to a brief pulse after a few reminders in the session", () => { sessionStorage.setItem(HINT_COUNT_KEY, "3"); render(); diff --git a/packages/web/src/components/PointerHint/PointerHint.tsx b/packages/web/src/components/PointerHint/PointerHint.tsx index 531001dc1..404674242 100644 --- a/packages/web/src/components/PointerHint/PointerHint.tsx +++ b/packages/web/src/components/PointerHint/PointerHint.tsx @@ -18,7 +18,11 @@ import { selectPointerBlockPulse, usePointerBlockStore, } from "@web/shortcuts/keyboard-only/pointer-block.store"; -import { CONNECTION_BANNER_SHORTCUT_KEY } from "@web/shortcuts/notice-focus/useNoticeActionShortcut"; +import { KEYMAP } from "@web/shortcuts/keymap"; +import { + CONNECTION_BANNER_SHORTCUT_KEY, + START_TRIAL_SHORTCUT_KEY, +} from "@web/shortcuts/notice-focus/useNoticeActionShortcut"; import { selectEventJumpPointerHintKey, useEventJumpStore, @@ -49,18 +53,15 @@ const Key = ({ children }: { children: string }) => ( {children} ); +/** + * A named action (or a bound shortcut) earns the full display time; only the + * anonymous "keyboard only" pulse shortens. Derived from the attempt rather + * than enumerated, so a new pointer action does not have to be registered + * here as well as in `pointerHintMessage`. + */ const isContextualAttempt = (attempt: BlockedPointerAttempt | null) => - attempt?.actionId === POINTER_ACTIONS.sidebarClose || - attempt?.actionId === POINTER_ACTIONS.sidebarOpen || - attempt?.actionId === POINTER_ACTIONS.goToToday || - attempt?.actionId === POINTER_ACTIONS.switchView || - attempt?.actionId === POINTER_ACTIONS.eventOpen || - attempt?.actionId === POINTER_ACTIONS.startTrial || - attempt?.actionId === POINTER_ACTIONS.reconnectGoogle || - attempt?.actionId === POINTER_ACTIONS.upNextDismiss || - attempt?.actionId === "grid.timed" || - attempt?.actionId === "grid.all-day" || - Boolean(attempt?.shortcutKey); + attempt != null && + (attempt.actionId !== "unknown" || Boolean(attempt.shortcutKey)); const pointerHintMessage = ({ attempt, @@ -110,7 +111,8 @@ const pointerHintMessage = ({ if (!eventJumpKey) { return ( <> - Press S to reveal event shortcuts. + Press to reveal + event shortcuts. ); } @@ -142,7 +144,7 @@ const pointerHintMessage = ({ if (attempt?.actionId === POINTER_ACTIONS.startTrial) { return ( <> - Press S to start your trial. + Press {START_TRIAL_SHORTCUT_KEY} to start your trial. ); }