From 9c29577b8207061b26634989f6c500522c1f06af Mon Sep 17 00:00:00 2001 From: Jens Date: Sat, 22 Aug 2026 18:56:21 +0200 Subject: [PATCH] collapse five duplicate overlay blocks into one tested builder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MapWorkspace held five near-identical useMemo blocks deciding which raster image the map draws under the active theme — terrain, flood depth, thematic raster, WALOUS land cover and bathymetry. Each filtered partitions by source name, read bbox_epsg4326 and assembled the same overlay shape, so the parts that genuinely differ per theme were buried in the repetition. One builder makes the rule testable and leaves only the source, the label and the opacity varying. A raster whose bounds are unusable is now skipped rather than drawn from a partial bbox: an overlay in the wrong place is worse than no overlay. The legend asked "are these thematic or WALOUS overlays" by inspecting two of the five lists. That is a property of the source, so it says so directly. Two contract tests needed fixing rather than repointing. One asserted `"api" not in source.lower()`, which the new hook name useMapImageOverlays matches inside "useM-api-mageOverlays" — as would rapid, capital or therapy. The contract is that this component talks to no API client, so it now says that. Co-Authored-By: Claude Opus 5 --- backend/tests/frontend_contract.py | 2 + .../test_sprint30_workbench_components.py | 6 +- frontend/src/components/map/MapWorkspace.tsx | 111 ++----------- .../src/hooks/useMapImageOverlays.test.ts | 120 +++++++++++++ frontend/src/hooks/useMapImageOverlays.ts | 157 ++++++++++++++++++ .../hooks/useMapRectangleSelection.test.tsx | 118 +++++++++++++ .../src/hooks/useMapRectangleSelection.ts | 94 +++++++++++ 7 files changed, 507 insertions(+), 101 deletions(-) create mode 100644 frontend/src/hooks/useMapImageOverlays.test.ts create mode 100644 frontend/src/hooks/useMapImageOverlays.ts create mode 100644 frontend/src/hooks/useMapRectangleSelection.test.tsx create mode 100644 frontend/src/hooks/useMapRectangleSelection.ts diff --git a/backend/tests/frontend_contract.py b/backend/tests/frontend_contract.py index 565bf360..bcc9ed91 100644 --- a/backend/tests/frontend_contract.py +++ b/backend/tests/frontend_contract.py @@ -30,6 +30,8 @@ MAP_WORKSPACE_SOURCES = ( "components/map/MapWorkspace.tsx", "components/map/mapWorkspaceThemes.ts", "components/map/mapWorkspaceUtils.ts", + "hooks/useMapImageOverlays.ts", + "hooks/useMapRectangleSelection.ts", "components/map/MapExplorerView.tsx", "components/map/MapAdvancedWorkbench.tsx", ) diff --git a/backend/tests/test_sprint30_workbench_components.py b/backend/tests/test_sprint30_workbench_components.py index f9a350ee..c35e1561 100644 --- a/backend/tests/test_sprint30_workbench_components.py +++ b/backend/tests/test_sprint30_workbench_components.py @@ -1,5 +1,6 @@ from __future__ import annotations +import re from pathlib import Path @@ -58,4 +59,7 @@ def test_map_workspace_owns_map_controls_and_feature_inspector_markup() -> None: assert "Objectinspectie" in map_workspace assert "onFeatureSelect={onSelectMapFeature}" in map_workspace assert "fetch(" not in map_workspace - assert "api" not in map_workspace.lower() + # The component owns markup and interaction, never transport. A bare "api" + # substring also matches useMapImageOverlays, so name what is forbidden. + assert "services/api" not in map_workspace + assert not re.search(r"\w*[Aa]pi\.(get|post|put|delete)\(", map_workspace) diff --git a/frontend/src/components/map/MapWorkspace.tsx b/frontend/src/components/map/MapWorkspace.tsx index 904cf098..a1202d68 100644 --- a/frontend/src/components/map/MapWorkspace.tsx +++ b/frontend/src/components/map/MapWorkspace.tsx @@ -2,18 +2,15 @@ import { useCallback, useEffect, useMemo, useRef, useState, type KeyboardEvent } import { BoxSelect, ChevronLeft, ChevronRight, MapPinned, Play, Search, SlidersHorizontal, Trash2 } from 'lucide-react' import GeoMap from '../GeoMap' import type { AreaRead, CoverageResolveResponse, CoverageStatus, DatasetCreateResponse, DetectionQaResult, MapResultExportRequest, MapViewportState, OrthophotoAcquisitionResult, OrthophotoProductRead, ProjectRead, QaComparisonResult, VectorSelectionBBox, VectorSelectionMetric, VectorSelectionResponse } from '../../types' -import { useMapThemeSelectionInsights, type MapThemeAcquisition, type MapThemeQuery } from '../../hooks/useMapThemeSelectionInsights' +import { useMapThemeSelectionInsights, type MapThemeQuery } from '../../hooks/useMapThemeSelectionInsights' import { useOfficialMapProducts } from '../../hooks/useOfficialMapProducts' import { useTemporalComparison } from '../../hooks/useTemporalComparison' +import { isValueRampRasterSource, useMapImageOverlays } from '../../hooks/useMapImageOverlays' import { getDatasetDisplayName, getDatasetSourceDisplayName } from '../../lib/datasetDisplay' import { TemporalTrendChart } from './TemporalTrendChart' import { MunicipalitySearch } from './MunicipalitySearch' import { LiveAnalysisJourney } from './LiveAnalysisJourney' import { SecondaryDisplayTarget } from '../shell/SecondaryDisplay' -import { terrainImageUrl } from '../../lib/terrainImage' -import { floodHazardImageUrl } from '../../lib/floodHazardImage' -import { thematicRasterImageUrl, walousRasterImageUrl } from '../../lib/thematicRaster' -import { bathymetryRasterImageUrl } from '../../lib/bathymetryRaster' import { FLANDERS_WORKSPACE_PROJECT_NAME } from '../../config/primaryFocus' import { MAP_ANALYSIS_BUDGET_MS, @@ -802,102 +799,16 @@ export function MapWorkspace({ themeDatasetMap, ]) - const terrainImageOverlays = useMemo( - () => - activeTheme.id === 'elevation' && selectedProjectId - ? activeThemePartitions.flatMap((dataset) => { - const bounds = dataset.source_metadata?.['bbox_epsg4326'] - return ['digitaal_vlaanderen_dhmv', 'spw_terrain'].includes(dataset.source_name ?? '') - && Array.isArray(bounds) - && bounds.length === 4 - ? [{ - url: terrainImageUrl(selectedProjectId, dataset.id), - bbox: bounds.map(Number) as [number, number, number, number], - label: getDatasetDisplayName(dataset), - opacity: 0.82, - }] - : [] - }) - : [], - [activeTheme.id, activeThemePartitions, selectedProjectId], - ) - const floodHazardImageOverlays = useMemo( - () => - activeTheme.id === 'flood_hazard' && selectedProjectId - ? activeThemePartitions.flatMap((dataset) => { - const bounds = dataset.source_metadata?.['bbox_epsg4326'] - return dataset.source_name === 'vmm_flood_hazard' - && Array.isArray(bounds) - && bounds.length === 4 - ? [{ - url: floodHazardImageUrl(selectedProjectId, dataset.id), - bbox: bounds.map(Number) as [number, number, number, number], - label: floodScenarioLabel(dataset), - opacity: 0.82, - }] - : [] - }) - : [], - [activeTheme.id, activeThemePartitions, selectedProjectId], - ) - const thematicRasterBounds = activeThemeDataset?.source_name === 'department_omgeving_thematic_raster' - ? activeThemeDataset.source_metadata?.['bbox_epsg4326'] - : null - const thematicRasterImageOverlays = useMemo( - () => activeThemeDataset?.source_name === 'department_omgeving_thematic_raster' && selectedProjectId && Array.isArray(thematicRasterBounds) && thematicRasterBounds.length === 4 - ? [{ - url: thematicRasterImageUrl(selectedProjectId, activeThemeDataset.id), - bbox: thematicRasterBounds.map(Number) as [number, number, number, number], - label: getDatasetDisplayName(activeThemeDataset), - opacity: 0.78, - }] - : [], - [activeThemeDataset, selectedProjectId, thematicRasterBounds], - ) - const walousRasterBounds = activeThemeDataset?.source_name === 'spw_walous_land_cover' - ? activeThemeDataset.source_metadata?.['bbox_epsg4326'] - : null - const walousRasterImageOverlays = useMemo( - () => activeThemeDataset?.source_name === 'spw_walous_land_cover' && selectedProjectId && Array.isArray(walousRasterBounds) && walousRasterBounds.length === 4 - ? [{ - url: walousRasterImageUrl(selectedProjectId, activeThemeDataset.id), - bbox: walousRasterBounds.map(Number) as [number, number, number, number], - label: getDatasetDisplayName(activeThemeDataset), - opacity: 0.82, - }] - : [], - [activeThemeDataset, selectedProjectId, walousRasterBounds], - ) - const bathymetryRasterBounds = activeThemeDataset?.source_name === 'spw_bathymetry' - ? activeThemeDataset.source_metadata?.['bbox_epsg4326'] - : null - const bathymetryRasterImageOverlays = useMemo( - () => activeThemeDataset?.source_name === 'spw_bathymetry' && selectedProjectId && Array.isArray(bathymetryRasterBounds) && bathymetryRasterBounds.length === 4 - ? [{ - url: bathymetryRasterImageUrl(selectedProjectId, activeThemeDataset.id), - bbox: bathymetryRasterBounds.map(Number) as [number, number, number, number], - label: 'Waterbodemhoogte in mDNG', - opacity: 0.86, - }] - : [], - [activeThemeDataset, bathymetryRasterBounds, selectedProjectId], - ) const thematicLegendMin = String(activeThemeDataset?.source_metadata?.['legend_min_label'] ?? 'Lagere waarde') const thematicLegendMax = String(activeThemeDataset?.source_metadata?.['legend_max_label'] ?? 'Hogere waarde') - const activeImageOverlays = useMemo( - () => bathymetryRasterImageOverlays.length > 0 - ? bathymetryRasterImageOverlays - : walousRasterImageOverlays.length > 0 - ? walousRasterImageOverlays - : thematicRasterImageOverlays.length > 0 - ? thematicRasterImageOverlays - : floodHazardImageOverlays.length > 0 - ? floodHazardImageOverlays - : terrainImageOverlays.length > 0 - ? terrainImageOverlays - : orthophotoImageOverlay ? [orthophotoImageOverlay] : [], - [bathymetryRasterImageOverlays, floodHazardImageOverlays, orthophotoImageOverlay, terrainImageOverlays, thematicRasterImageOverlays, walousRasterImageOverlays], - ) + const activeImageOverlays = useMapImageOverlays({ + selectedProjectId, + activeThemeId: activeTheme.id, + activeThemeDataset, + activeThemePartitions, + orthophotoImageOverlay, + floodScenarioLabel, + }) const municipalityAreaCount = areas.filter((area) => /^Gemeente\s/i.test(area.name)).length const themeTemporalSeriesMap = useMemo( () => @@ -2090,7 +2001,7 @@ export function MapWorkspace({ />
Werkgebied - {thematicRasterImageOverlays.length > 0 || walousRasterImageOverlays.length > 0 ? ( + {isValueRampRasterSource(activeThemeDataset) && activeImageOverlays.length > 0 ? ( {thematicLegendMin} → {thematicLegendMax} diff --git a/frontend/src/hooks/useMapImageOverlays.test.ts b/frontend/src/hooks/useMapImageOverlays.test.ts new file mode 100644 index 00000000..15b1636e --- /dev/null +++ b/frontend/src/hooks/useMapImageOverlays.test.ts @@ -0,0 +1,120 @@ +import { describe, expect, it } from 'vitest' + +import { buildRasterOverlays, pickActiveOverlays } from './useMapImageOverlays' +import type { DatasetCreateResponse } from '../types' + +/** + * Five near-identical useMemo blocks in MapWorkspace each filtered partitions by + * source name, read `bbox_epsg4326`, and built the same overlay shape. Collapsing + * them into one builder makes the rule testable and the differences — which + * source, which label, which opacity — visible instead of buried in repetition. + */ + +function dataset(overrides: Partial = {}): DatasetCreateResponse { + return { + id: 'dataset-1', + project_id: 'project-1', + name: 'dhmv.tif', + dataset_type: 'raster', + source: 'digitaal_vlaanderen_dhmv', + source_name: 'digitaal_vlaanderen_dhmv', + status: 'ready', + source_metadata: { bbox_epsg4326: [4.0, 51.0, 4.1, 51.1] }, + ...overrides, + } as DatasetCreateResponse +} + +describe('buildRasterOverlays', () => { + it('builds an overlay for each matching partition', () => { + const overlays = buildRasterOverlays({ + datasets: [dataset(), dataset({ id: 'dataset-2' })], + sourceNames: ['digitaal_vlaanderen_dhmv'], + opacity: 0.82, + url: (id) => `/terrain/${id}`, + label: () => 'Hoogtemodel', + }) + + expect(overlays.map((item) => item.url)).toEqual(['/terrain/dataset-1', '/terrain/dataset-2']) + expect(overlays[0].bbox).toEqual([4.0, 51.0, 4.1, 51.1]) + expect(overlays[0].opacity).toBe(0.82) + }) + + it('skips a dataset from another source', () => { + const overlays = buildRasterOverlays({ + datasets: [dataset({ source_name: 'vmm_flood_hazard' })], + sourceNames: ['digitaal_vlaanderen_dhmv'], + opacity: 0.82, + url: (id) => `/terrain/${id}`, + label: () => 'Hoogtemodel', + }) + + expect(overlays).toEqual([]) + }) + + it('skips a dataset without usable bounds rather than drawing it wrong', () => { + const cases = [ + dataset({ source_metadata: {} }), + dataset({ source_metadata: { bbox_epsg4326: [4.0, 51.0] } }), + dataset({ source_metadata: { bbox_epsg4326: 'not-an-array' } }), + ] + + for (const item of cases) { + expect( + buildRasterOverlays({ + datasets: [item], + sourceNames: ['digitaal_vlaanderen_dhmv'], + opacity: 0.82, + url: (id) => `/terrain/${id}`, + label: () => 'Hoogtemodel', + }), + ).toEqual([]) + } + }) + + it('accepts several source names for one theme', () => { + const overlays = buildRasterOverlays({ + datasets: [dataset({ source_name: 'spw_terrain' })], + sourceNames: ['digitaal_vlaanderen_dhmv', 'spw_terrain'], + opacity: 0.82, + url: (id) => `/terrain/${id}`, + label: () => 'Terrein', + }) + + expect(overlays).toHaveLength(1) + }) + + it('takes the label from the dataset so a scenario can name itself', () => { + const overlays = buildRasterOverlays({ + datasets: [dataset({ source_name: 'vmm_flood_hazard', name: 'T100' })], + sourceNames: ['vmm_flood_hazard'], + opacity: 0.82, + url: (id) => `/flood/${id}`, + label: (item) => `Scenario ${item.name}`, + }) + + expect(overlays[0].label).toBe('Scenario T100') + }) +}) + +describe('pickActiveOverlays', () => { + const overlay = (label: string) => ({ url: `/${label}`, bbox: [0, 0, 1, 1] as [number, number, number, number], label, opacity: 1 }) + + it('draws the most specific raster the theme resolved to', () => { + const active = pickActiveOverlays({ + candidates: [[], [overlay('walous')], [overlay('terrain')]], + fallback: overlay('orthophoto'), + }) + + expect(active.map((item) => item.label)).toEqual(['walous']) + }) + + it('falls back to the orthophoto when no raster theme is active', () => { + const active = pickActiveOverlays({ candidates: [[], []], fallback: overlay('orthophoto') }) + + expect(active.map((item) => item.label)).toEqual(['orthophoto']) + }) + + it('draws nothing when there is neither a raster nor an orthophoto', () => { + expect(pickActiveOverlays({ candidates: [[], []], fallback: null })).toEqual([]) + }) +}) diff --git a/frontend/src/hooks/useMapImageOverlays.ts b/frontend/src/hooks/useMapImageOverlays.ts new file mode 100644 index 00000000..4326002d --- /dev/null +++ b/frontend/src/hooks/useMapImageOverlays.ts @@ -0,0 +1,157 @@ +import { useMemo } from 'react' + +import type { DatasetCreateResponse } from '../types' +import { getDatasetDisplayName } from '../lib/datasetDisplay' +import { terrainImageUrl } from '../lib/terrainImage' +import { floodHazardImageUrl } from '../lib/floodHazardImage' +import { thematicRasterImageUrl, walousRasterImageUrl } from '../lib/thematicRaster' +import { bathymetryRasterImageUrl } from '../lib/bathymetryRaster' + +/** + * Which raster image the map draws underneath the active theme. + * + * MapWorkspace held five near-identical useMemo blocks for this — terrain, + * flood depth, thematic raster, WALOUS land cover and bathymetry — each + * filtering partitions by source name, reading `bbox_epsg4326` and assembling + * the same overlay shape. One builder makes the rule testable and leaves only + * the parts that genuinely differ per theme: the source, the label and the + * opacity. + */ + +export interface MapImageOverlay { + url: string + bbox: [number, number, number, number] + label: string + opacity: number +} + +interface RasterOverlayRequest { + datasets: DatasetCreateResponse[] + sourceNames: string[] + opacity: number + url: (datasetId: string) => string + label: (dataset: DatasetCreateResponse) => string +} + +/** A bbox is only usable if the source actually published four numbers for it. */ +function overlayBounds(dataset: DatasetCreateResponse): [number, number, number, number] | null { + const bounds = dataset.source_metadata?.['bbox_epsg4326'] + if (!Array.isArray(bounds) || bounds.length !== 4) return null + const numbers = bounds.map(Number) + return numbers.some((value) => !Number.isFinite(value)) + ? null + : (numbers as [number, number, number, number]) +} + +export function buildRasterOverlays({ + datasets, + sourceNames, + opacity, + url, + label, +}: RasterOverlayRequest): MapImageOverlay[] { + return datasets.flatMap((dataset) => { + if (!sourceNames.includes(dataset.source_name ?? '')) return [] + const bbox = overlayBounds(dataset) + // A raster we cannot place is not drawn at all: an overlay in the wrong + // spot is worse than no overlay. + if (!bbox) return [] + return [{ url: url(dataset.id), bbox, label: label(dataset), opacity }] + }) +} + +/** The first non-empty raster wins; the orthophoto is the base layer. */ +export function pickActiveOverlays({ + candidates, + fallback, +}: { + candidates: MapImageOverlay[][] + fallback: MapImageOverlay | null +}): MapImageOverlay[] { + const active = candidates.find((item) => item.length > 0) + if (active) return active + return fallback ? [fallback] : [] +} + +/** Sources drawn as a continuous value ramp, which is what the legend explains. */ +const VALUE_RAMP_SOURCES = ['department_omgeving_thematic_raster', 'spw_walous_land_cover'] + +export function isValueRampRasterSource(dataset: DatasetCreateResponse | null | undefined): boolean { + return VALUE_RAMP_SOURCES.includes(dataset?.source_name ?? '') +} + +interface MapImageOverlayOptions { + selectedProjectId: string | null + activeThemeId: string + activeThemeDataset: DatasetCreateResponse | null | undefined + activeThemePartitions: DatasetCreateResponse[] + orthophotoImageOverlay: MapImageOverlay | null + floodScenarioLabel: (dataset: DatasetCreateResponse) => string +} + +export function useMapImageOverlays({ + selectedProjectId, + activeThemeId, + activeThemeDataset, + activeThemePartitions, + orthophotoImageOverlay, + floodScenarioLabel, +}: MapImageOverlayOptions): MapImageOverlay[] { + return useMemo(() => { + if (!selectedProjectId) return orthophotoImageOverlay ? [orthophotoImageOverlay] : [] + + const singleDataset = activeThemeDataset ? [activeThemeDataset] : [] + // Ordered most specific first: a theme that resolved to a dedicated raster + // product should not be drawn as generic terrain underneath it. + const candidates = [ + buildRasterOverlays({ + datasets: singleDataset, + sourceNames: ['spw_bathymetry'], + opacity: 0.86, + url: (id) => bathymetryRasterImageUrl(selectedProjectId, id), + label: () => 'Waterbodemhoogte in mDNG', + }), + buildRasterOverlays({ + datasets: singleDataset, + sourceNames: ['spw_walous_land_cover'], + opacity: 0.82, + url: (id) => walousRasterImageUrl(selectedProjectId, id), + label: getDatasetDisplayName, + }), + buildRasterOverlays({ + datasets: singleDataset, + sourceNames: ['department_omgeving_thematic_raster'], + opacity: 0.78, + url: (id) => thematicRasterImageUrl(selectedProjectId, id), + label: getDatasetDisplayName, + }), + activeThemeId === 'flood_hazard' + ? buildRasterOverlays({ + datasets: activeThemePartitions, + sourceNames: ['vmm_flood_hazard'], + opacity: 0.82, + url: (id) => floodHazardImageUrl(selectedProjectId, id), + label: floodScenarioLabel, + }) + : [], + activeThemeId === 'elevation' + ? buildRasterOverlays({ + datasets: activeThemePartitions, + sourceNames: ['digitaal_vlaanderen_dhmv', 'spw_terrain'], + opacity: 0.82, + url: (id) => terrainImageUrl(selectedProjectId, id), + label: getDatasetDisplayName, + }) + : [], + ] + + return pickActiveOverlays({ candidates, fallback: orthophotoImageOverlay }) + }, [ + activeThemeDataset, + activeThemeId, + activeThemePartitions, + floodScenarioLabel, + orthophotoImageOverlay, + selectedProjectId, + ]) +} diff --git a/frontend/src/hooks/useMapRectangleSelection.test.tsx b/frontend/src/hooks/useMapRectangleSelection.test.tsx new file mode 100644 index 00000000..26af52c9 --- /dev/null +++ b/frontend/src/hooks/useMapRectangleSelection.test.tsx @@ -0,0 +1,118 @@ +import { act, renderHook } from '@testing-library/react' +import { describe, expect, it, vi } from 'vitest' + +import { useMapRectangleSelection } from './useMapRectangleSelection' +import type { VectorSelectionBBox } from '../types' + +/** + * The rule under test is not the geometry but the clearing: results from the + * previous rectangle must be retired before a new one is drawn, or an operator + * reads the old area's numbers as belonging to the new selection. + */ + +const BBOX: VectorSelectionBBox = { min_x: 5.0, min_y: 51.1, max_x: 5.2, max_y: 51.3, crs: 'EPSG:4326' } + +function setup() { + const setSelectionBbox = vi.fn() + const retireStaleResults = vi.fn() + const view = renderHook(() => + useMapRectangleSelection({ initialBbox: null, setSelectionBbox, retireStaleResults }), + ) + return { view, setSelectionBbox, retireStaleResults } +} + +describe('two-click rectangle', () => { + it('records the first corner without touching the results on screen', () => { + const { view, setSelectionBbox, retireStaleResults } = setup() + + act(() => view.result.current.handleCoordinateSelect([5.0, 51.1])) + + expect(view.result.current.firstCorner).toEqual([5.0, 51.1]) + expect(setSelectionBbox).not.toHaveBeenCalled() + expect(retireStaleResults).not.toHaveBeenCalled() + }) + + it('commits on the second corner and retires the previous results first', () => { + const { view, setSelectionBbox, retireStaleResults } = setup() + + act(() => view.result.current.handleCoordinateSelect([5.2, 51.3])) + act(() => view.result.current.handleCoordinateSelect([5.0, 51.1])) + + expect(retireStaleResults).toHaveBeenCalledTimes(1) + expect(setSelectionBbox).toHaveBeenCalledTimes(1) + const committed = setSelectionBbox.mock.calls[0][0] + expect(committed.min_x).toBeCloseTo(5.0) + expect(committed.max_y).toBeCloseTo(51.3) + }) + + it('leaves no half-finished gesture behind', () => { + const { view } = setup() + + act(() => view.result.current.handleCoordinateSelect([5.2, 51.3])) + act(() => view.result.current.handleCoordinateSelect([5.0, 51.1])) + + expect(view.result.current.firstCorner).toBeNull() + expect(view.result.current.drawing).toBe(false) + }) +}) + +describe('drag rectangle', () => { + it('moves the rectangle during the drag without retiring anything', () => { + const { view, setSelectionBbox, retireStaleResults } = setup() + + act(() => view.result.current.handleBboxPreview(BBOX)) + + expect(setSelectionBbox).toHaveBeenCalledWith(BBOX) + expect(retireStaleResults).not.toHaveBeenCalled() + }) + + it('retires the previous results when the drag is released', () => { + const { view, setSelectionBbox, retireStaleResults } = setup() + + act(() => view.result.current.handleBboxSelect(BBOX)) + + expect(retireStaleResults).toHaveBeenCalledTimes(1) + expect(setSelectionBbox).toHaveBeenCalledWith(BBOX) + }) + + it('ends the drawing mode on release', () => { + const { view } = setup() + + act(() => view.result.current.setDrawing(true)) + act(() => view.result.current.handleBboxSelect(BBOX)) + + expect(view.result.current.drawing).toBe(false) + }) +}) + +describe('typed coordinates', () => { + it('parses a complete rectangle from the coordinate fields', () => { + const { view } = setup() + + act(() => + view.result.current.setBboxInput({ min_x: '5.0', min_y: '51.1', max_x: '5.2', max_y: '51.3' }), + ) + + expect(view.result.current.typedBbox).toMatchObject({ min_x: 5.0, max_y: 51.3 }) + }) + + it('reports no rectangle while the fields are incomplete', () => { + const { view } = setup() + + act(() => view.result.current.setBboxInput({ min_x: '5.0', min_y: '', max_x: '', max_y: '' })) + + expect(view.result.current.typedBbox).toBeNull() + }) +}) + +describe('reset', () => { + it('abandons a half-drawn rectangle without committing it', () => { + const { view, setSelectionBbox } = setup() + + act(() => view.result.current.handleCoordinateSelect([5.0, 51.1])) + act(() => view.result.current.reset()) + + expect(view.result.current.firstCorner).toBeNull() + expect(setSelectionBbox).not.toHaveBeenCalled() + }) +}) diff --git a/frontend/src/hooks/useMapRectangleSelection.ts b/frontend/src/hooks/useMapRectangleSelection.ts new file mode 100644 index 00000000..9659ea7e --- /dev/null +++ b/frontend/src/hooks/useMapRectangleSelection.ts @@ -0,0 +1,94 @@ +import { useCallback, useState } from 'react' + +import type { VectorSelectionBBox } from '../types' +import { bboxToInputState, normalizeBboxFromCorners, parseBboxInput } from '../components/map/mapWorkspaceUtils' + +/** + * Drawing a rectangle on the map, by drag or by two clicks. + * + * The rule that matters here is not the geometry but the clearing: a new + * selection must retire the previous theme insights and temporal comparison + * before anything is drawn, so results from the old rectangle can never be read + * as belonging to the new one. + */ + +interface MapRectangleSelectionOptions { + initialBbox: VectorSelectionBBox | null + setSelectionBbox: (bbox: VectorSelectionBBox) => void + /** Everything that describes the *previous* rectangle and must not survive. */ + retireStaleResults: () => void +} + +export interface MapRectangleSelection { + drawing: boolean + setDrawing: (value: boolean) => void + firstCorner: [number, number] | null + bboxInput: ReturnType + setBboxInput: (value: ReturnType) => void + /** The rectangle currently typed into the coordinate fields, if it parses. */ + typedBbox: VectorSelectionBBox | null + handleCoordinateSelect: (coordinate: [number, number]) => void + handleBboxPreview: (bbox: VectorSelectionBBox) => void + handleBboxSelect: (bbox: VectorSelectionBBox) => void + reset: () => void +} + +export function useMapRectangleSelection({ + initialBbox, + setSelectionBbox, + retireStaleResults, +}: MapRectangleSelectionOptions): MapRectangleSelection { + const [drawing, setDrawing] = useState(false) + const [firstCorner, setFirstCorner] = useState<[number, number] | null>(null) + const [bboxInput, setBboxInput] = useState(bboxToInputState(initialBbox)) + + const commit = useCallback( + (bbox: VectorSelectionBBox) => { + setFirstCorner(null) + setDrawing(false) + retireStaleResults() + setSelectionBbox(bbox) + }, + [retireStaleResults, setSelectionBbox], + ) + + const handleCoordinateSelect = useCallback( + (coordinate: [number, number]) => { + // Two clicks make a rectangle: the first only records a corner, and must + // not disturb the results that are still on screen. + if (!firstCorner) { + setFirstCorner(coordinate) + return + } + commit(normalizeBboxFromCorners(firstCorner, coordinate)) + }, + [commit, firstCorner], + ) + + // A drag preview moves the rectangle without ending the gesture, so nothing + // is retired until the drag is released. + const handleBboxPreview = useCallback( + (bbox: VectorSelectionBBox) => setSelectionBbox(bbox), + [setSelectionBbox], + ) + + const handleBboxSelect = useCallback((bbox: VectorSelectionBBox) => commit(bbox), [commit]) + + const reset = useCallback(() => { + setFirstCorner(null) + setDrawing(false) + }, []) + + return { + drawing, + setDrawing, + firstCorner, + bboxInput, + setBboxInput, + typedBbox: parseBboxInput(bboxInput), + handleCoordinateSelect, + handleBboxPreview, + handleBboxSelect, + reset, + } +}