diff --git a/docs/sustainability.md b/docs/sustainability.md index 51cb930..401597d 100644 --- a/docs/sustainability.md +++ b/docs/sustainability.md @@ -49,7 +49,10 @@ repeat a query. So positions are rounded to 3 decimals (about 100 m) and cached for the visit, and pinning the same spot again sends nothing. A new pin cancels an unfinished lookup, requests are at least 1 s apart, and typed - coordinates send nothing. + coordinates send nothing. The place search on the same page follows the + same policy: it searches only when the user presses Enter or the button, + never while typing, shares the one request per second with the pin lookups + and caches answers for the visit, ignoring case and extra spaces. - **CI does each thing once.** Every CI run uses CPU and network on the VM, so it skips work that was already done: - A pull request run is cancelled when a newer commit replaces it. diff --git a/frontend/e2e/mock-geocoding.ts b/frontend/e2e/mock-geocoding.ts index 64711d9..e225788 100644 --- a/frontend/e2e/mock-geocoding.ts +++ b/frontend/e2e/mock-geocoding.ts @@ -20,3 +20,27 @@ export async function mockGeocoding(page: Page, address: NominatimAddress) { return lookups } + +/** A match of the place search, as Nominatim answers it. */ +export interface MockSearchResult { + place_id: number + display_name: string + lat: string + lon: string + address: NominatimAddress +} + +/** + * Answers the submit form's place search with `results` for any query. + * Returns the queries that were sent. + */ +export async function mockPlaceSearch(page: Page, results: MockSearchResult[]) { + const queries: string[] = [] + + await page.route('**/nominatim.openstreetmap.org/search?**', (route) => { + queries.push(new URL(route.request().url()).searchParams.get('q') ?? '') + return route.fulfill({ json: results }) + }) + + return queries +} diff --git a/frontend/e2e/submit-sighting.spec.ts b/frontend/e2e/submit-sighting.spec.ts index 11c6c69..f6ad151 100644 --- a/frontend/e2e/submit-sighting.spec.ts +++ b/frontend/e2e/submit-sighting.spec.ts @@ -1,6 +1,6 @@ import { expect, test, type Locator, type Page } from '@playwright/test' import type { CreateSightingInput } from '../src/graphql/mutations' -import { mockGeocoding } from './mock-geocoding' +import { mockGeocoding, mockPlaceSearch } from './mock-geocoding' import { answerOnce, mockGraphql } from './mock-graphql' // Fields by their labels, as a keyboard or screen reader user finds them. @@ -11,7 +11,8 @@ function form(page: Page) { country: page.getByLabel('Country'), state: page.getByLabel('State'), shape: page.getByLabel('Shape'), - duration: page.getByLabel('Duration in seconds'), + duration: page.getByLabel(/^Duration/), + durationUnit: page.getByLabel('Unit'), latitude: page.getByLabel('Latitude'), longitude: page.getByLabel('Longitude'), description: page.getByLabel('Description'), @@ -70,6 +71,17 @@ test('fill in and submit the form with the keyboard', async ({ page }) => { await page.keyboard.type('1045AM') await expect(fields.datetime).toHaveValue('2003-03-03T10:45') + // A closed select picks the option that starts with what is typed. + await tabTo(page, fields.shape) + await page.keyboard.type('sphere') + await expect(fields.shape).toHaveValue('sphere') + + await tabTo(page, fields.duration) + await page.keyboard.type('90') + // The unit select comes next. A closed select picks by typed prefix. + await tabTo(page, fields.durationUnit) + await page.keyboard.type('sec') + await tabTo(page, fields.city) await page.keyboard.type('Trondheim') @@ -82,13 +94,6 @@ test('fill in and submit the form with the keyboard', async ({ page }) => { // Only the US has states. await expect(fields.state).toBeHidden() - // A closed select picks the option that starts with what is typed. - await tabTo(page, fields.shape) - await page.keyboard.type('sphere') - await expect(fields.shape).toHaveValue('sphere') - - await tabTo(page, fields.duration) - await page.keyboard.type('90') await tabTo(page, fields.latitude) await page.keyboard.type('63.4305') await tabTo(page, fields.longitude) @@ -162,6 +167,65 @@ test('pinning a location fills in the city, state and country', async ({ ) }) +test('a place can be found and set with the keyboard alone', async ({ + page, +}) => { + const queries = await mockPlaceSearch(page, [ + { + place_id: 1, + display_name: 'Roswell, Chaves County, New Mexico, United States', + lat: '33.3943', + lon: '-104.5230', + address: { + town: 'Roswell', + 'ISO3166-2-lvl4': 'US-NM', + country_code: 'us', + }, + }, + { + place_id: 2, + display_name: 'Roswell, Fulton County, Georgia, United States', + lat: '34.0232', + lon: '-84.3616', + address: { + city: 'Roswell', + 'ISO3166-2-lvl4': 'US-GA', + country_code: 'us', + }, + }, + ]) + const fields = form(page) + const search = page.getByRole('combobox', { name: 'Search for a place' }) + // The shape select has options too, so only look inside the results. + const results = page.getByRole('listbox').getByRole('option') + + await tabTo(page, search) + await page.keyboard.type('Roswell') + await page.keyboard.press('Enter') + + await expect(results).toHaveCount(2) + await expect( + page.getByRole('status').filter({ hasText: '2 places found' }), + ).toBeVisible() + expect(queries).toEqual(['roswell']) + + await page.keyboard.press('ArrowDown') + await page.keyboard.press('Enter') + + await expect(fields.city).toHaveValue('Roswell') + await expect(fields.country).toHaveValue('United States') + await expect(fields.state).toHaveValue('NM') + await expect(fields.latitude).toHaveValue('33.394300') + await expect(fields.longitude).toHaveValue('-104.523000') + await expect(page.getByRole('listbox')).toHaveCount(0) + await expect( + page.getByRole('status').filter({ hasText: 'Location set' }), + ).toBeVisible() + // The marker is on the map, and Enter did not submit the form. + await expect(page.getByAltText('Selected location')).toBeVisible() + await expect(page.getByRole('alert')).toHaveCount(0) +}) + test('empty required fields show errors next to each field', async ({ page, }) => { @@ -181,10 +245,10 @@ test('empty required fields show errors next to each field', async ({ // "next to" the field for screen reader users. const expectedErrors: [Locator, string][] = [ [fields.datetime, 'Enter when you saw the object.'], + [fields.shape, 'Choose the closest shape.'], + [fields.duration, 'Enter how long it lasted, at least 1 second.'], [fields.city, 'Enter the city.'], [fields.country, 'Enter the country.'], - [fields.shape, 'Choose the closest shape.'], - [fields.duration, 'Enter a whole number of seconds, at least 1.'], [fields.latitude, 'Enter a latitude between -90 and 90.'], [fields.longitude, 'Enter a longitude between -180 and 180.'], [fields.description, 'Describe what you saw.'], @@ -206,8 +270,7 @@ test('empty required fields show errors next to each field', async ({ test('errors from the server show next to the right field', async ({ page, }) => { - // Only the server checks that the date is not in the future, and it - // answers with BAD_USER_INPUT and a list of field errors. + // The server has the final say on every rule, and it answers with BAD_USER_INPUT and a list of field errors. await answerOnce(page, 'CreateSighting', { json: { data: null, @@ -219,7 +282,7 @@ test('errors from the server show next to the right field', async ({ fieldErrors: [ { field: 'datetime', - message: 'datetime cannot be in the future.', + message: 'datetime is not a valid date and time.', }, ], }, @@ -229,7 +292,7 @@ test('errors from the server show next to the right field', async ({ }) const fields = form(page) - await fields.datetime.fill('2099-01-01T12:00') + await fields.datetime.fill('2003-03-03T12:00') await fields.city.fill('Bergen') await fields.country.fill('Norway') await fields.shape.selectOption('light') @@ -244,7 +307,7 @@ test('errors from the server show next to the right field', async ({ ) await expect(fields.datetime).toHaveAttribute('aria-invalid', 'true') await expect(fields.datetime).toHaveAccessibleDescription( - 'datetime cannot be in the future.', + 'datetime is not a valid date and time.', ) // Only the field the server named is marked. await expect(fields.city).toHaveAttribute('aria-invalid', 'false') @@ -260,3 +323,42 @@ test('errors from the server show next to the right field', async ({ page.getByRole('heading', { level: 1, name: 'Sighting submitted' }), ).toBeVisible() }) + +test('the error summary links to each invalid field', async ({ page }) => { + const fields = form(page) + await fields.submit.click() + + const summaryLinks = page.locator('.submit-sighting__error-summary a') + await expect(summaryLinks).toHaveCount(8) + await expect(summaryLinks.first()).toHaveText( + 'Date and time: Enter when you saw the object.', + ) + // Focus stays on the first invalid field, and Enter on a link moves it to + // that link's field. + await expect(fields.datetime).toBeFocused() + await page.getByRole('link', { name: /^Latitude:/ }).focus() + await page.keyboard.press('Enter') + await expect(fields.latitude).toBeFocused() +}) + +test('a future date is rejected before anything is sent', async ({ page }) => { + const fields = form(page) + await fields.datetime.fill('2099-01-01T12:00') + await fields.city.focus() + + await expect(fields.datetime).toHaveAccessibleDescription( + 'The date and time cannot be in the future.', + ) +}) + +test('a year before 1900 is rejected when leaving the field', async ({ + page, +}) => { + const fields = form(page) + await fields.datetime.fill('1850-01-01T12:00') + await fields.city.focus() + + await expect(fields.datetime).toHaveAccessibleDescription( + 'The year must be 1900 or later.', + ) +}) diff --git a/frontend/src/components/ErrorSummary.tsx b/frontend/src/components/ErrorSummary.tsx new file mode 100644 index 0000000..34d0aad --- /dev/null +++ b/frontend/src/components/ErrorSummary.tsx @@ -0,0 +1,44 @@ +import type { MouseEvent } from 'react' +import { fieldLabels, type FormErrors } from '../utils/sighting-form' + +interface ErrorSummaryProps { + errors: FormErrors +} + +/** + * Lists every invalid field with its error, each a link that moves focus to + * the field. Placed by the submit button, where the user just clicked, while + * focus goes to the first invalid field. + */ +function ErrorSummary({ errors }: ErrorSummaryProps) { + const items = fieldLabels.filter(([field]) => errors[field]) + if (items.length === 0) return null + + // The links hold a hash, which the router would take for a page. + function focusField(event: MouseEvent, id: string) { + event.preventDefault() + document.getElementById(id)?.focus() + } + + return ( +
+

