Keep the recipe action bar at the bottom and pause polling offline
The detail page now fills the shell's scroll area (min-height: 100% of
a stretched flex item) and the sheet grows into the room left, so on a
short or failed recipe Add to this week and the heart rest at the
bottom of the screen; over a long recipe the bar still sticks there.
useDinnerDetail reads through apiRead and keeps whether the answer was
the service worker's saved copy. A pending dinner is polled only while
the network status says the server can be reached and the last answer
was not a saved copy; a saved pending copy is read again once the
connection comes back. TanStack Query clears the interval on unmount.
The credit stays plain text. source.url is parsed when present (https,
no userinfo, query or fragment, else the read fails) and makes "Recipe
from {site}" a new-tab link; today's API never sends it.
Maestro task 70a903ef-107c-444e-b3b0-fb3d37e8fb7d.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,8 +1,12 @@
|
||||
/* Mock 1k. On mobile the photo and the sheet run full-bleed past the
|
||||
shell's 20 px side padding; from tablet up they sit in one column. */
|
||||
shell's 20 px side padding; from tablet up they sit in one column. The
|
||||
page is at least as tall as the shell's scroll area (a stretched flex
|
||||
item, so its height resolves the percentage) and the sheet grows into
|
||||
any room left, so the action bar rests at the bottom of a short recipe. */
|
||||
.page {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
min-height: 100%;
|
||||
font-family: var(--font-body);
|
||||
color: var(--color-text);
|
||||
}
|
||||
@@ -52,6 +56,7 @@
|
||||
/* The sheet overlaps the photo by 28 px. */
|
||||
.sheet {
|
||||
position: relative;
|
||||
flex-grow: 1;
|
||||
margin-top: -28px;
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
@@ -162,6 +167,10 @@
|
||||
color: var(--color-neutral-700);
|
||||
}
|
||||
|
||||
.source a {
|
||||
color: inherit;
|
||||
}
|
||||
|
||||
.missing {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
@@ -189,8 +198,10 @@
|
||||
font-weight: 700;
|
||||
}
|
||||
|
||||
/* Held at the bottom of the screen. Sticky offsets count from inside the
|
||||
scroll area's padding, which on mobile already keeps clear of the nav bar. */
|
||||
/* At the bottom of the page, which fills the scroll area, and held at the
|
||||
bottom of the screen over a long recipe. Sticky offsets count from inside
|
||||
the scroll area's padding, which on mobile already keeps clear of the nav
|
||||
bar. */
|
||||
.bar {
|
||||
position: sticky;
|
||||
bottom: 0;
|
||||
|
||||
@@ -1,10 +1,13 @@
|
||||
import { fireEvent, render, screen, within } from '@testing-library/react'
|
||||
import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react'
|
||||
import { createMemoryRouter, RouterProvider } from 'react-router'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest'
|
||||
import { AppShell } from '../shell/AppShell.tsx'
|
||||
import { cacheFallbackHeader } from '../sw/routes.ts'
|
||||
import { ApiProviders } from '../test/ApiProviders.tsx'
|
||||
import { declaredStyle } from '../test/cssRules.ts'
|
||||
import { errorResponse, jsonResponse, stubFetchByPath } from '../test/fetchStub.ts'
|
||||
import { mockViewport } from '../test/matchMedia.ts'
|
||||
import { GROUNDING_POLL_MS } from './api.ts'
|
||||
import { DinnerDetailPage, REGULAR_DINNERS } from './DinnerDetailPage.tsx'
|
||||
|
||||
const DINNER = '/api/v1/dinners/d1'
|
||||
@@ -68,12 +71,13 @@ const FAILED = {
|
||||
why_picked: null,
|
||||
}
|
||||
|
||||
// renderAt renders the detail screen at the last of entries, with probes
|
||||
// for where its links and back button go.
|
||||
// renderAt renders the detail screen at the last of entries, inside the
|
||||
// app shell's scroll area as App routes it, with probes for where its links
|
||||
// and back button go.
|
||||
function renderAt(entries: string[]) {
|
||||
const router = createMemoryRouter(
|
||||
[
|
||||
{ path: '/dinner/:id', element: <DinnerDetailPage /> },
|
||||
{ element: <AppShell />, children: [{ path: '/dinner/:id', element: <DinnerDetailPage /> }] },
|
||||
{ path: '/', element: <h1>Home probe</h1> },
|
||||
{ path: '/plan', element: <h1>Plan probe</h1> },
|
||||
{ path: REGULAR_DINNERS, element: <h1>Regular dinners probe</h1> },
|
||||
@@ -159,6 +163,29 @@ describe('DinnerDetailPage', () => {
|
||||
expect(screen.queryByRole('link', { name: /Recipe from/ })).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('links the credit to the recipe in a new tab once the server sends a safe url', async () => {
|
||||
serve(detail({ source: { site_name: 'Serious Eats', url: 'https://www.seriouseats.com/korean-beef-bowls' } }))
|
||||
renderAt(['/dinner/d1'])
|
||||
|
||||
const credit = await screen.findByRole('link', { name: 'Recipe from Serious Eats' })
|
||||
expect(credit).toHaveAttribute('href', 'https://www.seriouseats.com/korean-beef-bowls')
|
||||
expect(credit).toHaveAttribute('target', '_blank')
|
||||
expect(credit).toHaveAttribute('rel', 'noopener noreferrer')
|
||||
})
|
||||
|
||||
it.each([
|
||||
['http', 'http://www.seriouseats.com/korean-beef-bowls'],
|
||||
['a password', 'https://user:secret@www.seriouseats.com/korean-beef-bowls'],
|
||||
['a query', 'https://www.seriouseats.com/korean-beef-bowls?token=abc'],
|
||||
['a fragment', 'https://www.seriouseats.com/korean-beef-bowls#token=abc'],
|
||||
])('refuses a source url with %s', async (_, url) => {
|
||||
serve(detail({ source: { site_name: 'Serious Eats', url } }))
|
||||
renderAt(['/dinner/d1'])
|
||||
|
||||
expect(await screen.findByRole('alert')).toHaveTextContent('source url is not an https link')
|
||||
expect(screen.queryByRole('link', { name: /Recipe from/ })).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('leaves the credit out when the server names no site', async () => {
|
||||
serve(detail({ source: { site_name: null } }))
|
||||
renderAt(['/dinner/d1'])
|
||||
@@ -180,9 +207,41 @@ describe('DinnerDetailPage', () => {
|
||||
const bar = heart.parentElement!
|
||||
expect(bar).toContainElement(add)
|
||||
expect(declaredStyle(bar).position).toBe('sticky')
|
||||
expect(declaredStyle(bar).bottom).toBe('0px')
|
||||
})
|
||||
})
|
||||
|
||||
// jsdom does no layout, so no test here can measure where the bar lands.
|
||||
// This pins the CSS chain that puts it at the bottom of the screen on a
|
||||
// short recipe: the shell's scroll area is a full-height flex item, the
|
||||
// page fills it as a column, the sheet takes the room left and the bar is
|
||||
// the page's last child. Drop any link and the bar sits right after the
|
||||
// content again, and this fails.
|
||||
it('fills the scroll area so the bar rests at the bottom of a short recipe', async () => {
|
||||
serve(detail(FAILED))
|
||||
renderAt(['/dinner/d1'])
|
||||
|
||||
const bar = (await screen.findByRole('button', { name: 'Favorite' })).parentElement!
|
||||
const page = bar.parentElement!
|
||||
const main = page.parentElement!
|
||||
const shell = main.parentElement!
|
||||
expect(main.tagName).toBe('MAIN')
|
||||
expect(declaredStyle(shell).height).toBe('100dvh')
|
||||
expect(declaredStyle(shell).display).toBe('flex')
|
||||
expect(declaredStyle(shell).alignItems).toBe('')
|
||||
expect(declaredStyle(main).overflowY).toBe('auto')
|
||||
|
||||
expect(declaredStyle(page).minHeight).toBe('100%')
|
||||
expect(declaredStyle(page).display).toBe('flex')
|
||||
expect(declaredStyle(page).flexDirection).toBe('column')
|
||||
expect(page.lastElementChild).toBe(bar)
|
||||
const sheet = screen.getByRole('article')
|
||||
expect(sheet.parentElement).toBe(page)
|
||||
expect(sheet.nextElementSibling).toBe(bar)
|
||||
expect(declaredStyle(sheet).flexGrow).toBe('1')
|
||||
expect(declaredStyle(bar).position).toBe('sticky')
|
||||
})
|
||||
|
||||
it('shows the striped placeholder when the dinner has no photo', async () => {
|
||||
serve(detail({ photo_urls: null }))
|
||||
renderAt(['/dinner/d1'])
|
||||
@@ -232,6 +291,119 @@ describe('DinnerDetailPage', () => {
|
||||
expect(screen.queryByRole('link', { name: 'Fix recipe' })).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
describe('while the recipe is being found', () => {
|
||||
const PENDING = detail({ ...FAILED, grounding_status: 'pending', grounding_error: null })
|
||||
const GROUNDED = detail({ grounding_status: 'grounded' })
|
||||
let onLine: boolean
|
||||
let body: unknown
|
||||
let fromCache: boolean
|
||||
let fetchMock: Mock<typeof fetch>
|
||||
|
||||
const reads = () => fetchMock.mock.calls.length
|
||||
const tick = (intervals = 1) => act(() => vi.advanceTimersByTimeAsync(GROUNDING_POLL_MS * intervals))
|
||||
|
||||
function setOnline(value: boolean) {
|
||||
onLine = value
|
||||
act(() => {
|
||||
window.dispatchEvent(new Event(value ? 'online' : 'offline'))
|
||||
})
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
// Only intervals are faked: TanStack Query polls with setInterval, and
|
||||
// Testing Library's waits keep their real timers.
|
||||
vi.useFakeTimers({ toFake: ['setInterval', 'clearInterval'] })
|
||||
onLine = true
|
||||
vi.spyOn(navigator, 'onLine', 'get').mockImplementation(() => onLine)
|
||||
body = PENDING
|
||||
fromCache = false
|
||||
fetchMock = stubFetchByPath({
|
||||
[DINNER]: () =>
|
||||
new Response(JSON.stringify(body), {
|
||||
headers: { 'Content-Type': 'application/json', ...(fromCache ? { [cacheFallbackHeader]: '1' } : {}) },
|
||||
}),
|
||||
})
|
||||
})
|
||||
|
||||
// An offline event leaves TanStack Query's online manager, shared by
|
||||
// every test, offline until an online event.
|
||||
afterEach(() => {
|
||||
window.dispatchEvent(new Event('online'))
|
||||
vi.useRealTimers()
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
it('reads the dinner again until the recipe lands, then stops', async () => {
|
||||
renderAt(['/dinner/d1'])
|
||||
expect(await screen.findByText(/Still finding the recipe/)).toBeInTheDocument()
|
||||
expect(reads()).toBe(1)
|
||||
|
||||
await tick()
|
||||
await waitFor(() => expect(reads()).toBe(2))
|
||||
|
||||
body = GROUNDED
|
||||
await tick()
|
||||
expect(await screen.findByRole('heading', { level: 2, name: 'Ingredients' })).toBeInTheDocument()
|
||||
expect(reads()).toBe(3)
|
||||
|
||||
await tick(3)
|
||||
expect(reads()).toBe(3)
|
||||
})
|
||||
|
||||
it('pauses while the browser is offline and resumes when it is back', async () => {
|
||||
renderAt(['/dinner/d1'])
|
||||
expect(await screen.findByText(/Still finding the recipe/)).toBeInTheDocument()
|
||||
expect(reads()).toBe(1)
|
||||
|
||||
setOnline(false)
|
||||
await tick(3)
|
||||
expect(reads()).toBe(1)
|
||||
|
||||
setOnline(true)
|
||||
await tick()
|
||||
await waitFor(() => expect(reads()).toBe(2))
|
||||
})
|
||||
|
||||
it('reads a saved pending copy offline once, then waits for the connection', async () => {
|
||||
onLine = false
|
||||
fromCache = true
|
||||
renderAt(['/dinner/d1'])
|
||||
expect(await screen.findByText(/Still finding the recipe/)).toBeInTheDocument()
|
||||
expect(reads()).toBe(1)
|
||||
|
||||
await tick(3)
|
||||
expect(reads()).toBe(1)
|
||||
|
||||
// The connection comes back: the saved copy is read again at once,
|
||||
// and polling goes on once the server answers.
|
||||
fromCache = false
|
||||
setOnline(true)
|
||||
await waitFor(() => expect(reads()).toBe(2))
|
||||
await tick()
|
||||
await waitFor(() => expect(reads()).toBe(3))
|
||||
})
|
||||
|
||||
it('stops polling on a saved copy even while the browser says it is online', async () => {
|
||||
fromCache = true
|
||||
renderAt(['/dinner/d1'])
|
||||
expect(await screen.findByText(/Still finding the recipe/)).toBeInTheDocument()
|
||||
|
||||
await tick(3)
|
||||
expect(reads()).toBe(1)
|
||||
})
|
||||
|
||||
it('stops polling when the page is left', async () => {
|
||||
const router = renderAt(['/dinner/d1'])
|
||||
expect(await screen.findByText(/Still finding the recipe/)).toBeInTheDocument()
|
||||
expect(reads()).toBe(1)
|
||||
|
||||
await act(() => router.navigate('/'))
|
||||
expect(screen.getByRole('heading', { name: 'Home probe' })).toBeInTheDocument()
|
||||
await tick(3)
|
||||
expect(reads()).toBe(1)
|
||||
})
|
||||
})
|
||||
|
||||
it('says an unknown or removed dinner is not found', async () => {
|
||||
stubFetchByPath({ [DINNER]: () => errorResponse(404, 'not_found') })
|
||||
renderAt(['/dinner/d1'])
|
||||
|
||||
@@ -12,7 +12,7 @@ import { Photo } from '../ui/Photo.tsx'
|
||||
import { Skeleton } from '../ui/Skeleton.tsx'
|
||||
import { Tag } from '../ui/Tag.tsx'
|
||||
import type { Difficulty } from './api.ts'
|
||||
import { useDinnerDetail, type DinnerDetail, type Ingredient } from './detail.ts'
|
||||
import { useDinnerDetail, type DinnerDetail, type Ingredient, type RecipeSource } from './detail.ts'
|
||||
import styles from './DinnerDetailPage.module.css'
|
||||
|
||||
// REGULAR_DINNERS is Settings → Regular dinners, where a recipe is fixed.
|
||||
@@ -58,6 +58,23 @@ function IngredientRow({ ingredient }: { ingredient: Ingredient }) {
|
||||
)
|
||||
}
|
||||
|
||||
// SourceCredit is "Recipe from {site}": a link opening the recipe's page in
|
||||
// a new tab when the server sends a url safe to open, else plain text.
|
||||
function SourceCredit({ siteName, url }: RecipeSource & { siteName: string }) {
|
||||
const credit = `Recipe from ${siteName}`
|
||||
return (
|
||||
<p className={styles.source}>
|
||||
{url === null ? (
|
||||
credit
|
||||
) : (
|
||||
<a href={url} target="_blank" rel="noopener noreferrer">
|
||||
{credit}
|
||||
</a>
|
||||
)}
|
||||
</p>
|
||||
)
|
||||
}
|
||||
|
||||
// Recipe is the dinner's ingredients and steps, or what stands in for them:
|
||||
// still searching, or the link to fix a recipe that could not be found.
|
||||
function Recipe({ dinner }: { dinner: DinnerDetail }) {
|
||||
@@ -101,12 +118,16 @@ function Recipe({ dinner }: { dinner: DinnerDetail }) {
|
||||
</ol>
|
||||
</section>
|
||||
)}
|
||||
{dinner.siteName !== null && <p className={styles.source}>Recipe from {dinner.siteName}</p>}
|
||||
{dinner.source !== null && dinner.source.siteName !== null && (
|
||||
<SourceCredit siteName={dinner.source.siteName} url={dinner.source.url} />
|
||||
)}
|
||||
</>
|
||||
)
|
||||
}
|
||||
|
||||
// ActionBar is the bar held at the bottom of the screen: Add to this week
|
||||
// ActionBar is the bar at the bottom of the screen, the page's last child:
|
||||
// the page fills the scroll area so a short recipe still leaves it at the
|
||||
// bottom, and it sticks there over a long one. It holds Add to this week
|
||||
// and the favorite heart, showing whether the dinner is a favorite. Both
|
||||
// rest unavailable until the next update wires them.
|
||||
function ActionBar({ dinner }: { dinner: DinnerDetail }) {
|
||||
|
||||
+63
-20
@@ -1,4 +1,5 @@
|
||||
import { useQuery } from '@tanstack/react-query'
|
||||
import { useEffect, useSyncExternalStore } from 'react'
|
||||
import { useApi } from '../api/index.ts'
|
||||
import { isRecord } from '../api/errors.ts'
|
||||
import {
|
||||
@@ -31,11 +32,17 @@ export type Ingredient = {
|
||||
raw_text: string
|
||||
}
|
||||
|
||||
// RecipeSource is where a fetched recipe came from. siteName is the site,
|
||||
// or null when there is none to name: a page that named no site, or a name
|
||||
// the server withheld because it could have come from the link. url is the
|
||||
// recipe's page, or null: today's API never sends the stored link, which can
|
||||
// carry a token, so the credit is text until the server sends a url it has
|
||||
// checked is free of secrets.
|
||||
export type RecipeSource = { siteName: string | null; url: string | null }
|
||||
|
||||
// DinnerDetail is a dinner with its recipe (GET /api/v1/dinners/{id}).
|
||||
// siteName is the site the recipe was fetched from, or null when there is
|
||||
// none to name: a recipe typed in, a page that named no site, or a name the
|
||||
// server withheld because it could have come from the link. The recipe's
|
||||
// link itself never reaches the client.
|
||||
// source is null for a recipe typed in. cached says the service worker
|
||||
// answered from its saved copy because the server could not be reached.
|
||||
export type DinnerDetail = {
|
||||
id: string
|
||||
name: string
|
||||
@@ -47,9 +54,10 @@ export type DinnerDetail = {
|
||||
photo_id: string | undefined
|
||||
ingredients: Ingredient[]
|
||||
steps: string[]
|
||||
siteName: string | null
|
||||
source: RecipeSource | null
|
||||
grounding_status: GroundingStatus
|
||||
why_picked: string | null
|
||||
cached: boolean
|
||||
}
|
||||
|
||||
function text(where: string, field: string, value: unknown): string {
|
||||
@@ -84,14 +92,32 @@ function detailPhotoId(where: string, value: unknown): string | undefined {
|
||||
return id
|
||||
}
|
||||
|
||||
function siteName(where: string, value: unknown): string | null {
|
||||
if (value === null) return null
|
||||
if (!isRecord(value)) throw new Error(`${where}: source is not an object`)
|
||||
return nullableText(where, 'source site_name', value.site_name)
|
||||
// sourceUrl is source.url, which today's API does not send, or a link safe
|
||||
// to open: https, with no user, password, query or fragment.
|
||||
function sourceUrl(where: string, value: unknown): string | null {
|
||||
if (value === undefined || value === null) return null
|
||||
const href = text(where, 'source url', value)
|
||||
let link: URL
|
||||
try {
|
||||
link = new URL(href)
|
||||
} catch (error) {
|
||||
throw new Error(`${where}: source url is not a URL`, { cause: error })
|
||||
}
|
||||
if (link.protocol !== 'https:' || link.username !== '' || link.password !== '' || link.search !== '' || link.hash !== '') {
|
||||
throw new Error(`${where}: source url is not an https link without credentials, query or fragment`)
|
||||
}
|
||||
return link.href
|
||||
}
|
||||
|
||||
// parseDinnerDetail checks a dinner with its recipe as the API sends it.
|
||||
export function parseDinnerDetail(where: string, body: unknown): DinnerDetail {
|
||||
function parseSource(where: string, value: unknown): RecipeSource | null {
|
||||
if (value === null) return null
|
||||
if (!isRecord(value)) throw new Error(`${where}: source is not an object`)
|
||||
return { siteName: nullableText(where, 'source site_name', value.site_name), url: sourceUrl(where, value.url) }
|
||||
}
|
||||
|
||||
// parseDinnerDetail checks a dinner with its recipe as the API sends it;
|
||||
// cached says whether the service worker answered from its saved copy.
|
||||
export function parseDinnerDetail(where: string, body: unknown, cached: boolean): DinnerDetail {
|
||||
if (!isRecord(body) || typeof body.id !== 'string' || typeof body.name !== 'string') {
|
||||
throw new Error(`${where}: response is not a dinner`)
|
||||
}
|
||||
@@ -112,24 +138,41 @@ export function parseDinnerDetail(where: string, body: unknown): DinnerDetail {
|
||||
photo_id: detailPhotoId(where, body.photo_urls),
|
||||
ingredients: body.ingredients.map((ingredient) => parseIngredient(where, ingredient)),
|
||||
steps: body.steps.map((step) => text(where, 'step', step)),
|
||||
siteName: siteName(where, body.source),
|
||||
source: parseSource(where, body.source),
|
||||
grounding_status: oneOf(where, 'grounding_status', GROUNDING_STATUSES, body.grounding_status),
|
||||
why_picked: nullableText(where, 'why_picked', body.why_picked),
|
||||
cached,
|
||||
}
|
||||
}
|
||||
|
||||
// useDinnerDetail is dinner id with its recipe (GET /api/v1/dinners/{id}).
|
||||
// While the dinner is still finding its recipe it is read again every
|
||||
// GROUNDING_POLL_MS. An unknown or archived dinner fails with the API's 404.
|
||||
// It runs even while the browser reports offline, so the copy the service
|
||||
// worker saved is shown, or the read fails, instead of waiting for the
|
||||
// network.
|
||||
// GROUNDING_POLL_MS, but only while the server can be reached: polling pauses
|
||||
// while the browser is offline or the last read was the service worker's
|
||||
// saved copy, which would only answer the same again, and a saved pending
|
||||
// copy is read once more when the connection comes back. Polling stops on
|
||||
// unmount. An unknown or archived dinner fails with the API's 404. It runs
|
||||
// even while the browser reports offline, so the copy the service worker
|
||||
// saved is shown, or the read fails, instead of waiting for the network.
|
||||
export function useDinnerDetail(id: string) {
|
||||
const { apiFetch } = useApi()
|
||||
return useQuery({
|
||||
const { apiRead, network } = useApi()
|
||||
const offline = useSyncExternalStore(network.subscribe, network.offline)
|
||||
const query = useQuery({
|
||||
queryKey: ['dinners', 'detail', id],
|
||||
queryFn: async () => parseDinnerDetail(`GET ${DINNERS_PATH}/{id}`, await apiFetch<unknown>(dinnerPath(id))),
|
||||
queryFn: async () => {
|
||||
const read = await apiRead<unknown>(dinnerPath(id))
|
||||
return parseDinnerDetail(`GET ${DINNERS_PATH}/{id}`, read.body, read.cached)
|
||||
},
|
||||
networkMode: 'always',
|
||||
refetchInterval: (query) => (query.state.data?.grounding_status === 'pending' ? GROUNDING_POLL_MS : false),
|
||||
refetchInterval: (query) => {
|
||||
const dinner = query.state.data
|
||||
return dinner?.grounding_status === 'pending' && !dinner.cached && !offline ? GROUNDING_POLL_MS : false
|
||||
},
|
||||
})
|
||||
const { data, refetch } = query
|
||||
const stalled = data?.grounding_status === 'pending' && data.cached
|
||||
useEffect(() => {
|
||||
if (stalled && !offline) void refetch()
|
||||
}, [stalled, offline, refetch])
|
||||
return query
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user