diff --git a/web/src/dinners/DinnerDetailPage.module.css b/web/src/dinners/DinnerDetailPage.module.css index 4998f6e..dd65df2 100644 --- a/web/src/dinners/DinnerDetailPage.module.css +++ b/web/src/dinners/DinnerDetailPage.module.css @@ -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; diff --git a/web/src/dinners/DinnerDetailPage.test.tsx b/web/src/dinners/DinnerDetailPage.test.tsx index 1be3fad..5e93d46 100644 --- a/web/src/dinners/DinnerDetailPage.test.tsx +++ b/web/src/dinners/DinnerDetailPage.test.tsx @@ -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: }, + { element: , children: [{ path: '/dinner/:id', element: }] }, { path: '/', element:

Home probe

}, { path: '/plan', element:

Plan probe

}, { path: REGULAR_DINNERS, element:

Regular dinners probe

}, @@ -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 + + 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']) diff --git a/web/src/dinners/DinnerDetailPage.tsx b/web/src/dinners/DinnerDetailPage.tsx index 86c3219..afbf78e 100644 --- a/web/src/dinners/DinnerDetailPage.tsx +++ b/web/src/dinners/DinnerDetailPage.tsx @@ -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 ( +

+ {url === null ? ( + credit + ) : ( + + {credit} + + )} +

+ ) +} + // 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 }) { )} - {dinner.siteName !== null &&

Recipe from {dinner.siteName}

} + {dinner.source !== null && dinner.source.siteName !== null && ( + + )} ) } -// 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 }) { diff --git a/web/src/dinners/detail.ts b/web/src/dinners/detail.ts index c6f4d85..7e8ff2f 100644 --- a/web/src/dinners/detail.ts +++ b/web/src/dinners/detail.ts @@ -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(dinnerPath(id))), + queryFn: async () => { + const read = await apiRead(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 }