Fix the Discover deck review findings
CI / ci (push) Successful in 3m10s
CI / ci (pull_request) Successful in 2m39s

The Quest log pill is now a button that shows and hides the names of
the dinners in the Quest log, read from GET
/api/v1/dinners?status=quest_log; the designed Quest log sheet replaces
this list in its own task.

A batch whose job cannot be polled, or whose result cannot be read,
now shows an error with a Retry that asks for the batch again, instead
of leaving skeleton cards on screen with no way out.

Decisions go through one submit guarded by a ref set before the request
is sent, so presses before the next render cannot send a second,
conflicting decision. A decision counts as saved only when the answer
is that suggestion, accepted or declined as chosen; anything else shows
in the "Couldn't save that choice" banner.

Pass and Try it are IconButtons: IconButton gains a neutral tone and
showLabel, which shows the label as an 11 px caption under the circle.

Maestro task 1ecc121c-86eb-4784-a9fb-1314709b4a59.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-07 01:13:00 -04:00
co-authored by Claude Opus 5.5
parent 38fec723f6
commit fe87968ac9
7 changed files with 325 additions and 93 deletions
+36 -55
View File
@@ -10,6 +10,7 @@
.header {
display: flex;
flex-wrap: wrap;
align-items: flex-end;
justify-content: space-between;
gap: 16px;
@@ -33,7 +34,20 @@
line-height: 1.1;
}
/* The Quest log count pill. */
/* The Quest log count pill, in a 44 px tall hit box. */
.questLogButton {
flex: none;
display: inline-flex;
align-items: center;
min-width: 44px;
min-height: 44px;
padding: 0;
border: 0;
border-radius: var(--radius-pill);
background: transparent;
cursor: pointer;
}
.questLog {
flex: none;
padding: 7px 12px;
@@ -45,6 +59,26 @@
font-weight: 700;
}
/* The Quest log's dinner names, under the header while the pill is open. */
.questLogList {
flex: 1 0 100%;
font-family: var(--font-body);
font-size: 14px;
}
.questLogNames {
margin: 0;
padding-left: 20px;
display: flex;
flex-direction: column;
gap: 6px;
}
.questLogEmpty {
margin: 0;
color: var(--color-neutral-700);
}
/* The deck: three cards stacked in the same box, the top one in front. */
.deck {
position: relative;
@@ -172,7 +206,7 @@
color: var(--color-neutral-900);
}
/* Pass and Try it: a 62 px circle over an 11 px label, one button each. */
/* Pass and Try it: IconButtons with their 11 px labels shown. */
.actions {
display: flex;
justify-content: center;
@@ -180,53 +214,6 @@
margin-top: 4px;
}
.action {
display: flex;
flex-direction: column;
align-items: center;
gap: 6px;
min-width: 62px;
min-height: 62px;
padding: 0;
border: 0;
background: none;
color: var(--color-neutral-700);
font-family: var(--font-body);
font-size: 11px;
font-weight: 700;
cursor: pointer;
}
.action:disabled {
opacity: 0.45;
cursor: not-allowed;
}
.circle {
display: flex;
align-items: center;
justify-content: center;
width: 62px;
height: 62px;
border-radius: var(--radius-pill);
box-shadow: var(--shadow-md);
transition: transform 100ms var(--ease-out-soft);
}
.action:active:not(:disabled) .circle {
transform: scale(0.97);
}
.pass {
background: var(--color-neutral-100);
color: var(--color-text);
}
.tryIt {
background: var(--color-accent-2-600);
color: var(--on-accent);
}
/* Skeleton cards while the deck loads or a batch finds Side quests. */
.loading {
display: flex;
@@ -256,9 +243,3 @@
font-weight: 700;
color: var(--color-neutral-700);
}
@media (prefers-reduced-motion: reduce) {
.circle {
transition: none;
}
}
+113
View File
@@ -328,4 +328,117 @@ describe('DiscoverPage', () => {
fireEvent.click(within(alert).getByRole('button', { name: 'Retry' }))
await screen.findByRole('article', { name: 'Korean beef bowls' })
})
it('opens and closes the Quest log from its pill, a keyboard-reachable button', async () => {
stubServer([BOWLS], {}, 2)
await renderPage()
const pill = await screen.findByRole('button', { name: 'Quest log · 2' })
expect(pill).toHaveAttribute('aria-expanded', 'false')
expect(screen.queryByRole('list', { name: 'Quest log' })).not.toBeInTheDocument()
expectMinHitArea(pill)
pill.focus()
expect(pill).toHaveFocus()
fireEvent.click(pill)
expect(pill).toHaveAttribute('aria-expanded', 'true')
const list = screen.getByRole('list', { name: 'Quest log' })
expect(pill).toHaveAttribute('aria-controls', list.parentElement!.id)
expect(within(list).getAllByRole('listitem').map((item) => item.textContent)).toEqual(['Quest 0', 'Quest 1'])
fireEvent.click(pill)
expect(pill).toHaveAttribute('aria-expanded', 'false')
expect(screen.queryByRole('list', { name: 'Quest log' })).not.toBeInTheDocument()
})
it('says so when the Quest log it opens is empty', async () => {
stubServer([BOWLS], {}, 0)
await renderPage()
fireEvent.click(await screen.findByRole('button', { name: 'Quest log · 0' }))
expect(screen.getByText('No dinners in the Quest log yet. Try it on a Side quest to add one.')).toBeInTheDocument()
})
it('sends one decision for presses made before the page re-renders', async () => {
const accept = deferred()
const server = stubServer([BOWLS, TIKKA], { 'POST /api/v1/suggestions/s1/accept': () => accept.promise })
await renderPage()
await screen.findByRole('article', { name: 'Korean beef bowls' })
fireEvent.keyDown(window, { key: 'ArrowRight' })
fireEvent.keyDown(window, { key: 'ArrowLeft' })
fireEvent.click(screen.getByRole('button', { name: 'Pass' }))
await waitFor(() => expect(sent(server.fetchMock)).toContain('POST /api/v1/suggestions/s1/accept'))
expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/accept'])
expect(screen.getByRole('button', { name: 'Try it' })).toBeDisabled()
server.deck = [TIKKA]
accept.resolve(jsonResponse(200, { suggestion: { ...BOWLS, status: 'accepted' }, replenish_job_id: null }))
await screen.findByRole('article', { name: 'Crock pot chicken tikka masala' })
fireEvent.keyDown(window, { key: 'ArrowLeft' })
await waitFor(() =>
expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/s1/accept', 'POST /api/v1/suggestions/s2/decline']),
)
})
it.each([
['not a decision', {}],
['another suggestion', { suggestion: { ...TIKKA, status: 'accepted' }, replenish_job_id: null }],
['not accepted', { suggestion: { ...BOWLS, status: 'pending' }, replenish_job_id: null }],
])('shows a decision answered with %s as unsaved, keeping the card', async (_, answer) => {
stubServer([BOWLS], { 'POST /api/v1/suggestions/s1/accept': () => jsonResponse(200, answer) })
await renderPage()
await screen.findByRole('article', { name: 'Korean beef bowls' })
fireEvent.click(screen.getByRole('button', { name: 'Try it' }))
const alert = await screen.findByRole('alert')
expect(alert).toHaveTextContent("Couldn't save that choice. POST /api/v1/suggestions/s1/accept: response is not the accepted suggestion")
expect(within(alert).getByRole('button', { name: 'Retry' })).toBeInTheDocument()
expect(screen.getByRole('article', { name: 'Korean beef bowls' })).toBeInTheDocument()
})
it('shows a batch whose job cannot be polled with a retry, keeping the skeleton cards', async () => {
let failPoll = true
const server = stubServer([], {
'POST /api/v1/suggestions/batches': () =>
jsonResponse(202, { batch_id: 'b1', job_id: 'j1', trigger: 'manual', status: 'pending' }),
'GET /api/v1/jobs/j1': () => {
if (failPoll) return errorResponse(500, 'internal')
server.deck = [BOWLS]
return jsonResponse(200, {
id: 'j1',
kind: 'suggestion_batch',
status: 'succeeded',
result: { batch_id: 'b1', stage: 'done', suggestion_ids: ['s1'], dropped: 0 },
error: null,
})
},
})
await renderPage()
fireEvent.click(await screen.findByRole('button', { name: 'Get more now' }))
const alert = await screen.findByRole('alert')
expect(alert).toHaveTextContent("Couldn't check on new side quests.")
expect(screen.getAllByTestId('skeleton-card')).toHaveLength(3)
failPoll = false
fireEvent.click(within(alert).getByRole('button', { name: 'Retry' }))
await screen.findByRole('article', { name: 'Korean beef bowls' })
expect(screen.queryByRole('alert')).not.toBeInTheDocument()
expect(sent(server.fetchMock)).toEqual(['POST /api/v1/suggestions/batches', 'POST /api/v1/suggestions/batches'])
})
it('shows a batch whose job result is unreadable with a retry', async () => {
stubServer([], {
'POST /api/v1/suggestions/batches': () =>
jsonResponse(202, { batch_id: 'b1', job_id: 'j1', trigger: 'manual', status: 'pending' }),
'GET /api/v1/jobs/j1': () =>
jsonResponse(200, { id: 'j1', kind: 'suggestion_batch', status: 'succeeded', result: { batch_id: 'b1' }, error: null }),
})
await renderPage()
fireEvent.click(await screen.findByRole('button', { name: 'Get more now' }))
const alert = await screen.findByRole('alert')
expect(alert).toHaveTextContent("Couldn't find new side quests. GET /api/v1/jobs/j1: result is not a suggestion batch")
expect(within(alert).getByRole('button', { name: 'Retry' })).toBeInTheDocument()
expect(screen.getByRole('heading', { name: 'All caught up' })).toBeInTheDocument()
})
})
+84 -26
View File
@@ -1,18 +1,20 @@
import { useQueryClient } from '@tanstack/react-query'
import { Heart, X } from 'lucide-react'
import { useCallback, useEffect } from 'react'
import { useCallback, useEffect, useId, useMemo, useRef, useState } from 'react'
import { useJob } from '../api/index.ts'
import { Card } from '../ui/Card.tsx'
import { EmptyState } from '../ui/EmptyState.tsx'
import { ErrorBanner } from '../ui/ErrorBanner.tsx'
import { Icon } from '../ui/Icon.tsx'
import { IconButton } from '../ui/IconButton.tsx'
import { Skeleton } from '../ui/Skeleton.tsx'
import {
DECK_KEY,
parseBatchResult,
useDeck,
useDecide,
useQuestLogCount,
useQuestLog,
useStartBatch,
type BatchResult,
type Choice,
} from './api.ts'
import { DeckCard, DECK_DEPTH } from './DeckCard.tsx'
@@ -27,29 +29,88 @@ function isEditable(target: EventTarget | null): boolean {
)
}
// QuestLogPill is the "Quest log · N" pill. It shows and hides the names
// of the dinners in the Quest log; the Quest log sheet replaces this list.
function QuestLogPill({ dinners }: { dinners: { id: string; name: string }[] }) {
const [open, setOpen] = useState(false)
const listId = useId()
return (
<>
<button
type="button"
className={styles.questLogButton}
aria-expanded={open}
aria-controls={open ? listId : undefined}
onClick={() => setOpen(!open)}
>
<span className={styles.questLog}>Quest log · {dinners.length}</span>
</button>
{open && (
<Card id={listId} tone="muted" className={styles.questLogList}>
{dinners.length === 0 ? (
<p className={styles.questLogEmpty}>No dinners in the Quest log yet. Try it on a Side quest to add one.</p>
) : (
<ul aria-label="Quest log" className={styles.questLogNames}>
{dinners.map((dinner) => (
<li key={dinner.id}>{dinner.name}</li>
))}
</ul>
)}
</Card>
)}
</>
)
}
// DiscoverPage is mock 1a: the deck of Side quests to Try it or Pass, one
// at a time from the top card. Every decision — the buttons, the ← / →
// keys — goes through decide, which does nothing while busy.
// keys, Retry — goes through submit, which does nothing while one is in
// flight. The lock is a ref, set before the decision is sent, so a second
// press before the next render cannot send another.
export function DiscoverPage() {
const queryClient = useQueryClient()
const deck = useDeck()
const questLog = useQuestLogCount()
const questLog = useQuestLog()
const decision = useDecide()
const startBatch = useStartBatch()
const job = useJob<BatchResult>(startBatch.data?.job_id ?? null)
const job = useJob<unknown>(startBatch.data?.job_id ?? null)
const top = deck.data?.[0]
const busy = decision.isPending
const jobRunning = startBatch.isSuccess && job.status !== 'succeeded' && job.status !== 'failed'
const finding = startBatch.isPending || jobRunning
// The batch's result as far as it has gone, or why it cannot be read.
const batch = useMemo(() => {
if (job.result === null) return { stored: 0, error: null }
try {
return { stored: parseBatchResult(`GET /api/v1/jobs/${startBatch.data?.job_id}`, job.result).suggestion_ids.length, error: null }
} catch (error) {
return { stored: 0, error: error as Error }
}
}, [job.result, startBatch.data?.job_id])
// The job's failure, the latest poll's failure, or an unreadable result.
const batchError = job.error ?? batch.error
const { mutate: mutateDecision } = decision
const deciding = useRef(false)
const submit = useCallback(
(variables: { id: string; choice: Choice }) => {
if (deciding.current) return
deciding.current = true
mutateDecision(variables, {
onSettled: () => {
deciding.current = false
},
})
},
[mutateDecision],
)
const decide = useCallback(
(choice: Choice) => {
if (busy || top === undefined) return
mutateDecision({ id: top.id, choice })
if (top !== undefined) submit({ id: top.id, choice })
},
[busy, top, mutateDecision],
[top, submit],
)
useEffect(() => {
@@ -67,7 +128,7 @@ export function DiscoverPage() {
// The batch stores its Side quests as it goes, so the deck is read again
// whenever it reports more, and once it ends.
const stored = job.result?.suggestion_ids.length ?? 0
const { stored } = batch
useEffect(() => {
if (job.status === undefined) return
void queryClient.invalidateQueries({ queryKey: DECK_KEY })
@@ -85,6 +146,9 @@ export function DiscoverPage() {
))}
</div>
{finding && <p className={styles.finding}>Finding new side quests…</p>}
{jobRunning && batchError !== null && (
<ErrorBanner onRetry={() => startBatch.mutate()}>Couldn't check on new side quests. {batchError.message}</ErrorBanner>
)}
</section>
)
} else if (top === undefined) {
@@ -93,8 +157,8 @@ export function DiscoverPage() {
{startBatch.isError && (
<ErrorBanner onRetry={() => startBatch.mutate()}>Couldn't ask for more side quests. {startBatch.error.message}</ErrorBanner>
)}
{job.status === 'failed' && job.error !== null && (
<ErrorBanner onRetry={() => startBatch.mutate()}>Couldn't find new side quests. {job.error.message}</ErrorBanner>
{batchError !== null && (
<ErrorBanner onRetry={() => startBatch.mutate()}>Couldn't find new side quests. {batchError.message}</ErrorBanner>
)}
<EmptyState title="All caught up" action={{ label: 'Get more now', onClick: () => startBatch.mutate() }}>
New side quests arrive Sunday.
@@ -110,21 +174,15 @@ export function DiscoverPage() {
))}
</div>
<div className={styles.actions}>
<button type="button" className={styles.action} disabled={busy} onClick={() => decide('pass')}>
<span className={`${styles.circle} ${styles.pass}`} aria-hidden="true">
<Icon icon={X} size="nav" />
</span>
Pass
</button>
<button type="button" className={styles.action} disabled={busy} onClick={() => decide('try')}>
<span className={`${styles.circle} ${styles.tryIt}`} aria-hidden="true">
<Icon icon={Heart} size="nav" />
</span>
Try it
</button>
<IconButton label="Pass" size={62} tone="neutral" showLabel disabled={busy} onClick={() => decide('pass')}>
<Icon icon={X} size="nav" />
</IconButton>
<IconButton label="Try it" size={62} tone="teal" showLabel disabled={busy} onClick={() => decide('try')}>
<Icon icon={Heart} size="nav" />
</IconButton>
</div>
{decision.isError && (
<ErrorBanner onRetry={() => mutateDecision(decision.variables!)}>
<ErrorBanner onRetry={() => submit(decision.variables!)}>
Couldn't save that choice. {decision.error.message}
</ErrorBanner>
)}
@@ -139,7 +197,7 @@ export function DiscoverPage() {
<p className={styles.kicker}>Discover</p>
<h1 className={styles.title}>Side quests</h1>
</div>
{questLog.isSuccess && <span className={styles.questLog}>Quest log · {questLog.data}</span>}
{questLog.isSuccess && <QuestLogPill dinners={questLog.data} />}
</header>
{questLog.isError && (
<ErrorBanner onRetry={() => void questLog.refetch()}>Couldn't load the Quest log. {questLog.error.message}</ErrorBanner>
+44 -7
View File
@@ -32,6 +32,28 @@ export type Batch = { batch_id: string; job_id: string }
// BatchResult is the suggestion batch job's result, published as it goes.
export type BatchResult = { batch_id: string; stage: string; suggestion_ids: string[]; dropped: number }
// QuestLogDinner is a dinner in the Quest log, as the Quest log pill lists it.
export type QuestLogDinner = { id: string; name: string }
// DECISION_STATUS is the status a suggestion has once each choice is saved.
const DECISION_STATUS: Record<Choice, string> = { try: 'accepted', pass: 'declined' }
// parseBatchResult checks a suggestion batch job's result, which useJob
// passes on unread.
export function parseBatchResult(where: string, body: unknown): BatchResult {
if (
!isRecord(body) ||
typeof body.batch_id !== 'string' ||
typeof body.stage !== 'string' ||
!Array.isArray(body.suggestion_ids) ||
!body.suggestion_ids.every((id) => typeof id === 'string') ||
typeof body.dropped !== 'number'
) {
throw new Error(`${where}: result is not a suggestion batch`)
}
return { batch_id: body.batch_id, stage: body.stage, suggestion_ids: body.suggestion_ids as string[], dropped: body.dropped }
}
function parseSideQuest(where: string, body: unknown): SideQuest {
if (
!isRecord(body) ||
@@ -65,16 +87,22 @@ export function useDeck() {
})
}
// useQuestLogCount is how many dinners are in the Quest log (GET
// useQuestLog is the dinners in the Quest log, in the server's order (GET
// /api/v1/dinners?status=quest_log).
export function useQuestLogCount() {
export function useQuestLog() {
const { apiFetch } = useApi()
return useQuery({
queryKey: QUEST_LOG_KEY,
queryFn: async () => {
queryFn: async (): Promise<QuestLogDinner[]> => {
const where = `GET ${QUEST_LOG_PATH}`
const body = await apiFetch<unknown>(QUEST_LOG_PATH)
if (!isRecord(body) || !Array.isArray(body.dinners)) throw new Error(`GET ${QUEST_LOG_PATH}: response has no dinners list`)
return body.dinners.length
if (!isRecord(body) || !Array.isArray(body.dinners)) throw new Error(`${where}: response has no dinners list`)
return body.dinners.map((dinner) => {
if (!isRecord(dinner) || typeof dinner.id !== 'string' || typeof dinner.name !== 'string') {
throw new Error(`${where}: response is not a dinner`)
}
return { id: dinner.id, name: dinner.name }
})
},
})
}
@@ -97,14 +125,23 @@ export function useStartBatch() {
// useDecide records Try it (POST /api/v1/suggestions/{id}/accept) or Pass
// (POST /api/v1/suggestions/{id}/decline), then reads the deck and the
// Quest log again.
// Quest log again. The answer must be that suggestion, decided that way.
export function useDecide() {
const { apiFetch } = useApi()
const queryClient = useQueryClient()
return useMutation({
mutationFn: async ({ id, choice }: { id: string; choice: Choice }) => {
const action = choice === 'try' ? 'accept' : 'decline'
await apiFetch<unknown>(`${SUGGESTIONS_PATH}/${encodeURIComponent(id)}/${action}`, { method: 'POST' })
const path = `${SUGGESTIONS_PATH}/${encodeURIComponent(id)}/${action}`
const body = await apiFetch<unknown>(path, { method: 'POST' })
if (
!isRecord(body) ||
!isRecord(body.suggestion) ||
body.suggestion.id !== id ||
body.suggestion.status !== DECISION_STATUS[choice]
) {
throw new Error(`POST ${path}: response is not the ${DECISION_STATUS[choice]} suggestion`)
}
},
onSuccess: async () => {
await Promise.all([
+19
View File
@@ -29,6 +29,20 @@
height: 62px;
}
/* A shown label sits 6 px under the circle, and the button grows to hold it. */
.labelled {
flex-direction: column;
gap: 6px;
width: auto;
min-width: 62px;
height: auto;
min-height: 62px;
color: var(--color-neutral-700);
font-family: var(--font-body);
font-size: 11px;
font-weight: 700;
}
.face {
display: inline-flex;
align-items: center;
@@ -57,6 +71,11 @@
color: var(--color-text);
}
.neutral {
background: var(--color-neutral-100);
color: var(--color-text);
}
.accent {
background: var(--color-accent);
color: var(--on-accent);
+18
View File
@@ -1,5 +1,6 @@
import { fireEvent, render, screen } from '@testing-library/react'
import { describe, expect, it, vi } from 'vitest'
import { declaredStyle } from '../test/cssRules.ts'
import { hitArea, expectMinHitArea } from '../test/hitArea.ts'
import { IconButton } from './IconButton.tsx'
@@ -31,4 +32,21 @@ describe('IconButton', () => {
expectMinHitArea(button)
expect(hitArea(button)).toEqual({ width: 62, height: 62 })
})
it('shows its label as an 11 px caption under the circle, which names it', () => {
render(
<IconButton label="Pass" size={62} tone="neutral" showLabel>
<svg />
</IconButton>,
)
const button = screen.getByRole('button', { name: 'Pass' })
expect(button).not.toHaveAttribute('aria-label')
expect(button).toHaveTextContent('Pass')
expect(getComputedStyle(button).fontSize).toBe('11px')
expect(getComputedStyle(button).flexDirection).toBe('column')
expectMinHitArea(button)
const face = button.firstElementChild!
expect(hitArea(face)).toEqual({ width: 62, height: 62 })
expect(declaredStyle(face).background).toBe('var(--color-neutral-100)')
})
})
+11 -5
View File
@@ -2,32 +2,38 @@ import type { ComponentPropsWithRef } from 'react'
import styles from './IconButton.module.css'
export type IconButtonSize = 40 | 62
export type IconButtonTone = 'surface' | 'accent' | 'teal'
export type IconButtonTone = 'surface' | 'neutral' | 'accent' | 'teal'
export type IconButtonProps = Omit<ComponentPropsWithRef<'button'>, 'aria-label'> & {
// Accessible name; an icon alone never names a button.
label: string
size?: IconButtonSize
tone?: IconButtonTone
// Shows the label as an 11 px caption under the circle, which then names
// the button.
showLabel?: boolean
}
// Round icon-only button. The 40 px size keeps a 44 px hit box (see the CSS);
// the 62 px size is the round action button with --shadow-md.
// Round icon button, named by its label and showing it only with showLabel.
// The 40 px size keeps a 44 px hit box (see the CSS); the 62 px size is the
// round action button with --shadow-md.
export function IconButton({
label,
size = 40,
tone = 'surface',
showLabel = false,
type = 'button',
className,
children,
...rest
}: IconButtonProps) {
const classes = [styles.button, styles[`size${size}`], className].filter(Boolean).join(' ')
const classes = [styles.button, styles[`size${size}`], showLabel && styles.labelled, className].filter(Boolean).join(' ')
return (
<button type={type} aria-label={label} className={classes} {...rest}>
<button type={type} aria-label={showLabel ? undefined : label} className={classes} {...rest}>
<span className={[styles.face, styles[tone]].join(' ')} aria-hidden="true">
{children}
</span>
{showLabel && label}
</button>
)
}