From 3d67a7718361a0449d48484eff7cfb49a5db6ee9 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Mon, 26 Oct 2026 15:54:11 -0700 Subject: [PATCH] fix(app): question dock review fixes (#59383) --- .../session-question-dock.spec.ts | 127 +++++++++++++++--- .../requests/session-question-dock.tsx | 67 +++++---- packages/ui/src/components/rolling-number.css | 4 + packages/ui/src/components/rolling-number.tsx | 27 +++- 4 files changed, 160 insertions(+), 65 deletions(-) diff --git a/packages/app/component-tests/session-question-dock.spec.ts b/packages/app/component-tests/session-question-dock.spec.ts index 4ffef8520ed..0cc12b5ac82 100644 --- a/packages/app/component-tests/session-question-dock.spec.ts +++ b/packages/app/component-tests/session-question-dock.spec.ts @@ -260,7 +260,8 @@ story("switches questions with Left and Right outside the text field", async ({ // question text is moved, scaled and blurred, how far the count's last digit is rolled (in digit heights, down // positive), where the progress row's bar is, and the footer: how visible Back is and whether a click at its middle // reaches it, how visible the Next and Submit faces are, how visible the fainter of their two words is, and how wide -// the button they share shows. +// the button they share shows. It also keeps the header's text and, for the count's last digit, each digit drawn and +// how visible it is. const play = (question: Locator, keys: { key: string; after?: number }[]) => question.evaluate(async (root, keys) => { const opacity = (el: Element) => { @@ -277,6 +278,22 @@ const play = (question: Locator, keys: { key: string; after?: number }[]) => const live = () => root.querySelector('[data-slot="question-details"]:not([data-ghost])') + const mask = () => + ["question-details-frame", "question-options"].some((slot) => + (root.querySelector(`[data-slot="${slot}"]`)?.style.maskImage ?? "").includes("to right"), + ) + + const glyphs = () => + [ + ...root.querySelectorAll( + '[data-component="rolling-number"] [data-slot="rolling-number-digit"]:last-child [data-slot="rolling-number-glyph"]', + ), + ].flatMap((el) => { + const text = el.dataset.leaving ?? el.textContent ?? "" + + return text === "" ? [] : [{ text, shown: opacity(el) }] + }) + const roll = () => { const glyph = root.querySelector( '[data-component="rolling-number"] [data-slot="rolling-number-digit"]:last-child [data-slot="rolling-number-glyph"]:not([data-leaving])', @@ -316,8 +333,21 @@ const play = (question: Locator, keys: { key: string; after?: number }[]) => }), ) - // `click:N` clicks the Nth progress dash instead of pressing a key. + // `click:N` clicks the Nth progress dash, `next` clicks the Next or Submit button in use, and `mod+Enter` presses the + // submit shortcut. + const mac = /Mac|iPhone|iPad|iPod/.test(navigator.platform) + const press = (key: string) => { + if (key === "next") + return root + .querySelector('[data-slot="question-advance-face"][data-active="true"] button') + ?.click() + + if (key === "mod+Enter") + return (document.activeElement ?? root).dispatchEvent( + new KeyboardEvent("keydown", { key: "Enter", metaKey: mac, ctrlKey: !mac, bubbles: true, cancelable: true }), + ) + const dash = /^click:(\d+)$/.exec(key)?.[1] if (dash) @@ -347,6 +377,8 @@ const play = (question: Locator, keys: { key: string; after?: number }[]) => highlights: number footer: boolean animations: number + title: string + glyphs: { text: string; shown: number }[] }[] = [] const queue = [...keys] @@ -373,7 +405,7 @@ const play = (question: Locator, keys: { key: string; after?: number }[]) => x: matrix.m41, scale: matrix.m11, blur: (style?.filter ?? "none") !== "none", - mask: (style?.maskImage ?? "none") !== "none", + mask: mask(), digit: roll(), bar: bar(), back: getComputedStyle(back).visibility === "hidden" ? 0 : opacity(back), @@ -391,6 +423,8 @@ const play = (question: Locator, keys: { key: string; after?: number }[]) => highlights: root.querySelectorAll('[data-slot="question-highlight"]').length, footer: !!back.getAttribute("style") || faces.some((face) => !!face.getAttribute("style")), animations: document.getAnimations().length, + title: root.querySelector('[data-slot="question-header-title"]')?.textContent ?? "", + glyphs: glyphs(), } frames.push(frame) @@ -510,7 +544,71 @@ story.describe("with the question dock's motion on", () => { expect(Math.max(...backward.map((frame) => frame.digit))).toBeLessThanOrEqual(0) for (const frames of [forward, backward]) expect(frames.at(-1)?.digit).toBe(0) - await expect(question.locator('[data-slot="rolling-number-glyph"][data-leaving]')).toHaveText("") + await expect(question.locator('[data-slot="rolling-number-glyph"][data-leaving]')).toHaveAttribute( + "data-leaving", + "", + ) + }) + + story("takes Next clicks and the submit shortcut while the footer is still moving", async ({ mount }) => { + const component = await mount("session-question-switch--question-series") + const question = component.locator('[data-component="dock-prompt"][data-kind="question"]') + const title = question.locator('[data-slot="question-header-title"]') + await expect(question.getByRole("radio").first()).toBeFocused() + + // Two Next clicks and two submit shortcuts, two frames apart, each landing while Back is still fading in. + const frames = await play(question, [ + { key: "next" }, + { key: "next", after: 2 }, + { key: "mod+Enter", after: 2 }, + { key: "mod+Enter", after: 2 }, + ]) + + expect([2, 4, 6].every((index) => frames[index - 1]?.footer)).toBe(true) + await expect(question.getByRole("button", { name: "Submit", exact: true })).toBeVisible() + await expect(title).toHaveText("5 of 5 questions") + }) + + story("rolls the count on from where it is when it changes again mid-roll", async ({ mount }) => { + const component = await mount("session-question-switch--question-series") + const question = component.locator('[data-component="dock-prompt"][data-kind="question"]') + const title = question.locator('[data-slot="question-header-title"]') + await expect(question.getByRole("radio").first()).toBeFocused() + + // 1 → 2 → 3 and 1 → 2 → 1, the second change a few frames into the first roll. + for (const [second, end] of [ + ["ArrowRight", "3"], + ["ArrowLeft", "1"], + ] as const) + for (const after of [1, 2, 4]) { + const frames = await play(question, [{ key: "ArrowRight" }, { key: second, after }]) + const label = `1 → 2 → ${end} after ${after} frames` + + const shown = (frame: (typeof frames)[number], text: string) => + frame.glyphs.filter((glyph) => glyph.text === text).reduce((sum, glyph) => sum + glyph.shown, 0) + + // The digits drawn always add up to at least one whole digit, and the header's text never doubles a digit. + expect( + Math.min(...frames.map((frame) => frame.glyphs.reduce((sum, glyph) => sum + glyph.shown, 0))), + label, + ).toBeGreaterThanOrEqual(0.95) + expect( + frames.every((frame) => /^\d of 5 questions$/.test(frame.title.replace(/\s+/g, " ").trim())), + label, + ).toBe(true) + + // A 2 that had barely appeared goes back out and never crosses the middle. The frame sampled right after the + // second key shows the digits where the change found them; going back always sends the 2 out. + const at = shown(frames[after]!, "2") + + if (second === "ArrowLeft" || at < 0.45) + expect(Math.max(...frames.slice(after).map((frame) => shown(frame, "2"))), label).toBeLessThanOrEqual( + at + 0.02, + ) + expect(frames.at(-1)?.glyphs, label).toEqual([{ text: end, shown: 1 }]) + await expect(title).toHaveText(`${end} of 5 questions`) + await play(question, [{ key: "click:0" }]) + } }) // Where the bar sits against each dash, from the inline start of the progress row. @@ -767,21 +865,6 @@ story("switches questions, folds and submits on the same frame with the dock's m expect(submitted).toEqual(Array(3).fill({ dock: false, moving: false, animations: 0 })) }) -story("takes Next clicks and the submit shortcut while the footer is still moving", async ({ mount, page }) => { - const component = await mount("session-question-switch--question-series") - const question = component.locator('[data-component="dock-prompt"][data-kind="question"]') - const title = question.locator('[data-slot="question-header-title"]') - await expect(question.getByRole("radio").first()).toBeFocused() - - await question.getByRole("button", { name: "Next", exact: true }).click() - await question.getByRole("button", { name: "Next", exact: true }).click() - await page.keyboard.press("ControlOrMeta+Enter") - await expect(title).toHaveText("4 of 5 questions") - await page.keyboard.press("ControlOrMeta+Enter") - await expect(question.getByRole("button", { name: "Submit", exact: true })).toBeVisible() - await expect(title).toHaveText("5 of 5 questions") -}) - story("toggles checkboxes with Space on key down and moves focus with arrows", async ({ mount, page }) => { const component = await mount("app-current-session-surface--question-request") const question = component.locator('[data-component="dock-prompt"][data-kind="question"]') @@ -889,9 +972,11 @@ story("shows question navigation shortcuts in buttons and tooltips", async ({ mo await expect(submit.locator('[data-slot="question-submit-shortcut"]')).toBeVisible() await expect(submit.locator('[data-slot="question-submit-shortcut"]')).toHaveText(shortcut) await expect(submit).toHaveAttribute("aria-keyshortcuts", "Meta+Enter Control+Enter") + // Pressing the button closes its tooltip until the pointer comes back to it. + await page.mouse.move(0, 0) await submit.hover() - // Next stays in place under Submit, so its tooltip may still be fading out. - await expect(page.getByRole("tooltip", { name: /Submit/ })).toContainText(shortcut) + await expect(page.getByRole("tooltip")).toContainText("Submit") + await expect(page.getByRole("tooltip")).toContainText(shortcut) const back = component.getByRole("button", { name: "Back", exact: true }) await expect(back).toHaveText("Back") await expect(back).toHaveAttribute("aria-keyshortcuts", mac ? "Meta+[" : "Alt+ArrowLeft") diff --git a/packages/app/src/session/requests/session-question-dock.tsx b/packages/app/src/session/requests/session-question-dock.tsx index ea7b9654922..c606be68817 100644 --- a/packages/app/src/session/requests/session-question-dock.tsx +++ b/packages/app/src/session/requests/session-question-dock.tsx @@ -754,7 +754,8 @@ export const SessionQuestionDock: Component<{ request: FormInfo; onSubmit: () => : { ...track(live, -dir * reach, moving, options, v.launch()), tab: store.tab, - els: [copy(detailsRef, detailsFrame), copy(listRef, optionsTrack)], + // The answers box scrolls back to the top for the new question, so the copy keeps its place on screen. + els: [copy(detailsRef, detailsFrame), copy(listRef, optionsTrack, -(optionsRef?.scrollTop ?? 0))], details: content.details, list: content.list, } @@ -923,6 +924,8 @@ export const SessionQuestionDock: Component<{ request: FormInfo; onSubmit: () => }) optsRef.length = options().length + if (optionsRef) optionsRef.scrollTop = 0 + const reduced = !questionMotion() if (reduced) bar.snap(dash(tab)) @@ -1120,22 +1123,22 @@ export const SessionQuestionDock: Component<{ request: FormInfo; onSubmit: () => - {/* Next and Submit share one place; the one not in use lies under it, inert. */} -
-
(nextFace = el)} - > - - {language.t("ui.common.next")} - {submitShortcut()} - - } + {/* Next and Submit share one place and one tooltip; the one not in use lies under it, inert. */} + + {last() ? language.t("ui.common.submit") : language.t("ui.common.next")} + {submitShortcut()} + + } + > +
+
(nextFace = el)} > - -
-
(submitFace = el)} - > - - {language.t("ui.common.submit")} - {submitShortcut()} - - } +
+
(submitFace = el)} > - +
-
+
} @@ -1420,7 +1413,7 @@ function mask(el: HTMLElement, cut: number, shown: number, sliding: boolean) { // A still copy of `el` laid over it in `host`, after the live content so it never comes first in the DOM, inert and // hidden from assistive tech. Ids are dropped so they stay unique, and typed text is carried over. -function copy(el: HTMLElement, host: HTMLElement) { +function copy(el: HTMLElement, host: HTMLElement, top = 0) { // SAFETY: a deep clone of an element is an element of the same kind. const clone = el.cloneNode(true) as HTMLElement clone.removeAttribute("id") @@ -1433,7 +1426,7 @@ function copy(el: HTMLElement, host: HTMLElement) { clone.inert = true clone.setAttribute("aria-hidden", "true") clone.style.position = "absolute" - clone.style.top = "0" + clone.style.top = `${top}px` clone.style.insetInline = "var(--question-gutter)" clone.style.pointerEvents = "none" host.append(clone) diff --git a/packages/ui/src/components/rolling-number.css b/packages/ui/src/components/rolling-number.css index 742a0ae3656..82a8fe1b58d 100644 --- a/packages/ui/src/components/rolling-number.css +++ b/packages/ui/src/components/rolling-number.css @@ -16,5 +16,9 @@ [data-slot="rolling-number-glyph"][data-leaving] { pointer-events: none; + + &::before { + content: attr(data-leaving); + } } } diff --git a/packages/ui/src/components/rolling-number.tsx b/packages/ui/src/components/rolling-number.tsx index ba241daab8c..995db8d0332 100644 --- a/packages/ui/src/components/rolling-number.tsx +++ b/packages/ui/src/components/rolling-number.tsx @@ -46,12 +46,14 @@ function Digit(props: { digit: string; dir: number; spring: DurationOptions }) { let from = 0 let to = 0 + const progress = () => (spring ? Math.max(0, Math.min(1, spring.value)) : 1) + const paint = () => { - const p = spring ? Math.max(0, Math.min(1, spring.value)) : 1 + const p = progress() roll(incoming, enter * (1 - p)) roll(outgoing, from + (to - from) * p) - if (!spring && outgoing) outgoing.textContent = "" + if (!spring && outgoing) outgoing.dataset.leaving = "" } const tick = (dt: number) => { @@ -76,20 +78,30 @@ function Digit(props: { digit: string; dir: number; spring: DurationOptions }) { last = next if (!incoming || !outgoing || next === prev) return - incoming.textContent = next if (prefersReducedMotion() || !incoming.isConnected) { + incoming.textContent = next spring = undefined paint() return } - // A change mid-roll sends the digit on screen out from where it is. - from = enter * (1 - (spring ? Math.max(0, Math.min(1, spring.value)) : 1)) + // A change mid-roll carries on from where both digits are, so together they never show less than one digit. + // Going back to the digit still leaving brings it back from where it is, and the arriving one leaves the way it + // came. Otherwise the digit nearer the middle leaves from there, and the new one comes in as far out as the + // other was, so a digit that had barely appeared never crosses the middle. + const p = progress() + const now = { in: enter * (1 - p), out: from + (to - from) * p } + const back = !!spring && next === outgoing.dataset.leaving + const near = back || Math.abs(now.in) <= Math.abs(now.out) + const far = !spring ? 1 : Math.min(1, Math.abs(near ? now.out : now.in)) + + if (near) outgoing.dataset.leaving = prev + from = near ? now.in : now.out to = -props.dir - enter = props.dir - outgoing.textContent = prev + enter = back ? now.out : props.dir * far + incoming.textContent = next spring = Spring.fromDuration(0, props.spring).set(1) paint() stop ??= frameLoop.add(tick) @@ -109,6 +121,7 @@ function Digit(props: { digit: string; dir: number; spring: DurationOptions }) { el.textContent = props.digit }} /> + {/* The leaving digit is drawn from its attribute, so the number's text is only ever the value it shows. */} )