Make the report form's layout clearer

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.

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
+}

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.',
+  )
+})

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<HTMLAnchorElement>, id: string) {
+    event.preventDefault()
+    document.getElementById(id)?.focus()
+  }
+
+  return (
+    <div className="submit-sighting__error-summary">
+      <p className="submit-sighting__summary" role="alert">
+        Please correct the highlighted fields.
+      </p>
+      <ul>
+        {items.map(([field, label]) => (
+          <li key={field}>
+            <a
+              href={`#sighting-${field}`}
+              onClick={(event) => focusField(event, `sighting-${field}`)}
+            >
+              {label}: {errors[field]}
+            </a>
+          </li>
+        ))}
+      </ul>
+    </div>
+  )
+}
+
+export default ErrorSummary

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 (
+    <div className="submit-sighting__field">
+      <label htmlFor={id}>
+        {label}
+        {required && (
+          <>
+            {' '}
+            <span aria-hidden="true">*</span>
+          </>
+        )}
+      </label>
+      {children}
+      {error && (
+        <p className="submit-sighting__error" id={`${id}-error`}>
+          {error}
+        </p>
+      )}
+    </div>
+  )
+}

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 (
     <div className="location-picker">
       <MapContainer
@@ -47,6 +73,7 @@ function LocationPicker({ position, onPositionChange }: LocationPickerProps) {
         />
         <MapLabel label="Choose sighting location on map" />
         <MapClickHandler onPositionChange={onPositionChange} />
+        {showPlace && <MapMover target={showPlace} />}
         {/* Clicking the marker does nothing, so it is not made a button. */}
         {position && (
           <Marker