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

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;
     }

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<HTMLElement>(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<HTMLElement>(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<HTMLElement>(selector)].find(
-      (candidate) => !isGridDraftEventSurface(candidate),
-    );
-    if (element) {
-      element.focus();
-      return;
-    }
-    if (attempts < 30) requestAnimationFrame(tryFocus);
-  };
-
-  requestAnimationFrame(tryFocus);
+  focusAcrossFrames(
+    () =>
+      [...document.querySelectorAll<HTMLElement>(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<HTMLElement>(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) => {

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(<PointerHint />);
+
+    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(<PointerHint />);

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 }) => (
   <kbd className="c-keycap">{children}</kbd>
 );
 
+/**
+ * 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 <Key>S</Key> to reveal event shortcuts.
+          Press <ShortcutKeys keys={[...KEYMAP.eventJump.keycaps]} /> to reveal
+          event shortcuts.
         </>
       );
     }
@@ -142,7 +144,7 @@ const pointerHintMessage = ({
   if (attempt?.actionId === POINTER_ACTIONS.startTrial) {
     return (
       <>
-        Press <Key>S</Key> to start your trial.
+        Press <Key>{START_TRIAL_SHORTCUT_KEY}</Key> to start your trial.
       </>
     );
   }