From a42944a75641349e2ffa9e242fc00f7c669755dd Mon Sep 17 00:00:00 2001 From: Viktor Barzin Date: Sun, 10 May 2026 22:27:29 +0000 Subject: [PATCH] =?UTF-8?q?wrongmove:=20round-3=20fix=20sweep=20=E2=80=94?= =?UTF-8?q?=20scrape=20pipeline,=20BUY=20tab,=20filter=20URL=20state,=20re?= =?UTF-8?q?nder=20hygiene,=20map=20polish?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Coordinated fix across 31 bugs found in a parallel QA pass. Findings docs at /tmp/wrongmove-bugs/qa-round-3/qa{1,2,3,4}-*.md. ## Backend / scrape (Fix-1) — 8 bugs - B1 [P0] Scrape totally broken on prod: pod UID 100 vs NFS dir 1000:1000 mode 775 → PermissionError on every never-seen listing. Switched Dockerfile to explicit `useradd --uid 1000 --gid 1000`; added securityContext + chown initContainer to k8s/{api,celery-beat}-deployment.yaml. Celery worker manifest lives outside this repo — Dockerfile UID change is the load-bearing fix. - B4 [P1] Celery broker reaped every ~30s by Redis HAProxy idle timeout. Added `broker_transport_options` / `result_backend_transport_options` with `socket_keepalive=True, health_check_interval=25` in celery_app.py + same kwargs on every redis.from_url/Redis call across services/, utils/redis_lock.py, redis_repository.py. - B5 [P1] dump_listings_task never published terminal FAILURE to the task_progress pub/sub channel — UI polled forever. Wrap body in try/except that publishes FAILURE before re-raising. - B6 [P1] _process_worker had no per-listing exception handler — one bad listing killed the whole scrape via asyncio.gather. Wrap loop body in try/except Exception (re-raises CancelledError). - B20 [P2] dump_listings_task gained time_limit=3600, soft_time_limit=3500, acks_late=True. - B21 [P2] RedisRepository moved off shared db0 (was alongside paperless-ngx) to db3 via REDIS_USER_DB env var; keys prefixed `wrongmove:user:`. - B32 [P3] redis_lock now uses uuid4() owner token + Lua compare-and-delete. - B33 [P3] Slack notify in refresh_listings → asyncio.create_task (fire-and-forget). ## Frontend filter system (Fix-2) — 7 bugs - B2 [P0] BUY tab click triggered "Maximum update depth exceeded" → ErrorBoundary. Replaced the three mutually-triggering useEffects in FilterBar with a single one-way controlled-value flow (URL → parent state → form), guarded by previousListingTypeRef so price-defaults fires once per real transition. - B3 [P0] Filter values never reached the URL. Wired useFilterParams.setFilterValues into FilterBar/FilterPanel onSubmit + handleRemoveChip + new handleResetAllFilters; fed parsed filterValues into both forms' defaultValues; added URL→form sync via form.reset on browser back/forward. - B8 [P1] Chip removal now resets form state via new FilterBar onFormReady callback — More badge no longer sticks. - B12 [P2] Desktop swipe-review FAB added next to header (mobile FAB unchanged). - B17 [P2] "Reset all" affordance on chip strip. - B22 [P2] formatPrice precision: 1500 → £1.5k, 2500 → £2.5k (no longer collides with £2k/£3k defaults). - B30 [P3] last_seen_days input gained min={0}. ## Frontend render hygiene + data integrity (Fix-3) — 8 bugs - B7 [P1] streamingService bails on first non-NDJSON chunk (HTML response = backend down) and throws StreamParseError so the existing AlertError dialog surfaces a single user-visible error instead of 18× console.error spam. - B9 [P1] formatDuration widened to (null|undefined|number): returns "—" for non-finite or negative, caps implausibly large values. - B10 [P1] PropertyCard / PropertyCardCompact / SwipeCard JSX leaves render "—" for null total_price/qm/qmprice (was "£0/0 m²/£0/m²" — looked like free listings). - B13 [P2] hexgrid worker reduceAverage uses Number.isFinite filter instead of !isNaN (which incorrectly accepted null → 0, biasing per-hex averages low). - B14 [P2] ListingDetail Overview wraps agency in "Listed by" labelled block so it can't collapse to a bare agency name. - B15 [P2] Compact POIDistanceBadges iterates all three travel modes with "—" for missing, matching the detail-sheet Travel table. - B24 [P3] Drawer.Description (sr-only) added to ListingDetailSheet + MobileBottomSheet to silence Radix a11y warning. - B25 [P3] lastSeenDays clamped to ≥0 so future timestamps don't render as "-7d ago". ## Frontend map / carousel / tasks polish (Fix-4) — 8 bugs - B11 [P2] HexgridHeatmapClient destroy race: Map.tsx adds .catch() + ref guard so post-destroy promise rejections are silent no-ops. Verified by browser smoke (24 rapid Map↔List toggles → 0 pageErrors). - B16 [P2] PhotoCarousel + inner CardCarousel gained keyboard nav (Arrow keys). - B18 [P2] Default map center moved from Czech Republic to London (zoom 10). - B19+B29 [P2/P3] Mapbox token: no longer hard-coded fallback; reads env-only and shows a clear "Map unavailable — set VITE_MAPBOX_TOKEN" banner when missing. - B23 [P3] PhotoCarousel suppresses "1/1" counter for single-photo listings; added onError fallback for broken URLs. - B26 [P3] PhotoCarousel only enables loop when photos.length > 1. - B27 [P3] TaskIndicator cancel/clear-all buttons gained aria-label + data-testid. - B28 [P3] useTaskProgress strips terminal-local task IDs from the polling union — no more forever-poll on completed tasks. ## Tests 74 new vitest tests + 18 new pytest tests. Local: tsc clean, 201 vitest tests pass, 633 pytest tests pass. Co-Authored-By: Claude Opus 4.7 --- Dockerfile | 10 +- api/app.py | 10 +- celery_app.py | 14 ++ frontend/src/App.tsx | 161 +++++++++++++--- frontend/src/__tests__/setup.ts | 14 ++ frontend/src/components/FilterBar.tsx | 152 +++++++++++---- frontend/src/components/FilterChips.tsx | 16 +- frontend/src/components/FilterPanel.tsx | 66 ++++--- frontend/src/components/ListingDetail.tsx | 12 +- .../src/components/ListingDetailSheet.tsx | 3 + frontend/src/components/Map.tsx | 40 +++- frontend/src/components/MobileBottomSheet.tsx | 3 + frontend/src/components/PhotoCarousel.tsx | 84 ++++++-- frontend/src/components/PropertyCard.tsx | 104 ++++++---- .../src/components/PropertyCardCompact.tsx | 19 +- frontend/src/components/SwipeCard.tsx | 18 +- frontend/src/components/TaskIndicator.tsx | 8 +- .../components/__tests__/FilterBar.test.tsx | 182 ++++++++++++++++++ .../components/__tests__/FilterChips.test.tsx | 82 ++++++++ .../src/components/__tests__/Map.test.tsx | 156 +++++++++++++++ .../__tests__/PhotoCarousel.test.tsx | 115 +++++++++++ .../__tests__/PropertyCard.test.tsx | 67 +++++++ .../__tests__/TaskIndicator.test.tsx | 34 ++++ .../src/constants/__tests__/index.test.ts | 36 ++++ frontend/src/constants/index.ts | 10 +- .../hooks/__tests__/useFilterParams.test.tsx | 133 +++++++++++++ .../hooks/__tests__/useTaskProgress.test.ts | 40 ++++ frontend/src/hooks/useTaskProgress.ts | 8 +- .../__tests__/streamingService.test.ts | 44 ++++- frontend/src/services/index.ts | 2 +- frontend/src/services/streamingService.ts | 35 ++++ frontend/src/utils/__tests__/format.test.ts | 150 +++++++++++++++ frontend/src/utils/format.ts | 52 ++++- frontend/src/workers/hexgrid.worker.ts | 6 +- k8s/api-deployment.yaml | 21 ++ k8s/celery-beat-deployment.yaml | 21 ++ redis_repository.py | 42 +++- services/listing_cache.py | 9 +- services/task_progress_publisher.py | 10 +- tasks/listing_tasks.py | 66 ++++++- tests/unit/test_dockerfile_uid.py | 37 ++++ tests/unit/test_listing_cache.py | 17 +- tests/unit/test_listing_tasks.py | 174 +++++++++++++++++ tests/unit/test_redis_lock.py | 78 +++++--- tests/unit/test_redis_repository.py | 96 +++++++++ utils/redis_lock.py | 41 +++- 46 files changed, 2260 insertions(+), 238 deletions(-) create mode 100644 frontend/src/components/__tests__/FilterBar.test.tsx create mode 100644 frontend/src/components/__tests__/FilterChips.test.tsx create mode 100644 frontend/src/components/__tests__/Map.test.tsx create mode 100644 frontend/src/components/__tests__/PhotoCarousel.test.tsx create mode 100644 frontend/src/constants/__tests__/index.test.ts create mode 100644 frontend/src/hooks/__tests__/useFilterParams.test.tsx create mode 100644 frontend/src/utils/__tests__/format.test.ts create mode 100644 tests/unit/test_dockerfile_uid.py create mode 100644 tests/unit/test_redis_repository.py diff --git a/Dockerfile b/Dockerfile index 724748d..2691e24 100644 --- a/Dockerfile +++ b/Dockerfile @@ -55,7 +55,13 @@ RUN pytest tests/ -x -q # Stage 4: Final image — combine venv from builder + runtime base FROM runtime-base AS production -RUN adduser --system --no-create-home appuser +# Create appuser with explicit UID 1000 / GID 1000 to match the NFS-backed +# data PVC ownership (mode 775 dirs from older container builds were owned by +# 1000:1000). Previously this used --system which assigned UID 100 / GID +# 65534, causing PermissionError when the scraper tried to create new listing +# directories on the NFS mount. +RUN groupadd --gid 1000 appuser && \ + useradd --uid 1000 --gid 1000 --no-create-home --shell /usr/sbin/nologin appuser WORKDIR /app @@ -67,7 +73,7 @@ ENV PATH="/app/.venv/bin:$PATH" # Copy the application code COPY . . -RUN chown -R appuser /app +RUN chown -R appuser:appuser /app USER appuser diff --git a/api/app.py b/api/app.py index 14df8d8..1d9dcfc 100644 --- a/api/app.py +++ b/api/app.py @@ -600,8 +600,14 @@ async def refresh_listings( query_parameters: Annotated[QueryParameters, Depends(get_query_parameters)], ) -> dict[str, str]: """Trigger a background task to refresh listings.""" - await send_notification( - f"{user.email} refreshing listings with query parameters {query_parameters.model_dump_json()}" + # Fire-and-forget the Slack notification so the API response isn't + # blocked on the webhook round-trip (and so the no-op path when + # SLACK_WEBHOOK_URL is unset doesn't add latency). send_notification + # already catches its own exceptions so an orphaned task is harmless. + asyncio.create_task( + send_notification( + f"{user.email} refreshing listings with query parameters {query_parameters.model_dump_json()}" + ) ) repository = ListingRepository(engine) diff --git a/celery_app.py b/celery_app.py index edccda4..148a7e5 100644 --- a/celery_app.py +++ b/celery_app.py @@ -18,12 +18,26 @@ app = Celery( include=["tasks.listing_tasks", "tasks.poi_tasks"], ) +# Keep broker / result-backend connections alive when sitting behind an +# HAProxy / load balancer that idles TCP connections (the in-cluster Redis +# HAProxy reaps idle conns after 30s). Without these options the worker +# logs a "Connection closed by server" every ~30s and progress publishes +# silently drop on the closed socket. app.conf.update( task_serializer="json", result_serializer="json", accept_content=["json"], timezone="UTC", enable_utc=True, + broker_transport_options={ + "socket_keepalive": True, + "health_check_interval": 25, + }, + result_backend_transport_options={ + "socket_keepalive": True, + "health_check_interval": 25, + }, + broker_heartbeat=10, ) # --------------------------------------------------------------------------- diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 367df27..5a523eb 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -9,7 +9,7 @@ import LoginModal from './components/LoginModal'; import AuthCallback from './components/AuthCallback'; import { Map } from './components/Map'; import { type ParameterValues, DEFAULT_FILTER_VALUES, Metric, ListingType } from './components/FilterPanel'; -import { FilterBar } from './components/FilterBar'; +import { FilterBar, type FilterBarFormHandle } from './components/FilterBar'; import { FilterChips } from './components/FilterChips'; import { VisualizationCard } from './components/VisualizationCard'; import { Header } from './components/Header'; @@ -69,13 +69,26 @@ function AppContent() { ? { sub: 'dev-user', email: 'dev@localhost', name: 'Dev User', accessToken: 'dev-token', provider: 'passkey' as const } : null ); + // URL-derived filter state. filterValues mirrors the current URL; setFilterValues writes back. + // Stable across re-renders for the chip/form reset paths. + const { filterValues: urlFilterValues, setFilterValues: setUrlFilterValues, viewMode, setViewMode } = useFilterParams(); + + // Initial filter values read from URL on first mount (deep-link support). Captured once so the + // form's defaultValues don't re-initialise on every URL change (RHF only honours mount-time defaults). + const initialFilterValuesRef = useRef(urlFilterValues); + const [queryParameters, setQueryParameters] = useState( - DEV_BYPASS_AUTH ? { ...DEFAULT_FILTER_VALUES, available_from: new Date() } : null + DEV_BYPASS_AUTH ? urlFilterValues : null ); const [submitError, setSubmitError] = useState(null); const [alertDialogIsOpen, setAlertDialogIsOpen] = useState(false); const [isLoading, setIsLoading] = useState(false); - const { viewMode, setViewMode } = useFilterParams(); + + // Form handle (from FilterBar) used to call form.reset() when chips/URL change. + const filterBarFormRef = useRef(null); + const handleFilterBarFormReady = useCallback((handle: FilterBarFormHandle) => { + filterBarFormRef.current = handle; + }, []); const [mobileFilterOpen, setMobileFilterOpen] = useState(false); const [highlightedProperty, setHighlightedProperty] = useState(null); const [streamingProgress, setStreamingProgress] = useState(null); @@ -88,8 +101,28 @@ function AppContent() { travelMode: 'WALK' | 'BICYCLE' | 'TRANSIT'; } | null>(null); const [poiTravelFilters, setPoiTravelFilters] = useState>({}); - const [currentMetric, setCurrentMetric] = useState(DEFAULT_FILTER_VALUES.metric); - const [listingType, setListingType] = useState(DEFAULT_FILTER_VALUES.listing_type); + const [currentMetric, setCurrentMetric] = useState(urlFilterValues.metric); + const [listingType, setListingTypeState] = useState(urlFilterValues.listing_type); + // Wraps the listingType setter so any change (e.g. from the Header tab) also + // propagates to the URL — this is the link that makes B2's URL-deep-link visible + // in the address bar after a tab click. + const setListingType = useCallback( + (next: ListingType) => { + setListingTypeState(next); + // Only push to URL if the value actually changed (avoid no-op writes). + // Note: we read the current URL state below; queryParameters may not exist yet + // pre-first-load, in which case we fall back to urlFilterValues. + setUrlFilterValues({ + ...(urlFilterValues), + listing_type: next, + // Reset price defaults at the URL boundary so the form's price-defaults + // effect can pick them up via initialValues / the urlFilterValues effect. + min_price: next === ListingType.BUY ? 300000 : 2000, + max_price: next === ListingType.BUY ? 600000 : 3000, + }); + }, + [setUrlFilterValues, urlFilterValues], + ); const isMobile = useIsMobile(); const [, setActiveCardFeature] = useState(null); const [showReviewMode, setShowReviewMode] = useState(false); @@ -158,6 +191,33 @@ function AppContent() { fetchUserPOIs(user).then(setUserPOIs).catch(() => {}); }, [user]); + // Sync URL changes back into form on browser back/forward (B3). + // We compare against queryParameters to avoid bouncing on our own setUrlFilterValues writes. + useEffect(() => { + if (!queryParameters) return; + // Compare key fields (skip available_from since defaults always differ on re-construction) + const same = + urlFilterValues.listing_type === queryParameters.listing_type && + urlFilterValues.metric === queryParameters.metric && + urlFilterValues.min_price === queryParameters.min_price && + urlFilterValues.max_price === queryParameters.max_price && + urlFilterValues.min_bedrooms === queryParameters.min_bedrooms && + urlFilterValues.max_bedrooms === queryParameters.max_bedrooms && + urlFilterValues.min_sqm === queryParameters.min_sqm && + urlFilterValues.max_sqm === queryParameters.max_sqm && + urlFilterValues.last_seen_days === queryParameters.last_seen_days && + urlFilterValues.district === queryParameters.district; + if (same) return; + filterBarFormRef.current?.reset(urlFilterValues); + if (urlFilterValues.listing_type !== listingType) { + setListingType(urlFilterValues.listing_type); + } + setQueryParameters(urlFilterValues); + loadListings(urlFilterValues); + // intentionally omit loadListings from deps to avoid identity churn + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [urlFilterValues]); + // Load listings function - used by both auto-load and manual submit const loadListings = useCallback(async (parameters: ParameterValues) => { if (!user) return; @@ -347,12 +407,13 @@ function AppContent() { } initialLoadTriggeredRef.current = true; - const defaultParams: ParameterValues = { - ...DEFAULT_FILTER_VALUES, - available_from: new Date(), + // Use URL-derived filter values on initial auto-load so deep-links work (B3). + const initialParams: ParameterValues = { + ...initialFilterValuesRef.current, + available_from: initialFilterValuesRef.current.available_from ?? new Date(), }; - loadListings(defaultParams); + loadListings(initialParams); }, [user, loadListings]); const handleTaskCompleted = useCallback(() => { @@ -375,6 +436,12 @@ function AppContent() { } const onSubmit = async (action: 'fetch-data' | 'visualize', parameters: ParameterValues) => { + // Persist filter state to URL on every submit so reload + share-URL works (B3). + setUrlFilterValues(parameters); + // Reflect listing-type changes from the form into the parent state (single source of truth). + if (parameters.listing_type !== listingType) { + setListingType(parameters.listing_type); + } if (action === 'visualize') { loadListings(parameters); } else if (action === 'fetch-data') { @@ -409,7 +476,7 @@ function AppContent() { // Optionally: pan map to coordinates }; - /** Handle removing a filter chip: reset the field to its default value and re-submit */ + /** Handle removing a filter chip: reset the field, sync form + URL, then re-submit (B8). */ const handleRemoveChip = (key: keyof ParameterValues) => { if (!queryParameters) return; const updated = { ...queryParameters }; @@ -431,9 +498,26 @@ function AppContent() { default: (updated as Record)[key] = (DEFAULT_FILTER_VALUES as Record)[key]; } + // Reset the form so the popover inputs and the "More (N)" badge stay in sync (B8). + filterBarFormRef.current?.reset(updated); + setUrlFilterValues(updated); loadListings(updated); }; + /** Reset every filter to defaults (B17). Clears URL, form, and re-loads. */ + const handleResetAllFilters = () => { + const defaults: ParameterValues = { + ...DEFAULT_FILTER_VALUES, + available_from: new Date(), + }; + filterBarFormRef.current?.reset(defaults); + setUrlFilterValues(defaults); + if (defaults.listing_type !== listingType) { + setListingType(defaults.listing_type); + } + loadListings(defaults); + }; + const renderMainContent = () => { if (!processedListingData) { return ( @@ -607,6 +691,7 @@ function AppContent() { userPOIs={userPOIs} poiTravelFilters={poiTravelFilters} onPoiTravelFiltersChange={setPoiTravelFilters} + initialValues={initialFilterValuesRef.current} />
@@ -687,24 +772,28 @@ function AppContent() { ) : ( /* Desktop layout: no sidebar, full-width main area */ <> - {/* Horizontal Filter Bar */} - + {/* Horizontal Filter Bar with adjacent Review entry (B12) */} +
+ +
{/* Active Filter Chips */} {queryParameters && ( @@ -712,6 +801,7 @@ function AppContent() { values={queryParameters} defaults={{ ...DEFAULT_FILTER_VALUES, available_from: new Date() }} onRemove={handleRemoveChip} + onResetAll={handleResetAllFilters} /> )} @@ -727,8 +817,21 @@ function AppContent() { {/* Main content area (full width) */}
{/* Map/List Container */} -
+
{renderMainContent()} + {/* Desktop Swipe / Review entry point (B12) */} +
{/* Stats Bar with Metric Selector */} diff --git a/frontend/src/__tests__/setup.ts b/frontend/src/__tests__/setup.ts index d5156b0..485665d 100644 --- a/frontend/src/__tests__/setup.ts +++ b/frontend/src/__tests__/setup.ts @@ -26,6 +26,20 @@ if (typeof globalThis.ResizeObserver === 'undefined') { }; } +// Polyfill IntersectionObserver for jsdom (used by embla-carousel internally) +if (typeof globalThis.IntersectionObserver === 'undefined') { + // @ts-expect-error minimal jsdom stub + globalThis.IntersectionObserver = class IntersectionObserver { + observe() {} + unobserve() {} + disconnect() {} + takeRecords() { return []; } + root = null; + rootMargin = ''; + thresholds = []; + }; +} + // Mock mapbox-gl (requires WebGL which jsdom doesn't support) vi.mock('mapbox-gl', () => ({ default: { diff --git a/frontend/src/components/FilterBar.tsx b/frontend/src/components/FilterBar.tsx index 234f2ed..c6bcfed 100644 --- a/frontend/src/components/FilterBar.tsx +++ b/frontend/src/components/FilterBar.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect, useCallback } from 'react'; +import { useState, useEffect, useCallback, useRef } from 'react'; import { useForm } from 'react-hook-form'; import { zodResolver } from '@hookform/resolvers/zod'; import { z } from 'zod'; @@ -20,6 +20,7 @@ import { } from './FilterPanel'; import type { AuthUser } from '@/auth/types'; import type { POI, POITravelFilter } from '@/types'; +import type { UseFormReturn } from 'react-hook-form'; // ── Zod schema (same as FilterPanel) ── const formSchema = z.object({ @@ -41,6 +42,13 @@ const formSchema = z.object({ type FormValues = z.infer; // ── Props ── +export interface FilterBarFormHandle { + /** Reset the form to the given values (used by parent to mirror URL → form). */ + reset: (values: ParameterValues) => void; + /** Direct access to the form (used by sibling components like FilterChips). */ + form: UseFormReturn; +} + interface FilterBarProps { onSubmit: (action: 'fetch-data' | 'visualize', parameters: ParameterValues) => void; isLoading: boolean; @@ -49,6 +57,7 @@ interface FilterBarProps { onPOIsChange: (pois: POI[]) => void; poiTravelFilters: Record; onPoiTravelFiltersChange: (filters: Record) => void; + /** Controlled listing type (URL-driven). FilterBar reflects this value, never the inverse. */ listingType: ListingType; onListingTypeChange: (type: ListingType) => void; poiPickerActive: boolean; @@ -57,15 +66,50 @@ interface FilterBarProps { onPickedPoiLocationChange: (loc: { lat: number; lng: number } | null) => void; currentMetric: Metric; onTaskCreated?: (taskId: string) => void; + /** Initial filter values (typically read from URL). Used once for defaultValues. */ + initialValues?: ParameterValues; + /** Provides parent access to the form handle (e.g. for chip-remove resets). */ + onFormReady?: (handle: FilterBarFormHandle) => void; } // ── Helpers ── -function formatPrice(v: number): string { +/** + * Render a price as a compact label suitable for the chip-trigger button. + * For values below 10,000 we keep one decimal so a small change like 1500 -> 2000 + * doesn't visually collide with the next-thousand value (e.g. 2k). + * Sub-1k values render as the raw integer. + */ +export function formatPrice(v: number): string { if (v >= 1_000_000) return `\u00A3${(v / 1_000_000).toFixed(1)}M`; - if (v >= 1_000) return `\u00A3${(v / 1_000).toFixed(0)}k`; + if (v >= 10_000) return `\u00A3${(v / 1_000).toFixed(0)}k`; + if (v >= 1_000) { + const k = v / 1_000; + // Drop trailing .0 (so 2000 stays "2k", not "2.0k") + const label = Number.isInteger(k) ? `${k}` : k.toFixed(1); + return `\u00A3${label}k`; + } return `\u00A3${v}`; } +/** Convert a ParameterValues to FormValues. ParameterValues may omit available_from; default to now. */ +function toFormValues(values: ParameterValues): FormValues { + return { + listing_type: values.listing_type, + min_bedrooms: values.min_bedrooms, + max_bedrooms: values.max_bedrooms, + min_price: values.min_price, + max_price: values.max_price, + min_sqm: values.min_sqm, + max_sqm: values.max_sqm, + min_price_per_sqm: values.min_price_per_sqm, + max_price_per_sqm: values.max_price_per_sqm, + last_seen_days: values.last_seen_days, + available_from: values.available_from ?? new Date(), + district: values.district ?? '', + furnish_types: values.furnish_types, + }; +} + /** Read current ParameterValues from the form state (merges metric and furnish) */ function readFormParams( values: FormValues, @@ -94,57 +138,84 @@ export function FilterBar({ onPickedPoiLocationChange, currentMetric, onTaskCreated, + initialValues, + onFormReady, }: FilterBarProps) { - const [selectedFurnishTypes, setSelectedFurnishTypes] = useState([]); + const [selectedFurnishTypes, setSelectedFurnishTypes] = useState( + initialValues?.furnish_types ?? [], + ); const [availableFromRawInput, setAvailableFromRawInput] = useState('now'); + // Compute defaultValues ONCE from initialValues (or DEFAULT_FILTER_VALUES). React-Hook-Form + // does not re-read defaultValues after mount — use form.reset() for runtime updates. const form = useForm({ resolver: zodResolver(formSchema), - defaultValues: { - listing_type: DEFAULT_FILTER_VALUES.listing_type, - min_bedrooms: DEFAULT_FILTER_VALUES.min_bedrooms, - max_bedrooms: DEFAULT_FILTER_VALUES.max_bedrooms, - min_price: DEFAULT_FILTER_VALUES.min_price, - max_price: DEFAULT_FILTER_VALUES.max_price, - min_sqm: DEFAULT_FILTER_VALUES.min_sqm, - max_sqm: undefined, - min_price_per_sqm: undefined, - max_price_per_sqm: undefined, - last_seen_days: DEFAULT_FILTER_VALUES.last_seen_days, - available_from: new Date(), - district: '', - }, + defaultValues: initialValues + ? toFormValues(initialValues) + : { + listing_type: DEFAULT_FILTER_VALUES.listing_type, + min_bedrooms: DEFAULT_FILTER_VALUES.min_bedrooms, + max_bedrooms: DEFAULT_FILTER_VALUES.max_bedrooms, + min_price: DEFAULT_FILTER_VALUES.min_price, + max_price: DEFAULT_FILTER_VALUES.max_price, + min_sqm: DEFAULT_FILTER_VALUES.min_sqm, + max_sqm: undefined, + min_price_per_sqm: undefined, + max_price_per_sqm: undefined, + last_seen_days: DEFAULT_FILTER_VALUES.last_seen_days, + available_from: new Date(), + district: '', + }, }); - const watchedListingType = form.watch('listing_type'); + // ── Single source of truth for listing type ── + // The PARENT owns listingType; FilterBar mirrors it one-way. + // The header's BUY/RENT tabs call onListingTypeChange (parent setter), which causes a re-render + // with the new prop. We then push it into the form via the prop-driven effect below. + // + // We deliberately do NOT have a `form → parent` sync effect; instead any in-form control that + // wants to change listing_type calls onListingTypeChange directly (no internal listing_type tabs + // exist in FilterBar). This breaks the ping-pong loop that caused B2. + const previousListingTypeRef = useRef(form.getValues('listing_type')); - // Sync listing type with parent useEffect(() => { - if (watchedListingType !== listingType) { - onListingTypeChange(watchedListingType); - } - }, [watchedListingType, listingType, onListingTypeChange]); + if (form.getValues('listing_type') === listingType) return; - // Sync parent listing type changes back into form - useEffect(() => { - if (listingType !== form.getValues('listing_type')) { - form.setValue('listing_type', listingType); + // Reflect parent listing type in the form. + form.setValue('listing_type', listingType, { shouldDirty: false }); + + // On a real transition (not the mount-time no-op above), reset price defaults + // and clear furnish (BUY only). This runs at most once per listing-type change. + if (previousListingTypeRef.current !== listingType) { + if (listingType === ListingType.BUY) { + form.setValue('min_price', 300000, { shouldDirty: false }); + form.setValue('max_price', 600000, { shouldDirty: false }); + setSelectedFurnishTypes([]); + } else { + form.setValue('min_price', 2000, { shouldDirty: false }); + form.setValue('max_price', 3000, { shouldDirty: false }); + } + previousListingTypeRef.current = listingType; } }, [listingType, form]); - // Price defaults when listing type changes + // Expose form handle to parent for chip-remove resets etc. useEffect(() => { - if (watchedListingType === ListingType.BUY) { - form.setValue('min_price', 300000); - form.setValue('max_price', 600000); - } else { - form.setValue('min_price', 2000); - form.setValue('max_price', 3000); - } - if (watchedListingType === ListingType.BUY) { - setSelectedFurnishTypes([]); - } - }, [watchedListingType, form]); + if (!onFormReady) return; + onFormReady({ + reset: (values: ParameterValues) => { + form.reset(toFormValues(values)); + setSelectedFurnishTypes(values.furnish_types ?? []); + // Keep the local transition guard in sync so the next listingType prop change + // doesn't trigger the price-defaults branch erroneously. + previousListingTypeRef.current = values.listing_type; + }, + form, + }); + }, [form, onFormReady]); + + // `watchedListingType` is still used to drive conditional UI (e.g. furnish section, POI manager). + const watchedListingType = form.watch('listing_type'); const handleFormSubmit = useCallback( (action: 'fetch-data' | 'visualize') => { @@ -417,6 +488,7 @@ export function FilterBar({ void; + /** Optional handler for the "Reset all" affordance — clears every active filter to defaults. */ + onResetAll?: () => void; } /** Format a price value for display */ @@ -98,13 +100,13 @@ function buildChips(values: ParameterValues, defaults: ParameterValues): ChipDef return chips; } -export function FilterChips({ values, defaults, onRemove }: FilterChipsProps) { +export function FilterChips({ values, defaults, onRemove, onResetAll }: FilterChipsProps) { const chips = buildChips(values, defaults); if (chips.length === 0) return null; return ( -
+
{chips.map((chip) => ( ))} + {onResetAll && chips.length > 0 && ( + + )}
); } diff --git a/frontend/src/components/FilterPanel.tsx b/frontend/src/components/FilterPanel.tsx index b7d922f..54730a6 100644 --- a/frontend/src/components/FilterPanel.tsx +++ b/frontend/src/components/FilterPanel.tsx @@ -1,5 +1,5 @@ import { zodResolver } from "@hookform/resolvers/zod"; -import { useState, useEffect } from "react"; +import { useState, useEffect, useRef } from "react"; import { useForm } from "react-hook-form"; import { z } from "zod"; import { Button } from "./ui/button"; @@ -82,6 +82,8 @@ interface FilterPanelProps { userPOIs?: POI[]; poiTravelFilters?: Record; onPoiTravelFiltersChange?: (filters: Record) => void; + /** Initial filter values (typically read from URL). Used once for defaultValues. */ + initialValues?: ParameterValues; } const formSchema = z.object({ @@ -110,45 +112,64 @@ const PRICE_BOUNDS = { const BEDROOM_BOUNDS = { min: 0, max: 10, step: 1 } as const; -export function FilterPanel({ onSubmit, currentMetric, isLoading, listingCount, user, onTaskCreated, onStartPoiPicking, pickedPoiLocation, userPOIs, poiTravelFilters, onPoiTravelFiltersChange }: FilterPanelProps) { +export function FilterPanel({ onSubmit, currentMetric, isLoading, listingCount, user, onTaskCreated, onStartPoiPicking, pickedPoiLocation, userPOIs, poiTravelFilters, onPoiTravelFiltersChange, initialValues }: FilterPanelProps) { const [availableFromRawInput, setAvailableFromRawInput] = useState("now"); - const [selectedFurnishTypes, setSelectedFurnishTypes] = useState([]); + const [selectedFurnishTypes, setSelectedFurnishTypes] = useState( + initialValues?.furnish_types ?? [], + ); const form = useForm({ resolver: zodResolver(formSchema), - defaultValues: { - listing_type: DEFAULT_FILTER_VALUES.listing_type, - min_bedrooms: DEFAULT_FILTER_VALUES.min_bedrooms, - max_bedrooms: DEFAULT_FILTER_VALUES.max_bedrooms, - min_price: DEFAULT_FILTER_VALUES.min_price, - max_price: DEFAULT_FILTER_VALUES.max_price, - min_sqm: DEFAULT_FILTER_VALUES.min_sqm, - max_sqm: undefined, - min_price_per_sqm: undefined, - max_price_per_sqm: undefined, - last_seen_days: DEFAULT_FILTER_VALUES.last_seen_days, - available_from: new Date(), - district: '', - }, + defaultValues: initialValues + ? { + listing_type: initialValues.listing_type, + min_bedrooms: initialValues.min_bedrooms, + max_bedrooms: initialValues.max_bedrooms, + min_price: initialValues.min_price, + max_price: initialValues.max_price, + min_sqm: initialValues.min_sqm, + max_sqm: initialValues.max_sqm, + min_price_per_sqm: initialValues.min_price_per_sqm, + max_price_per_sqm: initialValues.max_price_per_sqm, + last_seen_days: initialValues.last_seen_days, + available_from: initialValues.available_from ?? new Date(), + district: initialValues.district ?? '', + furnish_types: initialValues.furnish_types, + } + : { + listing_type: DEFAULT_FILTER_VALUES.listing_type, + min_bedrooms: DEFAULT_FILTER_VALUES.min_bedrooms, + max_bedrooms: DEFAULT_FILTER_VALUES.max_bedrooms, + min_price: DEFAULT_FILTER_VALUES.min_price, + max_price: DEFAULT_FILTER_VALUES.max_price, + min_sqm: DEFAULT_FILTER_VALUES.min_sqm, + max_sqm: undefined, + min_price_per_sqm: undefined, + max_price_per_sqm: undefined, + last_seen_days: DEFAULT_FILTER_VALUES.last_seen_days, + available_from: new Date(), + district: '', + }, }); // Watch listing_type to make filters type-aware const watchedListingType = form.watch('listing_type'); const priceBounds = PRICE_BOUNDS[watchedListingType]; - // Update price defaults when listing type changes + // Update price defaults ONLY on real listing-type transitions, not on every render. + // Use a ref to detect actual changes (mount-time render is a no-op). + const previousListingTypeRef = useRef(watchedListingType); useEffect(() => { + if (watchedListingType === previousListingTypeRef.current) return; + previousListingTypeRef.current = watchedListingType; if (watchedListingType === ListingType.BUY) { form.setValue('min_price', 300000); form.setValue('max_price', 600000); + setSelectedFurnishTypes([]); } else { form.setValue('min_price', 2000); form.setValue('max_price', 3000); } - // Clear furnish types when switching to BUY - if (watchedListingType === ListingType.BUY) { - setSelectedFurnishTypes([]); - } }, [watchedListingType, form]); const handleFormSubmit = (action: 'fetch-data' | 'visualize') => { @@ -451,6 +472,7 @@ export function FilterPanel({ onSubmit, currentMetric, isLoading, listingCount, )} - {/* Agency */} + {/* Agency — wrapped in a labelled block so the Overview tab doesn't + collapse to just "Foxtons" when description/key_features/floorplans are empty. */} {detail.agency && ( -
- - {detail.agency} +
+

Listed by

+
+ + {detail.agency} +
)} diff --git a/frontend/src/components/ListingDetailSheet.tsx b/frontend/src/components/ListingDetailSheet.tsx index ef63ec4..9e5b3e6 100644 --- a/frontend/src/components/ListingDetailSheet.tsx +++ b/frontend/src/components/ListingDetailSheet.tsx @@ -47,6 +47,9 @@ export function ListingDetailSheet({ Listing Details + + Property details including price, location, photos, travel times, and price history. +
{isLoading && ( diff --git a/frontend/src/components/Map.tsx b/frontend/src/components/Map.tsx index 3cb2adb..71796aa 100644 --- a/frontend/src/components/Map.tsx +++ b/frontend/src/components/Map.tsx @@ -85,11 +85,18 @@ export function Map(props: MapProps) { // Pass all features to the heatmap — filtering is done server-side heatmap.setData(data); - // Compute color scale in worker (sorts + percentiles off main thread) + // Compute color scale in worker (sorts + percentiles off main thread). + // If the client is torn down mid-flight (rapid view-mode switch), the worker + // promise rejects with "HexgridHeatmapClient destroyed" — silently swallow + // that case rather than letting it surface as a pageerror (Bug B11). const colorResult = await heatmap.computeColorScale(metricMode, { minBound: PERCENTILE_CONFIG.MIN_BOUND, maxBound: PERCENTILE_CONFIG.MAX_BOUND, - }); + }).catch(() => null); + + // If the heatmap was destroyed (different ref) or the promise was cancelled, + // bail out — the component is unmounting or recreating. + if (!colorResult || heatmapRef.current !== heatmap) return; if (colorResult.hasValues) { makeLegend(colorScheme, colorResult.min, colorResult.max); @@ -111,20 +118,28 @@ export function Map(props: MapProps) { const boundsResult = await heatmap.computeBounds({ clipMin: PERCENTILE_CONFIG.BOUNDS_CLIP_MIN, clipMax: PERCENTILE_CONFIG.BOUNDS_CLIP_MAX, - }); + }).catch(() => null); - mapRef.current?.fitBounds([ - [boundsResult.minLng, boundsResult.minLat], - [boundsResult.maxLng, boundsResult.maxLat] - ], { duration: 0 }); + if (boundsResult && heatmapRef.current === heatmap) { + mapRef.current?.fitBounds([ + [boundsResult.minLng, boundsResult.minLat], + [boundsResult.maxLng, boundsResult.maxLat] + ], { duration: 0 }); + } } lastDataLengthRef.current = data.features.length; }, [data, metricMode, colorScheme]); + // Track whether the Mapbox token is configured. When missing we render a banner + // inside the map container instead of letting Mapbox 404 on the style request + // (B19/B29). + const isMapboxTokenMissing = !MAP_CONFIG.MAPBOX_TOKEN; + // Initialize map useEffect(() => { if (!mapContainerRef.current) return; + if (isMapboxTokenMissing) return; mapboxgl.accessToken = MAP_CONFIG.MAPBOX_TOKEN; mapRef.current = new mapboxgl.Map({ @@ -199,7 +214,7 @@ export function Map(props: MapProps) { mapRef.current?.remove(); }; // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }, [isMapboxTokenMissing]); // Debounced update effect - only update after 200ms of no changes useEffect(() => { @@ -417,6 +432,15 @@ export function Map(props: MapProps) { return (
+ {isMapboxTokenMissing && ( +
+ Map unavailable — set VITE_MAPBOX_TOKEN to enable the basemap. +
+ )} {props.isPickingPOI && (
diff --git a/frontend/src/components/MobileBottomSheet.tsx b/frontend/src/components/MobileBottomSheet.tsx index 79a0fb4..355f9db 100644 --- a/frontend/src/components/MobileBottomSheet.tsx +++ b/frontend/src/components/MobileBottomSheet.tsx @@ -81,6 +81,9 @@ export function MobileBottomSheet({ style={{ maxHeight: '85vh' }} > Property Listings + + Swipeable list of matching properties. Drag the handle to expand or collapse the sheet. + {/* Drag handle */}
diff --git a/frontend/src/components/PhotoCarousel.tsx b/frontend/src/components/PhotoCarousel.tsx index 7cd72c8..898885a 100644 --- a/frontend/src/components/PhotoCarousel.tsx +++ b/frontend/src/components/PhotoCarousel.tsx @@ -1,6 +1,6 @@ import { useState, useCallback, useEffect } from 'react'; import useEmblaCarousel from 'embla-carousel-react'; -import { ChevronLeft, ChevronRight } from 'lucide-react'; +import { ChevronLeft, ChevronRight, ImageOff } from 'lucide-react'; import type { ListingDetailPhoto } from '@/types'; interface PhotoCarouselProps { @@ -8,8 +8,13 @@ interface PhotoCarouselProps { } export function PhotoCarousel({ photos }: PhotoCarouselProps) { - const [emblaRef, emblaApi] = useEmblaCarousel({ loop: true }); + // Only enable Embla loop when there are multiple photos. Looping on a single + // image lets the user drag it off-screen and reveal it sliding back (B26). + const hasMultiple = photos.length > 1; + const [emblaRef, emblaApi] = useEmblaCarousel({ loop: hasMultiple }); const [selectedIndex, setSelectedIndex] = useState(0); + // Track which photo URLs failed so we can swap them for a placeholder (B23). + const [brokenIndexes, setBrokenIndexes] = useState>(() => new Set()); const onSelect = useCallback(() => { if (!emblaApi) return; @@ -22,6 +27,39 @@ export function PhotoCarousel({ photos }: PhotoCarouselProps) { return () => { emblaApi.off('select', onSelect); }; }, [emblaApi, onSelect]); + // Keyboard navigation: ArrowLeft / ArrowRight advance the active slide (B16). + // Embla doesn't ship a built-in keyboard plugin in this repo's deps, so we + // attach a focus-scoped listener instead of registering a global window key. + useEffect(() => { + if (!emblaApi || !hasMultiple) return; + const root = emblaApi.rootNode(); + if (!root) return; + // Make the carousel focusable so it can receive key events. + if (root.tabIndex === -1) { + root.tabIndex = 0; + } + const handleKey = (e: KeyboardEvent) => { + if (e.key === 'ArrowLeft') { + e.preventDefault(); + emblaApi.scrollPrev(); + } else if (e.key === 'ArrowRight') { + e.preventDefault(); + emblaApi.scrollNext(); + } + }; + root.addEventListener('keydown', handleKey); + return () => { root.removeEventListener('keydown', handleKey); }; + }, [emblaApi, hasMultiple]); + + const handleImgError = useCallback((i: number) => { + setBrokenIndexes((prev) => { + if (prev.has(i)) return prev; + const next = new Set(prev); + next.add(i); + return next; + }); + }, []); + if (photos.length === 0) { return (
@@ -32,22 +70,35 @@ export function PhotoCarousel({ photos }: PhotoCarouselProps) { return (
-
+
{photos.map((photo, i) => (
- {photo.caption + {brokenIndexes.has(i) ? ( +
+ + Photo unavailable +
+ ) : ( + {photo.caption handleImgError(i)} + /> + )}
))}
{/* Prev/Next arrows */} - {photos.length > 1 && ( + {hasMultiple && ( <>
diff --git a/frontend/src/components/PropertyCard.tsx b/frontend/src/components/PropertyCard.tsx index 663b088..5e1d539 100644 --- a/frontend/src/components/PropertyCard.tsx +++ b/frontend/src/components/PropertyCard.tsx @@ -2,7 +2,7 @@ import { useState, useCallback, useEffect } from 'react'; import useEmblaCarousel from 'embla-carousel-react'; import { ExternalLink, Footprints, Bike, Train } from 'lucide-react'; import type { PropertyProperties, POIDistanceInfo, POI } from '@/types'; -import { formatDuration } from '@/utils/format'; +import { formatDuration, formatPrice, formatInteger, formatPricePerSqmShort, isFiniteNumber, EM_DASH } from '@/utils/format'; function TravelModeIcon({ mode }: { mode: string }) { switch (mode) { @@ -13,15 +13,16 @@ function TravelModeIcon({ mode }: { mode: string }) { } } +const TRAVEL_MODES: Array<'WALK' | 'BICYCLE' | 'TRANSIT'> = ['WALK', 'BICYCLE', 'TRANSIT']; + function POIDistanceBadges({ distances }: { distances: POIDistanceInfo[] }) { if (!distances || distances.length === 0) return null; - // Group by POI name - const byPoi = new Map(); + // Group by POI name, indexing by travel_mode for consistent rendering. + const byPoi = new Map>(); for (const d of distances) { - const existing = byPoi.get(d.poi_name) || []; - existing.push(d); - byPoi.set(d.poi_name, existing); + if (!byPoi.has(d.poi_name)) byPoi.set(d.poi_name, new Map()); + byPoi.get(d.poi_name)!.set(d.travel_mode, d); } return ( @@ -29,20 +30,21 @@ function POIDistanceBadges({ distances }: { distances: POIDistanceInfo[] }) { {Array.from(byPoi.entries()).map(([poiName, dists]) => (
{poiName}: - {dists.map(d => ( - - - {formatDuration(d.duration_seconds)} - - ))} + {TRAVEL_MODES.map(mode => { + const d = dists.get(mode); + return ( + + + {d ? formatDuration(d.duration_seconds) : EM_DASH} + + ); + })}
))}
); } -const TRAVEL_MODES: Array<'WALK' | 'BICYCLE' | 'TRANSIT'> = ['WALK', 'BICYCLE', 'TRANSIT']; - function AllPOIDistances({ pois, distances }: { pois: POI[]; distances?: POIDistanceInfo[] }) { // Index distances by poi_id + travel_mode for O(1) lookup const distMap = new Map(); @@ -73,7 +75,10 @@ function AllPOIDistances({ pois, distances }: { pois: POI[]; distances?: POIDist } function CardCarousel({ photos, altText }: { photos: string[]; altText?: string }) { - const [emblaRef, emblaApi] = useEmblaCarousel({ loop: true }); + // Only loop when there's more than one image (single-image carousels should + // be static — mirrors PhotoCarousel B26). + const hasMultiple = photos.length > 1; + const [emblaRef, emblaApi] = useEmblaCarousel({ loop: hasMultiple }); const [selectedIndex, setSelectedIndex] = useState(0); const onSelect = useCallback(() => { @@ -87,7 +92,29 @@ function CardCarousel({ photos, altText }: { photos: string[]; altText?: string return () => { emblaApi.off('select', onSelect); }; }, [emblaApi, onSelect]); - if (photos.length <= 1) { + // Keyboard nav for the card carousel (B16). Listener is scoped to the + // embla root so it only fires when the user focuses this carousel. + useEffect(() => { + if (!emblaApi || !hasMultiple) return; + const root = emblaApi.rootNode(); + if (!root) return; + if (root.tabIndex === -1) { + root.tabIndex = 0; + } + const handleKey = (e: KeyboardEvent) => { + if (e.key === 'ArrowLeft') { + e.preventDefault(); + emblaApi.scrollPrev(); + } else if (e.key === 'ArrowRight') { + e.preventDefault(); + emblaApi.scrollNext(); + } + }; + root.addEventListener('keydown', handleKey); + return () => { root.removeEventListener('keydown', handleKey); }; + }, [emblaApi, hasMultiple]); + + if (!hasMultiple) { return ( e.stopPropagation()}> -
+
{photos.map((url, i) => (
@@ -145,22 +177,28 @@ export function PropertyCard({ allPOIs, onClick, }: PropertyCardProps) { - // BUY listings may have null numeric / date fields; coerce so renders don't throw. + // BUY listings may have null numeric / date fields; render "—" at the JSX leaf + // when the source is null/undefined/non-finite so the user can't mistake a missing + // value for a real £0 / 0 m². const lastSeenRaw = property.last_seen; const lastSeenDate = typeof lastSeenRaw === 'string' ? lastSeenRaw.split('T')[0] : null; const lastSeenTime = lastSeenDate ? new Date(lastSeenDate).getTime() : NaN; - const lastSeenDays = Number.isFinite(lastSeenTime) + const lastSeenDaysRaw = Number.isFinite(lastSeenTime) ? Math.round((Date.now() - lastSeenTime) / (1000 * 60 * 60 * 24)) : null; + // Clamp future timestamps to 0 so we don't render "-7d ago" for stale BUY rows. + const lastSeenDays = lastSeenDaysRaw !== null ? Math.max(0, lastSeenDaysRaw) : null; + // Coerced numerics used only where a number is structurally required (alt text, + // boolean comparisons). All visible numeric leaves use the format helpers. const safeNum = (v: unknown): number => (typeof v === 'number' && Number.isFinite(v) ? v : 0); const safeTotalPrice = safeNum(property.total_price); const safeQm = safeNum(property.qm); - const safeQmprice = safeNum(property.qmprice); const safeRooms = safeNum(property.rooms); - // Determine if this is a good deal - const isGoodDeal = avgPricePerSqm && property.qmprice > 0 && property.qmprice < avgPricePerSqm * 0.9; - const isExpensive = avgPricePerSqm && property.qmprice > avgPricePerSqm * 1.1; + // Determine if this is a good deal (guard requires a finite qmprice > 0) + const qmpriceForCompare = isFiniteNumber(property.qmprice) ? property.qmprice : null; + const isGoodDeal = avgPricePerSqm && qmpriceForCompare !== null && qmpriceForCompare > 0 && qmpriceForCompare < avgPricePerSqm * 0.9; + const isExpensive = avgPricePerSqm && qmpriceForCompare !== null && qmpriceForCompare > avgPricePerSqm * 1.1; const priceIndicator = isGoodDeal ? { dotColor: 'bg-[var(--deal-good)]', label: 'Good deal' } @@ -195,8 +233,8 @@ export function PropertyCard({ {/* Price */}
- £{safeTotalPrice.toLocaleString()} - {property.listing_type !== 'BUY' && ( + {formatPrice(property.total_price)} + {property.listing_type !== 'BUY' && isFiniteNumber(property.total_price) && ( /mo )} @@ -210,11 +248,11 @@ export function PropertyCard({ {/* Key metrics on one line */}
- {safeRooms}bed + {formatInteger(property.rooms)}bed · - {safeQm} m² + {formatInteger(property.qm)} m² · - £{safeQmprice}/m² + {formatPricePerSqmShort(property.qmprice)}
{/* Agency + freshness */} @@ -271,8 +309,8 @@ export function PropertyCard({ {/* Price as dominant element */}
- £{safeTotalPrice.toLocaleString()} - {property.listing_type !== 'BUY' && ( + {formatPrice(property.total_price)} + {property.listing_type !== 'BUY' && isFiniteNumber(property.total_price) && ( /mo )} @@ -286,11 +324,11 @@ export function PropertyCard({ {/* Key metrics on one line */}
- {safeRooms}bed + {formatInteger(property.rooms)}bed · - {safeQm} m² + {formatInteger(property.qm)} m² · - £{safeQmprice}/m² + {formatPricePerSqmShort(property.qmprice)}
{/* Location */} diff --git a/frontend/src/components/PropertyCardCompact.tsx b/frontend/src/components/PropertyCardCompact.tsx index f6d0d99..8f334f2 100644 --- a/frontend/src/components/PropertyCardCompact.tsx +++ b/frontend/src/components/PropertyCardCompact.tsx @@ -1,5 +1,6 @@ import { Bed, MapPin } from 'lucide-react'; import type { PropertyProperties } from '@/types'; +import { formatPrice, formatInteger, isFiniteNumber } from '@/utils/format'; interface PropertyCardCompactProps { property: PropertyProperties; @@ -16,15 +17,15 @@ export function PropertyCardCompact({ avgPricePerSqm, onClick, }: PropertyCardCompactProps) { - // BUY listings may have null numeric fields; coerce so renders don't throw. + // BUY listings may have null numeric fields; render "—" at the JSX leaf for + // missing values rather than coercing to 0 (which renders as "£0 / 0 m²"). const safeNum = (v: unknown): number => (typeof v === 'number' && Number.isFinite(v) ? v : 0); const safeTotalPrice = safeNum(property.total_price); - const safeQm = safeNum(property.qm); - const safeQmprice = safeNum(property.qmprice); const safeRooms = safeNum(property.rooms); - const isGoodDeal = avgPricePerSqm && safeQmprice > 0 && safeQmprice < avgPricePerSqm * 0.9; - const isExpensive = avgPricePerSqm && safeQmprice > avgPricePerSqm * 1.1; + const qmpriceForCompare = isFiniteNumber(property.qmprice) ? property.qmprice : null; + const isGoodDeal = avgPricePerSqm && qmpriceForCompare !== null && qmpriceForCompare > 0 && qmpriceForCompare < avgPricePerSqm * 0.9; + const isExpensive = avgPricePerSqm && qmpriceForCompare !== null && qmpriceForCompare > avgPricePerSqm * 1.1; const priceIndicator = isGoodDeal ? { dotColor: 'bg-[var(--deal-good)]', label: 'Good deal' } @@ -55,8 +56,8 @@ export function PropertyCardCompact({ {/* Price bold */}
- £{safeTotalPrice.toLocaleString()} - {property.listing_type !== 'BUY' && ( + {formatPrice(property.total_price)} + {property.listing_type !== 'BUY' && isFiniteNumber(property.total_price) && ( /mo )} @@ -69,10 +70,10 @@ export function PropertyCardCompact({
- {safeRooms} bed + {formatInteger(property.rooms)} bed · - {safeQm} m² + {formatInteger(property.qm)} m²
{/* Location */} diff --git a/frontend/src/components/SwipeCard.tsx b/frontend/src/components/SwipeCard.tsx index e080fef..f23911c 100644 --- a/frontend/src/components/SwipeCard.tsx +++ b/frontend/src/components/SwipeCard.tsx @@ -4,6 +4,7 @@ import { useDrag } from '@use-gesture/react'; import useEmblaCarousel from 'embla-carousel-react'; import { Bed, Maximize2, ExternalLink, ChevronLeft, ChevronRight, Building2, Calendar } from 'lucide-react'; import type { PropertyFeature } from '@/types'; +import { formatPrice, formatInteger, formatPricePerSqmShort, formatDuration, isFiniteNumber } from '@/utils/format'; interface SwipeCardProps { feature: PropertyFeature; @@ -19,11 +20,12 @@ export function SwipeCard({ feature, onSwipe, onTap, isTop, stackIndex }: SwipeC const hasSwiped = useRef(false); const p = feature.properties; const photos = p.photos?.length ? p.photos : p.photo_thumbnail ? [p.photo_thumbnail] : []; - // BUY listings may have null numeric fields; coerce so renders don't throw. + // BUY listings may have null numeric fields; render "—" at the JSX leaf for + // missing values rather than coercing to 0. safeNum-coerced values are still used + // for alt text (where rendering "£0" is acceptable for non-visible content). const safeNum = (v: unknown): number => (typeof v === 'number' && Number.isFinite(v) ? v : 0); const safeTotalPrice = safeNum(p.total_price); const safeQm = safeNum(p.qm); - const safeQmprice = safeNum(p.qmprice); const safeRooms = safeNum(p.rooms); const prefersReducedMotion = useMemo( @@ -177,8 +179,8 @@ export function SwipeCard({ feature, onSwipe, onTap, isTop, stackIndex }: SwipeC > {/* Price */}
- £{safeTotalPrice.toLocaleString()} - {p.listing_type !== 'BUY' && ( + {formatPrice(p.total_price)} + {p.listing_type !== 'BUY' && isFiniteNumber(p.total_price) && ( /mo )}
@@ -186,12 +188,12 @@ export function SwipeCard({ feature, onSwipe, onTap, isTop, stackIndex }: SwipeC {/* Key stats */}
- {safeRooms} bed + {formatInteger(p.rooms)} bed - {safeQm} m² + {formatInteger(p.qm)} m² - £{safeQmprice}/m² + {formatPricePerSqmShort(p.qmprice)}
{/* Agency & availability */} @@ -216,7 +218,7 @@ export function SwipeCard({ feature, onSwipe, onTap, isTop, stackIndex }: SwipeC key={`${d.poi_id}_${d.travel_mode}`} className="text-xs bg-muted px-2 py-0.5 rounded" > - {d.poi_name}: {Math.round(d.duration_seconds / 60)}m + {d.poi_name}: {formatDuration(d.duration_seconds)} ))}
diff --git a/frontend/src/components/TaskIndicator.tsx b/frontend/src/components/TaskIndicator.tsx index 0f6f960..bd97baa 100644 --- a/frontend/src/components/TaskIndicator.tsx +++ b/frontend/src/components/TaskIndicator.tsx @@ -194,9 +194,11 @@ export function TaskIndicator({ size="icon" onClick={handleCancel} disabled={isCancelling} + aria-label="Cancel task" + data-testid="task-cancel-button" className="h-6 w-6 text-muted-foreground hover:text-destructive" > - +