+ Please correct the highlighted fields. +

+ +
+ ) +} + +export default ErrorSummary diff --git a/frontend/src/components/FormField.tsx b/frontend/src/components/FormField.tsx new file mode 100644 index 0000000..79a3bed --- /dev/null +++ b/frontend/src/components/FormField.tsx @@ -0,0 +1,39 @@ +import type { ReactNode } from 'react' + +interface FormFieldProps { + /** The id of the control inside. The error gets `${id}-error`. */ + id: string + label: string + required?: boolean + error?: string + children: ReactNode +} + +/** A label, a control and the error message under it. */ +export function FormField({ + id, + label, + required, + error, + children, +}: FormFieldProps) { + return ( +
+ + {children} + {error && ( +

+ {error} +

+ )} +
+ ) +} diff --git a/frontend/src/components/LocationPicker.tsx b/frontend/src/components/LocationPicker.tsx index 1f6ddf0..7db4d15 100644 --- a/frontend/src/components/LocationPicker.tsx +++ b/frontend/src/components/LocationPicker.tsx @@ -1,5 +1,12 @@ import L from 'leaflet' -import { MapContainer, Marker, TileLayer, useMapEvents } from 'react-leaflet' +import { + MapContainer, + Marker, + TileLayer, + useMap, + useMapEvents, +} from 'react-leaflet' +import { useEffect } from 'react' import 'leaflet/dist/leaflet.css' import MapLabel from './MapLabel' import './LocationPicker.css' @@ -20,6 +27,21 @@ const marker = L.icon({ interface LocationPickerProps { position: [number, number] | null onPositionChange: (position: [number, number]) => void + /** + * Moves the map to a place chosen outside it, such as from the place + * search. A new object moves it again, even to the same place. + */ + showPlace?: { position: [number, number] } | null +} + +// The map is only moved for places found by name. For a click the marker is +// already under the pointer, and moving the map would shift it away. +function MapMover({ target }: { target: { position: [number, number] } }) { + const map = useMap() + useEffect(() => { + map.flyTo(target.position, 10) + }, [map, target]) + return null } function MapClickHandler({ @@ -31,7 +53,11 @@ function MapClickHandler({ return null } -function LocationPicker({ position, onPositionChange }: LocationPickerProps) { +function LocationPicker({ + position, + onPositionChange, + showPlace, +}: LocationPickerProps) { return (
+ {showPlace && } {/* Clicking the marker does nothing, so it is not made a button. */} {position && ( void + onChange: (field: keyof FormValues, value: string) => void + onPositionChange: (position: [number, number]) => void + onPlaceSelect: (result: PlaceResult) => void + shownPlace: { position: [number, number] } | null + lookupStatus: LookupStatus + isUnitedStates: boolean +} + +/** The place of the sighting: typed fields on one side, a map on the other. */ +function LocationSection({ + values, + errors, + mode, + onModeChange, + onChange, + onPositionChange, + onPlaceSelect, + shownPlace, + lookupStatus, + isUnitedStates, +}: LocationSectionProps) { + return ( +
+
+

Where did it happen?

+

+ Search for the place, click the map, or fill it in yourself. +

+
+ +
+ + +
+ + + +
+
+ + onChange('city', event.target.value)} + required + /> + + + {/* The datalist suggests names as the user types. The browser's + own autocomplete is off so its history doesn't crowd them. */} + onChange('country', event.target.value)} + required + /> + + {countries.map(({ code, name }) => ( + + + {/* After the country in the page order, so it appears where the + keyboard focus goes next once "United States" is chosen. */} + {isUnitedStates && ( + + onChange('state', event.target.value)} + /> + + )} +
+ + onChange('latitude', event.target.value)} + required + /> + + + onChange('longitude', event.target.value)} + required + /> + +
+
+
+ +

+ Place names from{' '} + OpenStreetMap{' '} + via Nominatim. +

+ {(values.city || (values.latitude && values.longitude)) && ( +

+ Pinned location:{' '} + + {values.city || `${values.latitude}, ${values.longitude}`} + {values.state ? `, ${values.state}` : ''} + {values.country ? `, ${values.country}` : ''} + +

+ )} + {/* Always rendered, so screen readers announce changes to it. */} +

+ {lookupStatus === 'loading' && 'Looking up the place…'} + {lookupStatus === 'failed' && + "Couldn't find the place. Fill in the fields manually."} +

+
+
+
+ ) +} + +export default LocationSection diff --git a/frontend/src/components/PlaceSearch.tsx b/frontend/src/components/PlaceSearch.tsx new file mode 100644 index 0000000..0538d88 --- /dev/null +++ b/frontend/src/components/PlaceSearch.tsx @@ -0,0 +1,131 @@ +import { useId, useState, type KeyboardEvent } from 'react' +import { usePlaceSearch } from '../hooks/use-place-search' +import type { PlaceResult } from '../utils/place-search' + +interface PlaceSearchProps { + onSelect: (result: PlaceResult) => void +} + +/** + * Finds a place by name, as a keyboard alternative to clicking the map. It + * follows the ARIA combobox pattern: focus stays in the input while the + * arrow keys move through the results, and Enter chooses one. Enter without + * a highlighted result, or the button, starts the search. It searches only + * on request, since Nominatim does not allow searching on every keystroke. + */ +function PlaceSearch({ onSelect }: PlaceSearchProps) { + const id = useId() + const listId = `${id}-results` + const hintId = `${id}-hint` + const [query, setQuery] = useState('') + const [chosen, setChosen] = useState() + const { status, results, activeIndex, search, moveActive, close } = + usePlaceSearch() + const open = status === 'done' && results.length > 0 + + function choose(result: PlaceResult) { + onSelect(result) + setChosen(result.name) + close() + } + + function startSearch() { + setChosen(undefined) + void search(query) + } + + function handleKeyDown(event: KeyboardEvent) { + if (event.key === 'ArrowDown' || event.key === 'ArrowUp') { + if (!open) return + event.preventDefault() + moveActive(event.key === 'ArrowDown' ? 1 : -1) + } else if (event.key === 'Enter') { + // The input is inside the report form, which Enter would submit. + event.preventDefault() + if (open && activeIndex >= 0) choose(results[activeIndex]) + else startSearch() + } else if (event.key === 'Escape' && (open || status === 'loading')) { + event.preventDefault() + close() + } + } + + // Read out by screen readers when it changes, so it is always rendered. + let message = '' + if (status === 'loading') message = 'Searching…' + else if (status === 'failed') { + message = "Couldn't search for places. Try again, or click the map." + } else if (status === 'done' && results.length === 0) { + message = 'No places found. Try another name, or click the map.' + } else if (open) { + message = `${results.length} ${results.length === 1 ? 'place' : 'places'} found.` + } else if (chosen) { + message = `Location set to ${chosen}.` + } + + return ( +
+ +
+ = 0 + ? `${listId}-option-${activeIndex}` + : undefined + } + aria-describedby={open ? undefined : hintId} + value={query} + onChange={(event) => setQuery(event.target.value)} + onKeyDown={handleKeyDown} + /> + +
+ {/* Gone once there are results, which have their own instructions. */} + {!open && ( +

+ For example "Trondheim". Press Enter to search. +

+ )} + {open && ( +
    + {results.map((result, index) => ( +
  • event.preventDefault()} + onClick={() => choose(result)} + > + {result.name} +
    • + ))} +
+ )} +

+ {message} +

+
+ ) +} + +export default PlaceSearch diff --git a/frontend/src/hooks/use-place-search.ts b/frontend/src/hooks/use-place-search.ts new file mode 100644 index 0000000..92e9576 --- /dev/null +++ b/frontend/src/hooks/use-place-search.ts @@ -0,0 +1,68 @@ +import { useEffect, useRef, useState } from 'react' +import { searchPlaces, type PlaceResult } from '../utils/place-search' + +// Long enough for a slow connection, short enough that the user isn't +// left waiting without being told it failed. +const SEARCH_TIMEOUT_MS = 8000 + +export type SearchStatus = 'idle' | 'loading' | 'done' | 'failed' + +/** + * Searches for places by name when `search` is called, and keeps the + * results and which one is highlighted for keyboard navigation. A new + * search cancels the one before it, so a slow answer for an old query + * can't replace the results. + */ +export function usePlaceSearch() { + const [status, setStatus] = useState('idle') + const [results, setResults] = useState([]) + // -1 means none, so Enter searches again until an arrow key picks one. + const [activeIndex, setActiveIndex] = useState(-1) + const controllerRef = useRef(null) + + useEffect(() => () => controllerRef.current?.abort(), []) + + async function search(query: string) { + if (!query.trim()) return + controllerRef.current?.abort() + const controller = new AbortController() + controllerRef.current = controller + setStatus('loading') + setResults([]) + setActiveIndex(-1) + const timeout = setTimeout(() => controller.abort(), SEARCH_TIMEOUT_MS) + + try { + const found = await searchPlaces(query, controller.signal) + if (controllerRef.current !== controller) return + setResults(found) + setStatus('done') + } catch { + // A newer search aborted this one and reports its own status. + if (controllerRef.current !== controller) return + setStatus('failed') + } finally { + clearTimeout(timeout) + } + } + + /** Moves the highlight, wrapping around at both ends. */ + function moveActive(step: 1 | -1) { + if (results.length === 0) return + setActiveIndex((current) => { + if (current === -1) return step === 1 ? 0 : results.length - 1 + return (current + step + results.length) % results.length + }) + } + + /** Hides the results and cancels a search that is still running. */ + function close() { + controllerRef.current?.abort() + controllerRef.current = null + setStatus('idle') + setResults([]) + setActiveIndex(-1) + } + + return { status, results, activeIndex, search, moveActive, close } +} diff --git a/frontend/src/hooks/use-sighting-form.ts b/frontend/src/hooks/use-sighting-form.ts new file mode 100644 index 0000000..32e2a8f --- /dev/null +++ b/frontend/src/hooks/use-sighting-form.ts @@ -0,0 +1,203 @@ +import { useMutation } from '@apollo/client/react' +import { useEffect, useRef, useState, type FormEvent } from 'react' +import { CREATE_SIGHTING_MUTATION } from '../graphql/mutations' +import { countryCodeFromName, countryNameFromCode } from '../utils/countries' +import type { PlaceResult } from '../utils/place-search' +import type { Place } from '../utils/reverse-geocode' +import { + datetimeError, + durationInSeconds, + hasLocationError, + initialValues, + placeFields, + serverFieldErrors, + validate, + type FormErrors, + type FormValues, + type PlaceField, +} from '../utils/sighting-form' +import { useReverseGeocode } from './use-reverse-geocode' + +export type LocationMode = 'map' | 'manual' + +/** + * State and behavior of the report form: the values, validation, the + * submit and what happens to focus afterwards. The page only renders it. + */ +export function useSightingForm() { + const [values, setValues] = useState(initialValues) + const [errors, setErrors] = useState({}) + const [submittedId, setSubmittedId] = useState() + const [saveFailed, setSaveFailed] = useState(false) + const [locationMode, setLocationMode] = useState('map') + // Set when a place is found by name, so the map moves to it. + const [shownPlace, setShownPlace] = useState<{ + position: [number, number] + } | null>(null) + const [createSighting, { loading }] = useMutation(CREATE_SIGHTING_MUTATION) + // Only US sightings have a state. The imported data uses US state codes, + // and other countries' regions don't fit that field. + const isUnitedStates = countryCodeFromName(values.country) === 'us' + + const formRef = useRef(null) + const headingRef = useRef(null) + // Set by the handlers below, so focus only moves in response to the user, + // never on the first render. + const focusFirstError = useRef(false) + const focusHeading = useRef(false) + + // After a failed submit, screen readers hear the first invalid field + // together with its error, and keyboard users start fixing it right away. + // Runs after render so the field already has aria-describedby. + useEffect(() => { + if (!focusFirstError.current) return + focusFirstError.current = false + formRef.current + ?.querySelector('[aria-invalid="true"]:not(:disabled)') + ?.focus() + }, [errors]) + + // Switching between the form and the confirmation removes the focused + // button, so focus goes to the new heading instead of the page body. + useEffect(() => { + if (!focusHeading.current) return + focusHeading.current = false + headingRef.current?.focus() + }, [submittedId]) + + function updateValue(field: keyof FormValues, value: string) { + setValues((previous) => ({ ...previous, [field]: value })) + setErrors((previous) => ({ ...previous, [field]: undefined })) + } + + // Tell the user as soon as they leave the date field, not only on submit. + // An empty value is left alone, since the browser reports one while the + // date is half typed. Typing clears the message again (see updateValue). + function checkDatetime(datetime: string) { + const message = datetime ? datetimeError(datetime) : undefined + if (message) setErrors((previous) => ({ ...previous, datetime: message })) + } + + // The pin decides the place. Whatever the fields held before, typed or + // from an earlier pin, belongs to the old location, so every field is + // replaced, and emptied where the lookup found nothing (e.g. at sea). + // The user can still edit the fields after the pin. + function fillPlace(place: Place | null) { + const found: Record = { + city: place?.city ?? '', + state: place?.state ?? '', + country: (place?.country && countryNameFromCode(place.country)) ?? '', + } + for (const field of placeFields) { + if (values[field] !== found[field]) updateValue(field, found[field]) + } + } + + const lookup = useReverseGeocode(fillPlace) + + // Only map clicks look up the place. Typed coordinates would send a + // request for every keystroke. + function updatePosition(position: [number, number]) { + const [latitude, longitude] = position + updateValue('latitude', latitude.toFixed(6)) + updateValue('longitude', longitude.toFixed(6)) + void lookup.lookUp(position) + } + + // A place found by name already has its address, so there is nothing to + // look up. Any lookup still running for an earlier pin is cancelled, so + // its answer can't overwrite the fields. + function selectPlace({ position, place }: PlaceResult) { + lookup.reset() + updateValue('latitude', position[0].toFixed(6)) + updateValue('longitude', position[1].toFixed(6)) + fillPlace(place) + setShownPlace({ position }) + } + + async function handleSubmit(event: FormEvent) { + event.preventDefault() + if (loading) return + setSaveFailed(false) + const nextErrors = validate(values) + setErrors(nextErrors) + // The form shows country names, but the database stores codes. + // validate() has already reported a name that has no code. + const country = countryCodeFromName(values.country) + if (Object.keys(nextErrors).length > 0 || country === null) { + if (hasLocationError(nextErrors) || country === null) { + setLocationMode('manual') + } + focusFirstError.current = true + return + } + + try { + const { data } = await createSighting({ + // The cached lists don't contain the new sighting. Dropping them + // makes the search page fetch fresh results when it is opened. + update: (cache) => cache.evict({ fieldName: 'sightings' }), + variables: { + input: { + // datetime-local gives local time without timezone, which is + // the format the API stores. + datetime: values.datetime, + city: values.city.trim(), + // A state typed before switching to another country is hidden, + // so it isn't sent either. + state: isUnitedStates ? values.state.trim() || null : null, + country, + shape: values.shape, + durationSeconds: durationInSeconds(values), + comments: values.comments.trim(), + latitude: Number(values.latitude), + longitude: Number(values.longitude), + }, + }, + }) + if (!data) throw new Error('createSighting returned no data') + focusHeading.current = true + setSubmittedId(data.createSighting.id) + } catch (error) { + const fieldErrors = serverFieldErrors(error) + if (fieldErrors) { + if (hasLocationError(fieldErrors)) setLocationMode('manual') + focusFirstError.current = true + setErrors(fieldErrors) + } else { + setSaveFailed(true) + } + } + } + + function startOver() { + setValues(initialValues) + lookup.reset() + setLocationMode('map') + setShownPlace(null) + focusHeading.current = true + setSubmittedId(undefined) + } + + return { + values, + setValues, + errors, + submittedId, + saveFailed, + saving: loading, + locationMode, + setLocationMode, + isUnitedStates, + lookupStatus: lookup.status, + formRef, + headingRef, + updateValue, + checkDatetime, + updatePosition, + selectPlace, + shownPlace, + handleSubmit, + startOver, + } +} diff --git a/frontend/src/pages/SubmitSightingPage.css b/frontend/src/pages/SubmitSightingPage.css index e82ebe7..2f7d683 100644 --- a/frontend/src/pages/SubmitSightingPage.css +++ b/frontend/src/pages/SubmitSightingPage.css @@ -1,54 +1,66 @@ .submit-sighting { - max-width: 720px; + max-width: 860px; margin: 0 auto; } -/* The same 24px below the back link as on the detail page. */ -.submit-sighting h1 { - margin: 24px 0 16px; -} +.submit-sighting__back { + justify-self: start; + padding: 7px 14px; + border-radius: 12px; + color: var(--accent-strong); + background: var(--accent-bg); + font-size: 15px; + font-weight: 600; + text-decoration: none; + transition: background-color 0.15s; -.submit-sighting__intro { - margin-bottom: 28px; -} + /* The same 24px below the back link as on the detail page. */ + .submit-sighting h1 { + margin: 24px 0 16px; + } -.submit-sighting__location { - display: grid; - gap: 12px; -} + .submit-sighting__intro { + margin-bottom: 28px; + } -.submit-sighting__location h2 { - margin-bottom: 4px; -} + &:hover { + background: var(--accent-bg-strong); + } -.submit-sighting__lookup, -.submit-sighting__attribution { - margin: 0; - font-size: 0.9em; -} + &:focus-visible { + outline: 2px solid var(--accent); + outline-offset: 2px; + } -/* Keeps the space when empty, so the page doesn't jump as it changes. */ -.submit-sighting__lookup { - min-height: 1.5em; + @media (forced-colors: active) { + border: 1px solid LinkText; + } + + @media (prefers-reduced-motion: reduce) { + transition: none; + } } -.submit-sighting__lookup--failed { - color: var(--error); +.submit-sighting h1 { + margin-bottom: 24px; } -.submit-sighting__attribution a { - color: var(--accent); +.submit-sighting__intro { + margin-bottom: 24px; } .submit-sighting__form { display: grid; - gap: 20px; + gap: 24px; } .submit-sighting__row { display: grid; grid-template-columns: repeat(2, minmax(0, 1fr)); gap: 20px; + /* Fields keep their own height. Stretched to the tallest one, a field + without a message under it would spread its label and input apart. */ + align-items: start; } .submit-sighting__field { @@ -56,6 +68,20 @@ gap: 6px; } +.submit-sighting__duration { + display: flex; + gap: 8px; +} + +.submit-sighting__field .submit-sighting__duration input { + flex: 1; + min-width: 0; +} + +.submit-sighting__field .submit-sighting__duration select { + width: auto; +} + .submit-sighting__field label { color: var(--text-h); font-weight: 600; @@ -67,7 +93,7 @@ box-sizing: border-box; width: 100%; border: 1px solid var(--border); - border-radius: 4px; + border-radius: 8px; padding: 10px 12px; color: var(--text-h); background: var(--bg); @@ -102,15 +128,156 @@ font-weight: 600; } +.submit-sighting__error-summary { + display: grid; + gap: 4px; + padding: 8px 12px; + font-size: 0.9em; + border: 1px solid var(--error); + border-radius: 8px; +} + +.submit-sighting__error-summary ul { + margin: 0; + padding-left: 18px; +} + +.submit-sighting__error-summary a { + display: inline-block; + /* Keeps wrapped links far enough apart to tap (WCAG target size). */ + padding-block: 4px; + color: var(--error); +} + +/* Where did it happen? section */ +.submit-sighting__location { + display: grid; + gap: 16px; + padding: 24px; + border: 1px solid var(--border); + border-radius: 14px; + background: var(--code-bg); +} + +.submit-sighting__location-header h2 { + margin: 0 0 4px; + font-size: 20px; + font-weight: 600; + color: var(--text-h); +} + +.submit-sighting__hint { + margin: 0; + color: var(--text); + font-size: 15px; +} + +.submit-sighting__location-toggle { + display: none; +} + +.place-search__row { + display: flex; + gap: 8px; +} + +.submit-sighting__field .place-search__row input { + flex: 1; + min-width: 0; +} + +.place-search__results { + margin: 0; + padding: 4px; + list-style: none; + border: 1px solid var(--border); + border-radius: 8px; + background: var(--bg); +} + +.place-search__results li { + padding: 8px 10px; + border-radius: 6px; + color: var(--text-h); + cursor: pointer; +} + +.place-search__results li:hover { + background: var(--accent-bg); +} + +/* The highlight follows the arrow keys, since focus stays in the input. */ +.place-search__results li[aria-selected='true'] { + background: var(--accent-bg-strong); + outline: 2px solid var(--accent); + outline-offset: -2px; +} + +.submit-sighting__location-grid { + display: grid; + grid-template-columns: minmax(0, 1fr) minmax(0, 1.1fr); + gap: 24px; + align-items: start; +} + +.submit-sighting__place-fields { + display: grid; + gap: 16px; +} + +.submit-sighting__row--coordinates { + gap: 12px; +} + +.submit-sighting__map-side { + display: grid; + gap: 10px; +} + +.submit-sighting__map-side .location-picker { + border-radius: 10px; + box-shadow: var(--shadow); +} + +.submit-sighting__picked-summary { + margin: 0; + font-size: 0.95em; + color: var(--text-h); +} + +.submit-sighting__lookup, +.submit-sighting__attribution { + margin: 0; + font-size: 0.9em; +} + +.submit-sighting__lookup { + min-height: 1.5em; +} + +.submit-sighting__lookup--failed { + color: var(--error); +} + +.submit-sighting__attribution a { + color: var(--accent); +} + +.submit-sighting__attribution { + justify-self: end; + text-align: right; +} + .submit-sighting__button, .submit-sighting button { justify-self: start; border: 0; - border-radius: 4px; + border-radius: 8px; padding: 11px 18px; color: var(--on-accent); background: var(--accent); font: inherit; + font-weight: 600; cursor: pointer; text-decoration: none; } @@ -131,8 +298,92 @@ background: var(--border); } +@media (min-width: 641px) { + .submit-sighting__picked-summary { + display: none; + } +} + @media (max-width: 640px) { + .submit-sighting__location { + padding: 16px; + } + .submit-sighting__row { grid-template-columns: 1fr; } + + .submit-sighting__row--coordinates { + grid-template-columns: 1fr 1fr; + } + + .submit-sighting__location-toggle { + display: flex; + gap: 8px; + } + + .submit-sighting .submit-sighting__toggle-btn { + flex: 1; + padding: 8px 12px; + border: 1px solid var(--border); + border-radius: 8px; + background: var(--bg); + color: var(--text-h); + font: inherit; + font-size: 14px; + font-weight: 600; + cursor: pointer; + text-align: center; + transition: + background-color 0.15s, + color 0.15s; + + &:hover { + background: var(--accent-bg); + } + + &:focus-visible { + outline: 2px solid var(--accent); + outline-offset: 2px; + } + + @media (forced-colors: active) { + border: 1px solid ButtonText; + } + } + + .submit-sighting .submit-sighting__toggle-btn--active { + background: var(--accent); + color: var(--on-accent); + border-color: var(--accent); + + &:hover { + background: var(--accent); + } + + @media (forced-colors: active) { + background: Highlight; + color: HighlightText; + } + } + + .submit-sighting__location-grid { + grid-template-columns: 1fr; + } + + .submit-sighting__location-grid[data-mode='map'] + .submit-sighting__place-fields { + display: none; + } + + .submit-sighting__location-grid[data-mode='manual'] + .submit-sighting__map-side { + display: none; + } +} + +@media (max-width: 400px) { + .submit-sighting__row--coordinates { + grid-template-columns: 1fr; + } } diff --git a/frontend/src/pages/SubmitSightingPage.tsx b/frontend/src/pages/SubmitSightingPage.tsx index 08bd82e..eaa0f16 100644 --- a/frontend/src/pages/SubmitSightingPage.tsx +++ b/frontend/src/pages/SubmitSightingPage.tsx @@ -1,25 +1,17 @@ -import { CombinedGraphQLErrors } from '@apollo/client' -import { useMutation, useQuery } from '@apollo/client/react' -import { useEffect, useRef, useState, type FormEvent } from 'react' +import { useQuery } from '@apollo/client/react' import { Link } from 'react-router' -import { - CREATE_SIGHTING_MUTATION, - type SightingFieldError, -} from '../graphql/mutations' +import ErrorSummary from '../components/ErrorSummary' +import { FormField } from '../components/FormField' +import { fieldProps } from '../utils/field-props' +import LocationSection from '../components/LocationSection' import BackLink from '../components/BackLink' -import LocationPicker from '../components/LocationPicker' import { SHAPES_QUERY } from '../graphql/queries' - -import { useReverseGeocode } from '../hooks/use-reverse-geocode' -import { - countries, - countryCodeFromName, - countryNameFromCode, -} from '../utils/countries' -import type { Place } from '../utils/reverse-geocode' import { useDocumentTitle } from '../hooks/use-document-title' +import { useSightingForm } from '../hooks/use-sighting-form' +import { durationUnits, type DurationUnit } from '../utils/sighting-form' import './SubmitSightingPage.css' +// Shown when the shapes can't be loaded, so the form still works. const fallbackShapes = [ 'circle', 'cylinder', @@ -33,100 +25,9 @@ const fallbackShapes = [ 'other', ] -interface FormValues { - datetime: string - city: string - state: string - country: string - shape: string - durationSeconds: string - comments: string - latitude: string - longitude: string -} - -type FormErrors = Partial> - -/** The fields a map pin can fill in. */ -const placeFields = ['city', 'state', 'country'] as const -type PlaceField = (typeof placeFields)[number] - -const initialValues: FormValues = { - datetime: '', - city: '', - state: '', - country: '', - shape: '', - durationSeconds: '', - comments: '', - latitude: '', - longitude: '', -} - -function validate(values: FormValues): FormErrors { - const errors: FormErrors = {} - const duration = Number(values.durationSeconds) - const latitude = Number(values.latitude) - const longitude = Number(values.longitude) - - if (!values.datetime) errors.datetime = 'Enter when you saw the object.' - if (!values.city.trim()) errors.city = 'Enter the city.' - if (!values.country.trim()) { - errors.country = 'Enter the country.' - } else if (!countryCodeFromName(values.country)) { - errors.country = 'Choose a country from the suggestions.' - } - if (!values.shape) errors.shape = 'Choose the closest shape.' - if (!values.durationSeconds || !Number.isInteger(duration) || duration < 1) { - errors.durationSeconds = 'Enter a whole number of seconds, at least 1.' - } - if (!values.comments.trim()) errors.comments = 'Describe what you saw.' - if ( - !values.latitude || - !Number.isFinite(latitude) || - latitude < -90 || - latitude > 90 - ) { - errors.latitude = 'Enter a latitude between -90 and 90.' - } - if ( - !values.longitude || - !Number.isFinite(longitude) || - longitude < -180 || - longitude > 180 - ) { - errors.longitude = 'Enter a longitude between -180 and 180.' - } - return errors -} - -/** - * Turns the server's BAD_USER_INPUT `fieldErrors` into form errors, so the - * rules only the server checks (dates in the future, text length) show up - * next to the right field. Returns null for any other kind of error. - */ -function serverFieldErrors(error: unknown): FormErrors | null { - if (!CombinedGraphQLErrors.is(error)) return null - const invalid = error.errors.find( - (graphQLError) => graphQLError.extensions?.code === 'BAD_USER_INPUT', - ) - const fieldErrors = invalid?.extensions?.fieldErrors as - SightingFieldError[] | undefined - if (!fieldErrors) return null - - const errors: FormErrors = {} - for (const { field, message } of fieldErrors) { - errors[field] = message - } - return errors -} - function SubmitSightingPage() { - const [values, setValues] = useState(initialValues) - const [errors, setErrors] = useState({}) - const [submittedId, setSubmittedId] = useState() - const [saveFailed, setSaveFailed] = useState(false) - const [createSighting, { loading }] = useMutation(CREATE_SIGHTING_MUTATION) + const { formRef, headingRef, ...form } = useSightingForm() + const { values, errors } = form const { data: shapeData, loading: shapesLoading, @@ -135,124 +36,12 @@ function SubmitSightingPage() { const shapes = shapeData?.shapes.length ? shapeData.shapes : fallbackShapes const shapesUnavailable = Boolean(shapesError) || shapeData?.shapes.length === 0 - // Fixing a field sets its error to undefined but keeps the key, so check - // the values rather than counting keys. - const hasErrors = Object.values(errors).some(Boolean) - // Only US sightings have a state. The imported data uses US state codes, - // and other countries' regions don't fit that field. - const isUnitedStates = countryCodeFromName(values.country) === 'us' - - useDocumentTitle(submittedId ? 'Sighting submitted' : 'Report a sighting') - - const formRef = useRef(null) - const headingRef = useRef(null) - // Set by the handlers below, so focus only moves in response to the user, - // never on the first render. - const focusFirstError = useRef(false) - const focusHeading = useRef(false) - - // After a failed submit, screen readers hear the first invalid field - // together with its error, and keyboard users start fixing it right away. - // Runs after render so the field already has aria-describedby. - useEffect(() => { - if (!focusFirstError.current) return - focusFirstError.current = false - formRef.current - ?.querySelector('[aria-invalid="true"]') - ?.focus() - }, [errors]) - - // Switching between the form and the confirmation removes the focused - // button, so focus goes to the new heading instead of the page body. - useEffect(() => { - if (!focusHeading.current) return - focusHeading.current = false - headingRef.current?.focus() - }, [submittedId]) - function updateValue(field: keyof FormValues, value: string) { - setValues((previous) => ({ ...previous, [field]: value })) - setErrors((previous) => ({ ...previous, [field]: undefined })) - } - - // The pin decides the place. Whatever the fields held before, typed or - // from an earlier pin, belongs to the old location, so every field is - // replaced, and emptied where the lookup found nothing (e.g. at sea). - // The user can still edit the fields after the pin. - function fillPlace(place: Place | null) { - const found: Record = { - city: place?.city ?? '', - state: place?.state ?? '', - country: (place?.country && countryNameFromCode(place.country)) ?? '', - } - for (const field of placeFields) { - if (values[field] !== found[field]) updateValue(field, found[field]) - } - } - - const lookup = useReverseGeocode(fillPlace) - - // Only map clicks look up the place. Typed coordinates would send a - // request for every keystroke. - function updatePosition(position: [number, number]) { - const [latitude, longitude] = position - updateValue('latitude', latitude.toFixed(6)) - updateValue('longitude', longitude.toFixed(6)) - void lookup.lookUp(position) - } - - async function handleSubmit(event: FormEvent) { - event.preventDefault() - if (loading) return - setSaveFailed(false) - const nextErrors = validate(values) - setErrors(nextErrors) - // The form shows country names, but the database stores codes. - // validate() has already reported a name that has no code. - const country = countryCodeFromName(values.country) - if (Object.keys(nextErrors).length > 0 || country === null) { - focusFirstError.current = true - return - } - - try { - const { data } = await createSighting({ - // The cached lists don't contain the new sighting. Dropping them - // makes the search page fetch fresh results when it is opened. - update: (cache) => cache.evict({ fieldName: 'sightings' }), - variables: { - input: { - // datetime-local gives local time without timezone, which is - // the format the API stores. - datetime: values.datetime, - city: values.city.trim(), - // A state typed before switching to another country is hidden, - // so it isn't sent either. - state: isUnitedStates ? values.state.trim() || null : null, - country, - shape: values.shape, - durationSeconds: Number(values.durationSeconds), - comments: values.comments.trim(), - latitude: Number(values.latitude), - longitude: Number(values.longitude), - }, - }, - }) - if (!data) throw new Error('createSighting returned no data') - focusHeading.current = true - setSubmittedId(data.createSighting.id) - } catch (error) { - const fieldErrors = serverFieldErrors(error) - if (fieldErrors) { - focusFirstError.current = true - setErrors(fieldErrors) - } else { - setSaveFailed(true) - } - } - } + useDocumentTitle( + form.submittedId ? 'Sighting submitted' : 'Report a sighting', + ) - if (submittedId) { + if (form.submittedId) { return (
Back to sightings @@ -263,19 +52,11 @@ function SubmitSightingPage() {
View your sighting -
@@ -295,116 +76,45 @@ function SubmitSightingPage() {
-
- + updateValue('datetime', event.target.value)} - aria-invalid={Boolean(errors.datetime)} - aria-describedby={ - errors.datetime ? 'sighting-datetime-error' : undefined + onChange={(event) => + form.updateValue('datetime', event.target.value) } + onBlur={(event) => form.checkDatetime(event.target.value)} required /> - {errors.datetime && ( -

- {errors.datetime} -

- )} -
-
- - updateValue('city', event.target.value)} - aria-invalid={Boolean(errors.city)} - aria-describedby={errors.city ? 'sighting-city-error' : undefined} - required - /> - {errors.city && ( -

- {errors.city} -

- )} -
+
-
- - {/* The datalist suggests names as the user types. The browser's - own autocomplete is off so its history doesn't crowd them. */} - updateValue('country', event.target.value)} - aria-invalid={Boolean(errors.country)} - aria-describedby={ - errors.country ? 'sighting-country-error' : undefined - } - required - /> - - {countries.map(({ code, name }) => ( - - {errors.country && ( -

- {errors.country} -

- )} -
- {/* After the country in the page order, so it appears where the - keyboard focus goes next once "United States" is chosen. */} - {isUnitedStates && ( -
- - updateValue('state', event.target.value)} - /> -
- )} -
-
-
- + - updateValue('durationSeconds', event.target.value) - } - aria-invalid={Boolean(errors.durationSeconds)} - aria-describedby={ - errors.durationSeconds ? 'sighting-duration-error' : undefined - } - required - /> - {errors.durationSeconds && ( -

- {errors.durationSeconds} -

- )} -
-
-
-
- - updateValue('latitude', event.target.value)} - aria-invalid={Boolean(errors.latitude)} - aria-describedby={ - errors.latitude ? 'sighting-latitude-error' : undefined - } - required - /> - {errors.latitude && ( -

- {errors.latitude} -

- )} -
-
- - updateValue('longitude', event.target.value)} - aria-invalid={Boolean(errors.longitude)} - aria-describedby={ - errors.longitude ? 'sighting-longitude-error' : undefined - } - required - /> - {errors.longitude && ( -

- {errors.longitude} -

- )} -
-
-
-
-

Where did it happen?

-

- Click the map to place the marker and fill in the city and - country, or enter coordinates above. -

-
- - {/* Always rendered, so screen readers announce changes to it. */} -

+ - {lookup.status === 'loading' && 'Looking up the place…'} - {lookup.status === 'failed' && - "Couldn't find the place. Fill in the fields manually."} -

-

- Place names from{' '} - OpenStreetMap{' '} - via Nominatim. -

+
+ + form.updateValue('duration', event.target.value) + } + required + /> + +
+
-
- + +