From e6c1825130964905667c95fda7f63dfaa93630e7 Mon Sep 17 00:00:00 2001 From: Duncan Appleby Date: Mon, 10 Aug 2026 00:40:29 +0100 Subject: [PATCH 01/28] perf: stop rebuilding whole map source per frame; fix stale initial centre Map.tsx's walker animation loop rebuilt the entire GeoJSON source (buildings/roads/fields/boundaries) every 16ms even though only character positions change per frame. Split buildVillageSourceData into buildStaticVillageFeatures (memoized, rebuilt only when features change) and buildCharacterPointFeatures (per-frame). The animation loop is now also gated on there actually being walking characters, instead of running unconditionally. Separately, useMap.ts's initial-camera logic assumed a single seeded village and picked "the first population centre" from the full population-centres list, which is now stale with multiple villages imported from locations/data/. Added a lightweight InitialMapCentreView (/map/initial-centre/) that returns just enough (id/name/bbox) to frame the camera, preferring the population centre of the requesting player's actively linked character (Player.active_link) and falling back to the lowest-pk centre otherwise. Frontend now fetches this cheaply before the bbox-scoped viewport poll takes over as the source of truth. Backend: locations/tests/test_initial_map_centre_view.py (3 tests). Frontend: MapPage.test.tsx updated for the new fetchInitialMapCentre mock; also fixed a flaky prefetch effect that used a dynamic import() for fetchPopulationCentreMap, which could race and skip the vi.mock when prefetching two different villages concurrently - now a normal static import. --- api/urls.py | 6 ++ frontend/src/api/map.ts | 26 ++++-- frontend/src/components/Map/Map.tsx | 56 ++++++++---- frontend/src/components/Map/sourceData.ts | 57 +++++++++--- frontend/src/hooks/useMap.ts | 44 ++++----- frontend/src/pages/MapPage/MapPage.test.tsx | 50 ++++------ frontend/src/pages/MapPage/MapPage.tsx | 40 +++++--- .../tests/test_initial_map_centre_view.py | 91 +++++++++++++++++++ locations/views.py | 48 ++++++++++ 9 files changed, 307 insertions(+), 111 deletions(-) create mode 100644 locations/tests/test_initial_map_centre_view.py diff --git a/api/urls.py b/api/urls.py index 947d9a01..3b94c728 100644 --- a/api/urls.py +++ b/api/urls.py @@ -43,6 +43,7 @@ from locations.views import ( PopulationCentreMapView, + InitialMapCentreView, MapCharacterDetailView, MapViewportView, MapWorldBoundsView, @@ -135,6 +136,11 @@ def to_url(self, value): PopulationCentreMapView.as_view(), name="populationcentre-map", ), + path( + "map/initial-centre/", + InitialMapCentreView.as_view(), + name="map-initial-centre", + ), path( "map/viewport/", MapViewportView.as_view(), diff --git a/frontend/src/api/map.ts b/frontend/src/api/map.ts index 49636b66..36b7b700 100644 --- a/frontend/src/api/map.ts +++ b/frontend/src/api/map.ts @@ -1,18 +1,24 @@ // src/api/map.ts import { apiFetch } from "../utils/api"; -interface PopulationCentreListItem { - id: number; +export interface InitialMapCentre { + id: number | null; + name: string | null; + // [minX, minY, maxX, maxY] in raw EPSG:3857 metres - just enough to frame + // the camera on this village; not the full per-village feature payload + // fetchPopulationCentreMap returns (see InitialMapCentreView's docstring). + bbox: [number, number, number, number] | null; } -type PopulationCentreListResponse = - | { results?: PopulationCentreListItem[] } - | PopulationCentreListItem[]; - -export async function fetchFirstPopulationCentreId(): Promise { - const data = await apiFetch("/population-centres/"); - const list = Array.isArray(data) ? data : (data?.results ?? []); - return list.length > 0 ? list[0].id : null; +// Which village the map's camera should open on - the requesting player's +// linked character's village if they have one, otherwise an arbitrary but +// deterministic fallback (see InitialMapCentreView, locations/views.py). +// Deliberately a separate, lightweight endpoint rather than reusing +// fetchPopulationCentreMap: this only needs to get the camera pointed at the +// right place before useMapViewport (bbox-scoped, polled) takes over as the +// source of truth moments later, once the camera's first move settles. +export function fetchInitialMapCentre(): Promise { + return apiFetch("/map/initial-centre/"); } export interface PopulationCentreSummary { diff --git a/frontend/src/components/Map/Map.tsx b/frontend/src/components/Map/Map.tsx index a901897e..6c6d296a 100644 --- a/frontend/src/components/Map/Map.tsx +++ b/frontend/src/components/Map/Map.tsx @@ -43,7 +43,12 @@ import { TOOLTIP_ONLY_SELECTION_OPACITY, VILLAGE_LABEL_LAYER, } from "./layers"; -import { buildVillageSourceData, type WalkerState } from "./sourceData"; +import { + buildCharacterPointFeatures, + buildStaticVillageFeatures, + buildVillageSourceData, + type WalkerState, +} from "./sourceData"; import MapDetailCard from "../MapDetailCard/MapDetailCard"; import CharacterDetail from "../CharacterDetail/CharacterDetail"; import BuildingDetail from "../BuildingDetail/BuildingDetail"; @@ -212,6 +217,17 @@ export default function PopulationCentreMap({ [features] ); + // Styled buildings/roads/fields/boundaries - everything the map draws + // except characters. Only recomputed when `features` itself changes (each + // ~2s poll), unlike character positions, which the walker loop below + // recomputes on every animation frame - keeping this out of that loop is + // what keeps a village's buildings/roads/fields from being re-styled and + // re-reprojected 60 times a second while nothing about them has changed. + const staticVillageFeatures = useMemo( + () => buildStaticVillageFeatures(features), + [features] + ); + // Lets scatterCharacters spread a field_shelter's idle workers across the // crops Subzone(s) it services instead of clustering them at the // shelter's own small footprint - see scatterCharacters' own comment. @@ -258,16 +274,19 @@ export default function PopulationCentreMap({ const walkersRef = useRef>(new Map()); const refreshVillageSource = useCallback(() => { - sourceRef.current?.setData( - buildVillageSourceData({ - features, - characterFeatures, - idleCharacterPositions, - walkers: walkersRef.current, - now: Date.now(), - }) - ); - }, [features, characterFeatures, idleCharacterPositions]); + sourceRef.current?.setData({ + type: "FeatureCollection", + features: [ + ...staticVillageFeatures, + ...buildCharacterPointFeatures({ + characterFeatures, + idleCharacterPositions, + walkers: walkersRef.current, + now: Date.now(), + }), + ], + }); + }, [staticVillageFeatures, characterFeatures, idleCharacterPositions]); // Creates the map once. onViewportChange and refreshVillageSource are each // read via a ref inside the handlers below rather than as effect deps, so @@ -689,18 +708,19 @@ export default function PopulationCentreMap({ // Each frame recomputes position from scratch - the checkpoint plus how // much time has passed since it was taken - rather than stepping forward // from wherever the previous frame left off, so nothing compounds across - // frames or across polls (see the WalkerState comment above). + // frames or across polls (see the WalkerState comment above). Only runs + // while at least one character actually has an active journey - an idle + // village (the common case) has nothing to animate, so there's no reason + // to keep a 60fps timer alive rebuilding the source every 16ms. useEffect(() => { - const step = () => { - if (mapReady) { - refreshVillageSource(); - } - }; + if (!mapReady || walkingFeatures.length === 0) return; + + const step = () => refreshVillageSource(); step(); const intervalId = window.setInterval(step, 16); return () => window.clearInterval(intervalId); - }, [mapReady, refreshVillageSource]); + }, [mapReady, walkingFeatures.length, refreshVillageSource]); // Outlines whichever building/character the detail card currently has // open (see SELECTED_BUILDING_OUTLINE_LAYER/SELECTED_CHARACTER_HIGHLIGHT_LAYER diff --git a/frontend/src/components/Map/sourceData.ts b/frontend/src/components/Map/sourceData.ts index 1a0da24e..45832b43 100644 --- a/frontend/src/components/Map/sourceData.ts +++ b/frontend/src/components/Map/sourceData.ts @@ -48,22 +48,37 @@ function positionAlongPath( return pos; } -interface BuildVillageSourceDataArgs { - features: GeoJSONFeature[]; +// Styled buildings/roads/fields/boundaries - everything except characters. +// This only changes when `features` itself changes (i.e. once per ~2s poll), +// unlike character positions, which are recomputed on every animation frame +// by the walker loop in Map.tsx. Callers should memoize this separately +// (keyed on `features`) rather than folding it into buildVillageSourceData, +// so that per-frame loop isn't re-styling and re-reprojecting every building/ +// road/field 60 times a second when only the characters are actually moving. +export function buildStaticVillageFeatures(features: GeoJSONFeature[]) { + return [ + ...styledPolygonFeatures(features), + ...styledLineFeatures(features), + ...styledPointFeatures( + features.filter((feature) => feature.properties?.feature_type !== "character") + ), + ]; +} + +interface BuildCharacterPointFeaturesArgs { characterFeatures: GeoJSONFeature[]; idleCharacterPositions: Map; walkers: Map; now: number; } -export function buildVillageSourceData({ - features, +export function buildCharacterPointFeatures({ characterFeatures, idleCharacterPositions, walkers, now, -}: BuildVillageSourceDataArgs) { - const characterPointFeatures: LngLatPointFeature[] = characterFeatures.map((feature) => { +}: BuildCharacterPointFeaturesArgs): LngLatPointFeature[] { + return characterFeatures.map((feature) => { const id = String(feature.properties?.id); const walker = walkers.get(id); const rawPoint = walker @@ -83,16 +98,34 @@ export function buildVillageSourceData({ properties: feature.properties, }; }); +} +interface BuildVillageSourceDataArgs { + features: GeoJSONFeature[]; + characterFeatures: GeoJSONFeature[]; + idleCharacterPositions: Map; + walkers: Map; + now: number; +} + +// Full rebuild of the source's FeatureCollection - static features plus +// current character positions. Used on mount and whenever `features` itself +// changes; the per-frame walker loop in Map.tsx calls +// buildCharacterPointFeatures directly against a memoized +// buildStaticVillageFeatures result instead, since that loop only ever needs +// to update character positions, not the static geometry around them. +export function buildVillageSourceData({ + features, + characterFeatures, + idleCharacterPositions, + walkers, + now, +}: BuildVillageSourceDataArgs) { return { type: "FeatureCollection" as const, features: [ - ...styledPolygonFeatures(features), - ...styledLineFeatures(features), - ...styledPointFeatures( - features.filter((feature) => feature.properties?.feature_type !== "character") - ), - ...characterPointFeatures, + ...buildStaticVillageFeatures(features), + ...buildCharacterPointFeatures({ characterFeatures, idleCharacterPositions, walkers, now }), ], }; } diff --git a/frontend/src/hooks/useMap.ts b/frontend/src/hooks/useMap.ts index 411ba128..24657f98 100644 --- a/frontend/src/hooks/useMap.ts +++ b/frontend/src/hooks/useMap.ts @@ -1,7 +1,7 @@ // src/hooks/useMap.ts import { useQuery } from "@tanstack/react-query"; import { - fetchFirstPopulationCentreId, + fetchInitialMapCentre, fetchMapCharacterDetail, fetchMapViewport, fetchMapWorldBounds, @@ -13,31 +13,23 @@ import { // tracks actual journeys closely, without polling every single tick. export const MAP_POLL_INTERVAL_MS = 2000; -// Player-character linking isn't implemented yet (fetch_info deliberately -// omits it), so the map view can't key off character.population_centre_id. -// Instead it just picks the first population centre - fine while there's -// only ever the one small seeded village. -export function usePopulationCentreId() { +// One-shot (not polled) fetch of just enough (id/name/bbox) to know where +// the camera should start - the requesting player's linked character's +// village if they have one, otherwise an arbitrary but deterministic +// fallback (see InitialMapCentreView's docstring, locations/views.py). +// Deliberately not the full per-village map payload fetchPopulationCentreMap +// returns: once the camera exists, all ongoing content comes from +// useMapViewport below instead, so there's nothing here worth paying for +// beyond the bbox to fit to. A short staleTime (rather than +// effectively-forever) means a player who links to a different character +// mid-session and revisits the map later still gets pointed at the right +// village instead of a stale cached one. +export function useInitialMapCentre() { return useQuery({ - queryKey: ["population-centres", "first-id"], - queryFn: fetchFirstPopulationCentreId, - staleTime: 15 * 60 * 1000, - gcTime: 30 * 60 * 1000, - }); -} - -// One-shot (not polled) fetch of the single existing village's map, used -// only to derive where the camera should start (see design decision #6 in -// the map-viewport plan: initial view centres on the single seeded -// PopulationCentre until multiple villages/player-linking exist). Once the -// camera exists, all ongoing data comes from useMapViewport below instead. -export function useInitialMapCentre(pcId: number | null | undefined) { - return useQuery({ - queryKey: ["map", "population-centre", "initial-centre", pcId], - queryFn: () => fetchPopulationCentreMap(pcId as number), - enabled: pcId != null, - staleTime: 15 * 60 * 1000, - gcTime: 30 * 60 * 1000, + queryKey: ["map", "initial-centre"], + queryFn: fetchInitialMapCentre, + staleTime: 5 * 60 * 1000, + gcTime: 15 * 60 * 1000, }); } @@ -50,7 +42,7 @@ export function useInitialMapCentre(pcId: number | null | undefined) { // the prefetch already completed; only hits the network if it hasn't. export function useTargetCentreMap(centreId: number | null) { return useQuery({ - queryKey: ["map", "population-centre", "initial-centre", centreId], + queryKey: ["map", "population-centre", "full-map", centreId], queryFn: () => fetchPopulationCentreMap(centreId as number), enabled: centreId != null, staleTime: 15 * 60 * 1000, diff --git a/frontend/src/pages/MapPage/MapPage.test.tsx b/frontend/src/pages/MapPage/MapPage.test.tsx index 82a47d67..d0c8b598 100644 --- a/frontend/src/pages/MapPage/MapPage.test.tsx +++ b/frontend/src/pages/MapPage/MapPage.test.tsx @@ -5,15 +5,14 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import MapPage from './MapPage'; -const mockFetchFirstPopulationCentreId = vi.fn(); +const mockFetchInitialMapCentre = vi.fn(); const mockFetchPopulationCentreMap = vi.fn(); const mockFetchMapViewport = vi.fn(); const mockFetchMapWorldBounds = vi.fn(); const mockFetchPopulationCentres = vi.fn(); vi.mock('../../api/map', () => ({ - fetchFirstPopulationCentreId: (...args: unknown[]) => - mockFetchFirstPopulationCentreId(...args), + fetchInitialMapCentre: (...args: unknown[]) => mockFetchInitialMapCentre(...args), fetchPopulationCentreMap: (...args: unknown[]) => mockFetchPopulationCentreMap(...args), fetchMapViewport: (...args: unknown[]) => mockFetchMapViewport(...args), fetchMapWorldBounds: (...args: unknown[]) => mockFetchMapWorldBounds(...args), @@ -60,12 +59,20 @@ function renderMapPage(queryClient?: QueryClient) { describe('MapPage', () => { beforeEach(() => { - mockFetchFirstPopulationCentreId.mockReset(); + mockFetchInitialMapCentre.mockReset(); mockFetchPopulationCentreMap.mockReset(); mockFetchMapViewport.mockReset(); mockFetchMapWorldBounds.mockReset(); mockFetchPopulationCentres.mockReset(); - mockFetchFirstPopulationCentreId.mockResolvedValue(1); + mockFetchInitialMapCentre.mockResolvedValue({ + id: 1, + name: 'Driftmoor', + bbox: [0, 0, 100, 100], + }); + mockFetchPopulationCentreMap.mockResolvedValue({ + meta: { population_centre_name: 'Driftmoor' }, + bbox: [0, 0, 100, 100], + }); mockFetchMapViewport.mockResolvedValue({ meta: { population_centre_name: 'Driftmoor' } }); mockFetchMapWorldBounds.mockResolvedValue({ bbox: [-1000, -1000, 1000, 1000] }); mockFetchPopulationCentres.mockResolvedValue([ @@ -77,23 +84,13 @@ describe('MapPage', () => { vi.useRealTimers(); }); - it('renders the map once the population centre and its initial map data have loaded', async () => { - mockFetchPopulationCentreMap.mockResolvedValue({ - meta: { population_centre_name: 'Driftmoor' }, - bbox: [0, 0, 100, 100], - }); - + it('renders the map once the initial map centre has loaded', async () => { renderMapPage(); expect(await screen.findByTestId('map-stub')).toHaveTextContent('Driftmoor'); }); it('reuses cached viewport data when the page is remounted within the cache window', async () => { - mockFetchPopulationCentreMap.mockResolvedValue({ - meta: { population_centre_name: 'Driftmoor' }, - bbox: [0, 0, 100, 100], - }); - const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false }, @@ -117,10 +114,6 @@ describe('MapPage', () => { }); it('prefetches the next village map data once the village list is available', async () => { - mockFetchPopulationCentreMap.mockResolvedValue({ - meta: { population_centre_name: 'Driftmoor' }, - bbox: [0, 0, 100, 100], - }); mockFetchPopulationCentres.mockResolvedValue([ { id: 1, name: 'Driftmoor village', location: [0, 0] }, { id: 2, name: 'Cedar Hollow', location: [100, 100] }, @@ -134,11 +127,6 @@ describe('MapPage', () => { }); it('does not issue a second viewport request while the previous poll is still in flight (#624)', async () => { - mockFetchPopulationCentreMap.mockResolvedValue({ - meta: { population_centre_name: 'Driftmoor' }, - bbox: [0, 0, 100, 100], - }); - vi.useFakeTimers(); const pending: { resolve: (value: unknown) => void }[] = []; mockFetchMapViewport.mockImplementation( @@ -150,12 +138,12 @@ describe('MapPage', () => { renderMapPage(); - // Flush the population-centre lookup and initial-centre fetch, then the - // stub's onViewportChange call, so the first viewport fetch fires. This - // is a chain of several dependent async hops (pcId -> initial centre -> - // Map mounts -> onViewportChange -> viewport query starts), each of - // which may need its own microtask turn under fake timers, so flush - // repeatedly rather than assuming one pass covers it. + // Flush the initial-centre fetch, then the stub's onViewportChange call, + // so the first viewport fetch fires. This is a chain of several + // dependent async hops (initial centre -> Map mounts -> onViewportChange + // -> viewport query starts), each of which may need its own microtask + // turn under fake timers, so flush repeatedly rather than assuming one + // pass covers it. await act(async () => { await vi.runOnlyPendingTimersAsync(); }); diff --git a/frontend/src/pages/MapPage/MapPage.tsx b/frontend/src/pages/MapPage/MapPage.tsx index 4a94f2f6..fd15e349 100644 --- a/frontend/src/pages/MapPage/MapPage.tsx +++ b/frontend/src/pages/MapPage/MapPage.tsx @@ -6,11 +6,11 @@ import PopulationCentreMap, { type PopulationCentreMapHandle, } from "../../components/Map/Map"; import TodayPointsBadge from "../../components/TodayPointsBadge/TodayPointsBadge"; +import { fetchPopulationCentreMap } from "../../api/map"; import { useInitialMapCentre, useMapViewport, useMapWorldBounds, - usePopulationCentreId, usePopulationCentres, useTargetCentreMap, } from "../../hooks/useMap"; @@ -41,12 +41,12 @@ function HomeIcon(): React.ReactElement { export default function MapPage(): React.ReactElement { const queryClient = useQueryClient(); - const { data: pcId } = usePopulationCentreId(); - // One-time fetch of the single seeded village's map, used only to give the - // camera somewhere to start (see useInitialMapCentre) and to have - // something on screen before the camera has settled on its first - // viewport below. - const { data: initialCentre } = useInitialMapCentre(pcId); + // One-time fetch of just enough (id/name/bbox) to give the camera + // somewhere to start - the player's linked village if they have one, + // otherwise a deterministic fallback (see InitialMapCentreView). Only + // frames the camera; there's no feature data here to show before the + // camera's first viewport below settles (see the `geojson` placeholder). + const { data: initialCentre } = useInitialMapCentre(); const [bbox, setBbox] = useState(null); const { data: viewportGeojson } = useMapViewport(bbox); @@ -65,9 +65,19 @@ export default function MapPage(): React.ReactElement { // Once the map's camera has fitted itself to the initial village and // reported its first viewport (Map.tsx's onViewportChange), the // bbox-scoped viewport poll becomes the source of truth for what's on - // screen; until then, fall back to the mid-flight target (if any) or the - // one-time initial fetch. - const geojson = viewportGeojson ?? targetCentreGeojson ?? initialCentre; + // screen; until then, fall back to the mid-flight target (if any) or a + // feature-less placeholder built from the one-time initial fetch, just so + // Map.tsx has a bbox to fit its first camera position to. + const geojson = + viewportGeojson ?? + targetCentreGeojson ?? + (initialCentre?.bbox + ? { + bbox: initialCentre.bbox, + features: [], + meta: { population_centre_name: initialCentre.name }, + } + : null); const handleViewportChange = (nextBbox: string) => { setBbox(nextBbox); @@ -86,16 +96,18 @@ export default function MapPage(): React.ReactElement { : null; useEffect(() => { - if (!populationCentres?.length || !initialCentre) return; + if (!populationCentres?.length || !initialCentre?.bbox) return; const currentVillage = populationCentres[cycleIndex % populationCentres.length]; const nextVillageToPrefetch = populationCentres[(cycleIndex + 1) % populationCentres.length]; const villagesToPrefetch = [currentVillage, nextVillageToPrefetch].filter(Boolean); for (const village of villagesToPrefetch) { + // Query key must match useTargetCentreMap's, so the prefetch here and + // the read there hit the same cache entry. void queryClient.prefetchQuery({ - queryKey: ["map", "population-centre", "initial-centre", village.id], - queryFn: () => import("../../api/map").then(({ fetchPopulationCentreMap }) => fetchPopulationCentreMap(village.id)), + queryKey: ["map", "population-centre", "full-map", village.id], + queryFn: () => fetchPopulationCentreMap(village.id), staleTime: 15 * 60 * 1000, gcTime: 30 * 60 * 1000, }); @@ -116,7 +128,7 @@ export default function MapPage(): React.ReactElement { visible title (the map itself is the content). */}

{geojson?.meta?.population_centre_name || "Village map"}

- {initialCentre ? ( + {geojson ? ( Date: Mon, 10 Aug 2026 01:01:52 +0100 Subject: [PATCH 02/28] feat: import watabou "squares" as communal-space Subzones Watabou's village generator can export a "squares" MultiPolygon (open outside communal space, e.g. a market square/plaza) that import_watabou_village previously ignored entirely, like "greens" and "prisms" still are. Add a "square" Subzone.usage choice and a _import_squares helper (watabou_import.py) that imports it the same way _import_fields already imports "fields" - one LandArea wrapping the union of the polygons, one Subzone per polygon - refactored the shared logic into _import_polygon_subzones. Unlike a crops Subzone, a square has no FieldCrop growth cycle or other economy behaviour attached; it's purely a map feature. Wire it into both map views (PopulationCentreMapView, MapViewportView): they now query Subzone with usage__in=["crops", "square"] instead of usage="crops" alone, since SubzoneFeatureSerializer already returns None for every crop_* field when there's no attached FieldCrop. Frontend: styledPolygonFeatures gives usage="square" subzones a distinct warm/paved fill (geojson.tsx) instead of falling through to the crops-green/building-grey styling, and the tooltip labels them "Square" instead of the raw bookkeeping name. --- frontend/src/components/Map/geojson.tsx | 11 +++ .../migrations/0011_alter_subzone_usage.py | 30 ++++++++ locations/models.py | 1 + locations/services/watabou_import.py | 73 ++++++++++++++++--- locations/tests/test_map_serializers.py | 15 ++++ locations/tests/test_watabou_import.py | 63 +++++++++++++++- locations/views.py | 24 ++++-- 7 files changed, 197 insertions(+), 20 deletions(-) create mode 100644 locations/migrations/0011_alter_subzone_usage.py diff --git a/frontend/src/components/Map/geojson.tsx b/frontend/src/components/Map/geojson.tsx index a2960472..b5dc544a 100644 --- a/frontend/src/components/Map/geojson.tsx +++ b/frontend/src/components/Map/geojson.tsx @@ -111,6 +111,7 @@ export function polygonTooltipContent( ); } if (properties?.feature_type === "subzone") { + if (properties?.usage === "square") return "Square"; if (properties?.usage !== "crops") return properties?.name; const stage = properties?.crop_stage as string | null | undefined; @@ -125,6 +126,12 @@ export function polygonTooltipContent( return properties?.name; } +// Open communal outdoor space (see watabou_import._import_squares) - a +// warm, paved tone distinct from both a crops Subzone's green (fieldFillFor) +// and a building's default grey, so a plaza reads as open ground rather +// than a structure. +const SQUARE_FILL_COLOR = "#d8c9a8"; + // Precomputes per-feature presentation properties (fill/stroke) so map // styling can stay simple `["get", ...]` paint expressions instead of // duplicating fieldFillFor's stage/progress logic as a style expression. @@ -135,6 +142,8 @@ export function styledPolygonFeatures(features: GeoJSONFeature[]) { const isBoundary = f.properties?.feature_type === "boundary"; const isCropSubzone = f.properties?.feature_type === "subzone" && f.properties?.usage === "crops"; + const isSquareSubzone = + f.properties?.feature_type === "subzone" && f.properties?.usage === "square"; const fillColor = isBoundary ? "transparent" : isCropSubzone @@ -142,6 +151,8 @@ export function styledPolygonFeatures(features: GeoJSONFeature[]) { f.properties?.crop_stage as string | null | undefined, f.properties?.crop_progress as number | null | undefined ) + : isSquareSubzone + ? SQUARE_FILL_COLOR : "#ddd"; return { type: "Feature" as const, diff --git a/locations/migrations/0011_alter_subzone_usage.py b/locations/migrations/0011_alter_subzone_usage.py new file mode 100644 index 00000000..97401e0d --- /dev/null +++ b/locations/migrations/0011_alter_subzone_usage.py @@ -0,0 +1,30 @@ +# Generated by Django 5.2.17 on 2026-08-09 23:56 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("locations", "0010_building_open_time_override_and_more"), + ] + + operations = [ + migrations.AlterField( + model_name="subzone", + name="usage", + field=models.CharField( + choices=[ + ("crops", "Crop Growing"), + ("grazing", "Grazing Land"), + ("foraging", "Foraging"), + ("woodland", "Woodland"), + ("orchard", "Orchard"), + ("square", "Square"), + ("other", "Other"), + ], + default="crops", + max_length=50, + ), + ), + ] diff --git a/locations/models.py b/locations/models.py index 30b77cf9..da145baf 100644 --- a/locations/models.py +++ b/locations/models.py @@ -576,6 +576,7 @@ class Subzone(models.Model): ("foraging", "Foraging"), ("woodland", "Woodland"), ("orchard", "Orchard"), + ("square", "Square"), ("other", "Other"), ], default="crops", diff --git a/locations/services/watabou_import.py b/locations/services/watabou_import.py index 567992db..dc93a276 100644 --- a/locations/services/watabou_import.py +++ b/locations/services/watabou_import.py @@ -14,9 +14,9 @@ This only creates static geometry - PopulationCentre, Building, Road, the Node graph's CENTRE/BUILDING points plus BUILDING_ENTRANCE for non-granary -buildings, and (if the export -has a "fields" feature) a LandArea/Subzone pair per field polygon. It -deliberately does not generate Path edges: Path is the movement/pathfinding +buildings, and (if the export has a "fields" and/or "squares" feature) a +LandArea/Subzone pair per field/square polygon (see _import_polygon_subzones). +It deliberately does not generate Path edges: Path is the movement/pathfinding graph and Road is just the drawn street, so wiring the graph is left to the existing `generate_paths` command (see the `--generate-paths` flag on import_watabou_village's management command) rather than trying to derive @@ -198,19 +198,28 @@ def _translate_linestring(coordinates, offset, srid=3857) -> LineString: return LineString(points, srid=srid) -def _import_fields( - fields_feature: dict, population_centre: PopulationCentre, offset +def _import_polygon_subzones( + feature: dict, + population_centre: PopulationCentre, + offset, + *, + land_area_name: str, + subzone_name: str, + usage: str, ) -> None: """ - Create one LandArea (wrapping the whole imported field area) and one - "crops" Subzone per polygon in the "fields" MultiPolygon - unlike + Create one LandArea (wrapping the whole imported area) and one Subzone + per polygon in a watabou MultiPolygon feature - unlike generate_landarea's procedurally-synthesized Subzone geometry, these polygons are real imported shapes, so they're used directly rather than - derived from a size fraction. + derived from a size fraction. Shared by _import_fields (usage="crops") + and _import_squares (usage="square") below - the only difference + between them is naming and the usage tag, since neither FieldCrop + growth nor any other economy behaviour is usage-specific here. """ polygons = [ _translate_polygon(polygon_coords, offset) - for polygon_coords in fields_feature.get("coordinates", []) + for polygon_coords in feature.get("coordinates", []) ] if not polygons: return @@ -225,7 +234,7 @@ def _import_fields( ) land_area = LandArea.objects.create( - name=f"Fields of ({population_centre.name})", + name=land_area_name, population_centre=population_centre, location=boundary.centroid, boundary=boundary, @@ -235,14 +244,50 @@ def _import_fields( for i, polygon in enumerate(polygons): Subzone.objects.create( land_area=land_area, - name=f"Field {i + 1} of ({population_centre.name})", + name=f"{subzone_name} {i + 1} of ({population_centre.name})", location=polygon.centroid, boundary=polygon, size=polygon.area / SQUARE_METRES_PER_HECTARE, - usage="crops", + usage=usage, ) +def _import_fields( + fields_feature: dict, population_centre: PopulationCentre, offset +) -> None: + _import_polygon_subzones( + fields_feature, + population_centre, + offset, + land_area_name=f"Fields of ({population_centre.name})", + subzone_name="Field", + usage="crops", + ) + + +def _import_squares( + squares_feature: dict, population_centre: PopulationCentre, offset +) -> None: + """ + Import watabou's "squares" feature - open communal outdoor space (a + market square/plaza) rather than property. Modelled the same way as a + crops Subzone (see _import_polygon_subzones) since LandArea is already + documented as "not property... a communal or functional area", but + tagged usage="square" instead of "crops": squares have no FieldCrop + growth cycle or other economy behaviour attached, they're purely a map + feature (see SubzoneFeatureSerializer, which already returns None for + every crop_* property when there's no attached FieldCrop). + """ + _import_polygon_subzones( + squares_feature, + population_centre, + offset, + land_area_name=f"Squares of ({population_centre.name})", + subzone_name="Square", + usage="square", + ) + + @transaction.atomic def import_watabou_village(data: dict, *, name: str, origin: Point) -> PopulationCentre: """ @@ -258,6 +303,7 @@ def import_watabou_village(data: dict, *, name: str, origin: Point) -> Populatio districts_feature = _feature_by_id(data, "districts") earth_feature = _feature_by_id(data, "earth") fields_feature = _feature_by_id(data, "fields") + squares_feature = _feature_by_id(data, "squares") # Districts are the actual town/village extent, each a named ward - # prefer their union over the "earth" feature, which is just the @@ -360,6 +406,9 @@ def import_watabou_village(data: dict, *, name: str, origin: Point) -> Populatio if fields_feature and fields_feature.get("type") == "MultiPolygon": _import_fields(fields_feature, population_centre, offset) + if squares_feature and squares_feature.get("type") == "MultiPolygon": + _import_squares(squares_feature, population_centre, offset) + # Compute-and-log only for now (see population_estimation's module # docstring and .claude/plans/village-capacity-sizing-plan.md step 3) - # this doesn't yet change which buildings get created. It's here to diff --git a/locations/tests/test_map_serializers.py b/locations/tests/test_map_serializers.py index 824d87ab..adcdae09 100644 --- a/locations/tests/test_map_serializers.py +++ b/locations/tests/test_map_serializers.py @@ -347,6 +347,21 @@ def test_ready_stage(self): self.assertEqual(props["crop_stage"], "ready") self.assertIsNone(props["crop_progress"]) + def test_square_usage_has_no_crop_properties(self): + square = Subzone.objects.create( + name="Testville - Square", + land_area=self.subzone.land_area, + usage="square", + size=0.2, + boundary=SQUARE, + ) + square = Subzone.objects.select_related("field_crop").get(pk=square.pk) + props = SubzoneFeatureSerializer(square).data["properties"] + self.assertEqual(props["usage"], "square") + self.assertIsNone(props["crop_stage"]) + self.assertIsNone(props["crop_progress"]) + self.assertIsNone(props["shelter_building_id"]) + class PopulationCentreLabelFeatureSerializerTest(TestCase): """ diff --git a/locations/tests/test_watabou_import.py b/locations/tests/test_watabou_import.py index ea8dfe25..495a9a7a 100644 --- a/locations/tests/test_watabou_import.py +++ b/locations/tests/test_watabou_import.py @@ -30,7 +30,13 @@ def _make_export( - *, districts=None, buildings=None, roads=None, road_width=None, fields=None + *, + districts=None, + buildings=None, + roads=None, + road_width=None, + fields=None, + squares=None, ): features = [ {"type": "Feature", "id": "earth", **EARTH}, @@ -51,6 +57,10 @@ def _make_export( features.append({"type": "Feature", "id": "districts", "geometries": districts}) if fields is not None: features.append({"type": "MultiPolygon", "id": "fields", "coordinates": fields}) + if squares is not None: + features.append( + {"type": "MultiPolygon", "id": "squares", "coordinates": squares} + ) return {"features": features} @@ -348,3 +358,54 @@ def test_empty_fields_coordinates_creates_no_land_area(self): centre = import_watabou_village(data, name="Empty Fields", origin=origin) self.assertFalse(LandArea.objects.filter(population_centre=centre).exists()) + + +# Reuses FIELD_ONE/FIELD_TWO's shapes for the "squares" MultiPolygon too - +# same convention, different feature id/usage. +SQUARE_ONE = FIELD_ONE +SQUARE_TWO = FIELD_TWO + + +class WatabouImportSquaresTest(TestCase): + def test_creates_one_square_subzone_per_square_polygon(self): + data = _make_export( + districts=[TRADE_DISTRICT, MILL_WARD], squares=[SQUARE_ONE, SQUARE_TWO] + ) + origin = Point(0, 0, srid=3857) + + centre = import_watabou_village(data, name="Plaza Wards", origin=origin) + + land_area = LandArea.objects.get( + population_centre=centre, name__startswith="Squares" + ) + subzones = list(land_area.subzones.all()) + self.assertEqual(len(subzones), 2) + self.assertTrue(all(s.usage == "square" for s in subzones)) + + def test_squares_and_fields_create_separate_land_areas(self): + data = _make_export( + districts=[TRADE_DISTRICT, MILL_WARD], + fields=[FIELD_ONE], + squares=[SQUARE_TWO], + ) + origin = Point(0, 0, srid=3857) + + centre = import_watabou_village(data, name="Mixed Wards", origin=origin) + + self.assertEqual(LandArea.objects.filter(population_centre=centre).count(), 2) + crop_subzone = Subzone.objects.get(usage="crops") + square_subzone = Subzone.objects.get(usage="square") + self.assertNotEqual(crop_subzone.land_area_id, square_subzone.land_area_id) + + def test_no_squares_feature_creates_no_square_subzone(self): + data = _make_export(districts=[TRADE_DISTRICT, MILL_WARD]) + origin = Point(0, 0, srid=3857) + + centre = import_watabou_village(data, name="No Squares", origin=origin) + + self.assertFalse( + LandArea.objects.filter( + population_centre=centre, name__startswith="Squares" + ).exists() + ) + self.assertFalse(Subzone.objects.filter(usage="square").exists()) diff --git a/locations/views.py b/locations/views.py index 943196b7..2828b4b4 100644 --- a/locations/views.py +++ b/locations/views.py @@ -73,9 +73,15 @@ def get(self, request, pk): "character_locations", "goods_stocks" ) ) - crop_subzones = list( + # "crops" drives FieldCrop growth-cycle rendering (see + # SubzoneFeatureSerializer); "square" is purely a communal-space map + # feature with no economy behaviour attached (see + # watabou_import._import_squares) - both are just polygons on the + # map, so they're queried and serialized together. + visible_subzones = list( Subzone.objects.filter( - land_area__population_centre=population_centre, usage="crops" + land_area__population_centre=population_centre, + usage__in=["crops", "square"], ).select_related("field_crop") ) @@ -109,7 +115,7 @@ def get(self, request, pk): features.append(BoundaryFeatureSerializer(population_centre).data) features.extend(CharacterPointFeatureSerializer(characters, many=True).data) features.extend(BuildingFeatureSerializer(buildings, many=True).data) - features.extend(SubzoneFeatureSerializer(crop_subzones, many=True).data) + features.extend(SubzoneFeatureSerializer(visible_subzones, many=True).data) features.extend(PathFeatureSerializer(paths, many=True).data) features.extend(RoadFeatureSerializer(roads, many=True).data) @@ -120,7 +126,7 @@ def get(self, request, pk): ) for polygon_obj, polygon_attr in [ *((b, "footprint") for b in buildings), - *((s, "boundary") for s in crop_subzones), + *((s, "boundary") for s in visible_subzones), *((r, "geom") for r in roads), ]: geom = getattr(polygon_obj, polygon_attr) @@ -174,9 +180,13 @@ def get(self, request): footprint__isnull=False, footprint__bboverlaps=bbox ).prefetch_related("character_locations", "goods_stocks") ) - crop_subzones = list( + # See PopulationCentreMapView's matching comment - "crops" and + # "square" are both just polygon map features, queried together. + visible_subzones = list( Subzone.objects.filter( - usage="crops", boundary__isnull=False, boundary__bboverlaps=bbox + usage__in=["crops", "square"], + boundary__isnull=False, + boundary__bboverlaps=bbox, ).select_related("field_crop") ) paths = ( @@ -214,7 +224,7 @@ def get(self, request): features.extend(CharacterPointFeatureSerializer(characters, many=True).data) features.extend(BuildingFeatureSerializer(buildings, many=True).data) - features.extend(SubzoneFeatureSerializer(crop_subzones, many=True).data) + features.extend(SubzoneFeatureSerializer(visible_subzones, many=True).data) features.extend(PathFeatureSerializer(paths, many=True).data) features.extend(RoadFeatureSerializer(roads, many=True).data) From 41516392836354ecc3ff380086db0e5d104ca019 Mon Sep 17 00:00:00 2001 From: Duncan Appleby Date: Mon, 10 Aug 2026 01:19:02 +0100 Subject: [PATCH 03/28] feat: auto-pick an unused village_layout slot for import_village When --x/--y are omitted, import_village now picks the first VILLAGE_LAYOUT slot with no existing PopulationCentre on it, instead of requiring coordinates to be hand-picked. Passing only one of --x/--y now raises a clear CommandError instead of silently doing something unintended. This lets ad-hoc imports outside the setup_world/import_villages pipeline (e.g. trying out a village file not in locations/data/) claim spare grid space without colliding with the pipeline's own slots - though it's not persistent across a setup_world rerun, which wipes and reimports only locations/data/'s contents. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01C6BX7dFJWn2xMYn9qggdos --- .../management/commands/import_village.py | 43 ++++++++++- .../tests/test_import_village_command.py | 73 +++++++++++++++++++ 2 files changed, 112 insertions(+), 4 deletions(-) create mode 100644 locations/tests/test_import_village_command.py diff --git a/locations/management/commands/import_village.py b/locations/management/commands/import_village.py index 8707e3f9..c3ef1132 100644 --- a/locations/management/commands/import_village.py +++ b/locations/management/commands/import_village.py @@ -9,6 +9,7 @@ from locations.services.population_centre_admin import delete_population_centre from locations.services.road_connections import connect_nearest_village_roads from locations.services.watabou_import import import_watabou_village +from locations.village_layout import VILLAGE_LAYOUT from locations.village_names import VILLAGE_NAMES @@ -27,13 +28,17 @@ def add_arguments(self, parser): "--x", type=int, help="Origin X coordinate (metres, SRID 3857) to centre the village on. " - "Required unless --overwrite is reusing an existing centre's location.", + "Pass both --x and --y together, or omit both to auto-pick the " + "first unoccupied village_layout.VILLAGE_LAYOUT slot (ignored if " + "--overwrite ends up reusing an existing centre's location).", ) parser.add_argument( "--y", type=int, help="Origin Y coordinate (metres, SRID 3857) to centre the village on. " - "Required unless --overwrite is reusing an existing centre's location.", + "Pass both --x and --y together, or omit both to auto-pick the " + "first unoccupied village_layout.VILLAGE_LAYOUT slot (ignored if " + "--overwrite ends up reusing an existing centre's location).", ) parser.add_argument( "--overwrite", @@ -154,9 +159,39 @@ def _pick_village_name(self) -> str: ) def _resolve_origin(self, x: int | None, y: int | None) -> Point: + if x is None and y is None: + return self._pick_unused_layout_slot() if x is None or y is None: raise CommandError( - "Pass both --x and --y for the village's origin (or --overwrite " - "an existing centre to reuse its location)." + "Pass both --x and --y together for the village's origin, or " + "neither to auto-pick an unoccupied village_layout.VILLAGE_LAYOUT " + "slot." ) return Point(x, y, srid=3857) + + def _pick_unused_layout_slot(self) -> Point: + """ + First VILLAGE_LAYOUT slot with no existing PopulationCentre already + sitting on it - lets an ad-hoc import (e.g. trying out a village file + outside locations/data/, so outside the setup_world/import_villages + pipeline) claim spare grid space without hand-picking coordinates. + + Not persistent across a setup_world rerun: that command deletes every + existing PopulationCentre before reimporting only locations/data/'s + files (see setup_world.py), so an ad-hoc import placed here will need + to be redone afterwards - and may land on a different free slot next + time, since which slots are "unoccupied" depends on whatever other + centres exist at that moment. + """ + occupied = { + (round(centre.location.x), round(centre.location.y)) + for centre in PopulationCentre.objects.only("location") + } + for x, y in VILLAGE_LAYOUT: + if (x, y) not in occupied: + return Point(x, y, srid=3857) + raise CommandError( + f"Every village_layout.VILLAGE_LAYOUT slot ({len(VILLAGE_LAYOUT)}) is " + "already occupied by a PopulationCentre - pass --x/--y explicitly, " + "or add more slots (GRID_COLUMNS/GRID_ROWS)." + ) diff --git a/locations/tests/test_import_village_command.py b/locations/tests/test_import_village_command.py new file mode 100644 index 00000000..707d273e --- /dev/null +++ b/locations/tests/test_import_village_command.py @@ -0,0 +1,73 @@ +from django.contrib.gis.geos import Point +from django.core.management.base import CommandError +from django.test import TestCase + +from locations.management.commands.import_village import Command +from locations.models import PopulationCentre +from locations.village_layout import VILLAGE_LAYOUT + + +class ResolveOriginTest(TestCase): + def setUp(self): + self.command = Command() + + def test_both_x_and_y_given_uses_them_directly(self): + origin = self.command._resolve_origin(123, 456) + self.assertEqual(origin, Point(123, 456, srid=3857)) + + def test_only_x_given_raises(self): + with self.assertRaises(CommandError): + self.command._resolve_origin(123, None) + + def test_only_y_given_raises(self): + with self.assertRaises(CommandError): + self.command._resolve_origin(None, 456) + + def test_neither_given_auto_picks_a_slot(self): + origin = self.command._resolve_origin(None, None) + x, y = VILLAGE_LAYOUT[0] + self.assertEqual(origin, Point(x, y, srid=3857)) + + +class PickUnusedLayoutSlotTest(TestCase): + def setUp(self): + self.command = Command() + + def test_picks_first_slot_when_none_occupied(self): + origin = self.command._pick_unused_layout_slot() + x, y = VILLAGE_LAYOUT[0] + self.assertEqual(origin, Point(x, y, srid=3857)) + + def test_skips_occupied_slots(self): + first_x, first_y = VILLAGE_LAYOUT[0] + second_x, second_y = VILLAGE_LAYOUT[1] + PopulationCentre.objects.create( + name="Occupied village", + location=Point(first_x, first_y, srid=3857), + ) + + origin = self.command._pick_unused_layout_slot() + + self.assertEqual(origin, Point(second_x, second_y, srid=3857)) + + def test_raises_when_every_slot_is_occupied(self): + for i, (x, y) in enumerate(VILLAGE_LAYOUT): + PopulationCentre.objects.create( + name=f"Village {i}", + location=Point(x, y, srid=3857), + ) + + with self.assertRaises(CommandError): + self.command._pick_unused_layout_slot() + + def test_ignores_centres_not_on_a_layout_slot(self): + # A centre placed off-grid (e.g. hand-picked --x/--y) shouldn't + # affect which layout slots count as unoccupied. + PopulationCentre.objects.create( + name="Off-grid village", + location=Point(999_999, 999_999, srid=3857), + ) + + origin = self.command._pick_unused_layout_slot() + x, y = VILLAGE_LAYOUT[0] + self.assertEqual(origin, Point(x, y, srid=3857)) From ddde5c06f7fc13ecfb4296df0109a699769eb24e Mon Sep 17 00:00:00 2001 From: Duncan Appleby Date: Mon, 10 Aug 2026 01:25:46 +0100 Subject: [PATCH 04/28] perf: prefetch initial-centre/world-bounds map data at login GameContext now primes useInitialMapCentre's and useMapWorldBounds's query cache entries as soon as fetch_info resolves, instead of only firing them once the player navigates to the map page. Both are cheap, one-shot fetches, so warming them at login lets MapPage skip a network round-trip on mount for players who go on to open it. Extracted the {queryKey, queryFn, staleTime, gcTime} for each into exported query-option objects in useMap.ts, reused by both the hooks and GameContext's prefetch, so the two can't drift onto different cache keys. Deliberately doesn't prefetch /map/viewport/: it needs a bbox only the mounted map component can produce, and starts a 2s poll once enabled - prefetching it at login would poll map data in the background for every session regardless of whether the player ever opens the map. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01C6BX7dFJWn2xMYn9qggdos --- frontend/src/context/GameContext.tsx | 21 +++++++++++++++++ frontend/src/hooks/useMap.ts | 35 ++++++++++++++++++---------- 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/frontend/src/context/GameContext.tsx b/frontend/src/context/GameContext.tsx index 3560e6b4..76b14bf7 100644 --- a/frontend/src/context/GameContext.tsx +++ b/frontend/src/context/GameContext.tsx @@ -1,12 +1,14 @@ // GameContext.tsx import { useState, useEffect, useCallback, useMemo } from 'react'; import type { ReactElement, ReactNode } from 'react'; +import { useQueryClient } from '@tanstack/react-query'; import { useBootstrapGameData } from '../hooks/useBootstrapGameData'; import { useEventCallback } from '../hooks/useEventCallback'; import { apiFetch } from "../utils/api"; import useActivityTimer from '../hooks/useActivityTimer'; import useUnloadWarning from '../hooks/useUnloadWarning'; +import { initialMapCentreQueryOptions, mapWorldBoundsQueryOptions } from '../hooks/useMap'; import { useAuth } from './AuthContext'; import { GameContext, type GameContextValue } from './gameContext'; import type { @@ -85,6 +87,8 @@ export const GameProvider = ({ children }: ProviderProps): ReactElement => { useUnloadWarning(activityTimer.status === 'active'); + const queryClient = useQueryClient(); + // ---------------------------------------- // STABLE CALLBACKS @@ -150,6 +154,23 @@ export const GameProvider = ({ children }: ProviderProps): ReactElement => { player?.is_premium, ]); + // Primes the map's two cheap, one-shot "where/how big is the world" + // queries as soon as fetch_info has resolved, rather than waiting for the + // player to navigate to the map page and pay for them there. Both use the + // same {queryKey, queryFn, staleTime} as useInitialMapCentre/ + // useMapWorldBounds (see useMap.ts) so this primes exactly the cache entry + // those hooks read - if the player never opens the map, the prefetched + // data just sits unused until its gcTime expires. Deliberately doesn't + // prefetch /map/viewport/: that needs a bbox only the mounted map + // component can produce, and (unlike these two) starts a 2s poll once + // enabled, which would run in the background for every session whether or + // not the player ever opens the map. + useEffect(() => { + if (loading || !isAuthenticated) return; + void queryClient.prefetchQuery(initialMapCentreQueryOptions); + void queryClient.prefetchQuery(mapWorldBoundsQueryOptions); + }, [loading, isAuthenticated, queryClient]); + const onAuthReadyFetchActivities = useEventCallback(fetchActivities); useEffect(() => { if (!authLoading && isAuthenticated) { diff --git a/frontend/src/hooks/useMap.ts b/frontend/src/hooks/useMap.ts index 24657f98..deb73979 100644 --- a/frontend/src/hooks/useMap.ts +++ b/frontend/src/hooks/useMap.ts @@ -13,6 +13,13 @@ import { // tracks actual journeys closely, without polling every single tick. export const MAP_POLL_INTERVAL_MS = 2000; +// Shared {queryKey, queryFn, staleTime, gcTime} for the map's two cheap, +// one-shot "where/how big is the world" queries - exported (rather than +// inlined in useInitialMapCentre/useMapWorldBounds below) so GameContext's +// login-time prefetch (see useBootstrapGameData) primes the exact same cache +// entries these hooks read, instead of duplicating the queryKey literals and +// risking the two drifting apart. +// // One-shot (not polled) fetch of just enough (id/name/bbox) to know where // the camera should start - the requesting player's linked character's // village if they have one, otherwise an arbitrary but deterministic @@ -24,13 +31,15 @@ export const MAP_POLL_INTERVAL_MS = 2000; // effectively-forever) means a player who links to a different character // mid-session and revisits the map later still gets pointed at the right // village instead of a stale cached one. +export const initialMapCentreQueryOptions = { + queryKey: ["map", "initial-centre"] as const, + queryFn: fetchInitialMapCentre, + staleTime: 5 * 60 * 1000, + gcTime: 15 * 60 * 1000, +}; + export function useInitialMapCentre() { - return useQuery({ - queryKey: ["map", "initial-centre"], - queryFn: fetchInitialMapCentre, - staleTime: 5 * 60 * 1000, - gcTime: 15 * 60 * 1000, - }); + return useQuery(initialMapCentreQueryOptions); } // Reads the same cache entry MapPage's prefetch effect primes for the @@ -71,13 +80,15 @@ export function useMapViewport(bbox: string | null) { // MapLibre's maxBounds (see design decision #6 in the map-viewport plan). // One-shot per session rather than polled - the world's overall extent // changes far more slowly than any individual viewport's contents. +export const mapWorldBoundsQueryOptions = { + queryKey: ["map", "world-bounds"] as const, + queryFn: fetchMapWorldBounds, + staleTime: 30 * 60 * 1000, + gcTime: 60 * 60 * 1000, +}; + export function useMapWorldBounds() { - return useQuery({ - queryKey: ["map", "world-bounds"], - queryFn: fetchMapWorldBounds, - staleTime: 30 * 60 * 1000, - gcTime: 60 * 60 * 1000, - }); + return useQuery(mapWorldBoundsQueryOptions); } // On-demand fetch for one character's map detail card (see DetailCard/ From ee3e482c9b4b2f3d739179a59d9ff1eb8706c5be Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 18:21:10 +0000 Subject: [PATCH 05/28] Replace map-view badge with daily goals + completion bonus (#751) Replaces the truncation-prone `points_today` badge with three easy daily goals (logged in, completed an activity, 3+ minutes recorded) plus a one-off AP bonus for clearing all three in a day. - progression.daily_goals: live goal-state computation and idempotent bonus award, gated by a new DailyGoalAward(player, date) row - Wired into both activity-completion paths (ActivityTimer.complete and offline logging), not just the timer flow - GameSettings.daily_goals_completion_bonus_ap controls the bonus amount - MeViewSet.daily_goals replaces today_points with the full goal state - Removed the now-dead PlayerCharacterLink.player_time_today/points_today - Frontend: DailyGoalsBadge replaces TodayPointsBadge Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_014hFSsXRtBgoJX7HTqdmTEb --- api/tests.py | 60 +++++- api/views.py | 51 ++++- character/models/character.py | 28 --- character/tests/test_models.py | 59 ------ ...ettings_daily_goals_completion_bonus_ap.py | 21 ++ core/models.py | 9 + frontend/src/api/player.ts | 26 ++- .../ActivityInput/useActivityInput.test.ts | 4 +- .../ActivityInput/useActivityInput.ts | 4 +- .../DailyGoalsBadge.module.scss | 60 ++++++ .../DailyGoalsBadge/DailyGoalsBadge.test.tsx | 96 +++++++++ .../DailyGoalsBadge/DailyGoalsBadge.tsx | 65 ++++++ .../TodayPointsBadge.module.scss | 12 -- .../TodayPointsBadge.test.tsx | 46 ----- .../TodayPointsBadge/TodayPointsBadge.tsx | 23 --- frontend/src/featureFlags.ts | 2 +- frontend/src/hooks/usePlayer.ts | 20 +- frontend/src/pages/MapPage/MapPage.tsx | 6 +- frontend/src/types/enums.ts | 2 +- gameplay/models.py | 7 + gameplay/tests/test_activity_timer_premium.py | 1 + gameplay/tests/test_models.py | 38 ++++ progression/admin.py | 18 ++ progression/daily_goals.py | 123 +++++++++++ progression/migrations/0023_dailygoalaward.py | 48 +++++ progression/models.py | 32 +++ progression/services.py | 5 + progression/tests/test_daily_goals.py | 192 ++++++++++++++++++ .../tests/test_offline_activity_logging.py | 26 +++ 29 files changed, 876 insertions(+), 208 deletions(-) create mode 100644 core/migrations/0016_gamesettings_daily_goals_completion_bonus_ap.py create mode 100644 frontend/src/components/DailyGoalsBadge/DailyGoalsBadge.module.scss create mode 100644 frontend/src/components/DailyGoalsBadge/DailyGoalsBadge.test.tsx create mode 100644 frontend/src/components/DailyGoalsBadge/DailyGoalsBadge.tsx delete mode 100644 frontend/src/components/TodayPointsBadge/TodayPointsBadge.module.scss delete mode 100644 frontend/src/components/TodayPointsBadge/TodayPointsBadge.test.tsx delete mode 100644 frontend/src/components/TodayPointsBadge/TodayPointsBadge.tsx create mode 100644 progression/daily_goals.py create mode 100644 progression/migrations/0023_dailygoalaward.py create mode 100644 progression/tests/test_daily_goals.py diff --git a/api/tests.py b/api/tests.py index 6a4d5fe8..2e40ffba 100644 --- a/api/tests.py +++ b/api/tests.py @@ -194,32 +194,78 @@ def test_complete_onboarding_sets_flag(self): self.assertTrue(player_for(self.user).onboarding_completed) self.assertEqual(res.data, {"onboarding_completed": True}) - def test_today_points_null_when_no_active_link(self): + def test_daily_goals_null_when_no_active_link(self): self.authenticate() player = player_for(self.user) self.assertIsNone(player.active_link) - res = self.client.get(reverse("me-today-points")) + res = self.client.get(reverse("me-daily-goals")) self.assertEqual(res.status_code, status.HTTP_200_OK) - self.assertIsNone(res.data["points_today"]) + self.assertIsNone(res.data["goals"]) - def test_today_points_reflects_todays_completed_activities(self): + def test_daily_goals_all_false_when_linked_but_no_activity_or_login(self): self.authenticate() player = player_for(self.user) PlayerCharacterLink.objects.create(player=player, character=self.character) + res = self.client.get(reverse("me-daily-goals")) + + self.assertEqual(res.status_code, status.HTTP_200_OK) + goals = res.data["goals"] + self.assertFalse(goals["logged_in_today"]) + self.assertFalse(goals["completed_activity_today"]) + self.assertEqual(goals["activity_minutes_today"], 0) + self.assertFalse(goals["minutes_goal_met"]) + self.assertFalse(goals["all_goals_met"]) + self.assertFalse(goals["bonus_awarded_today"]) + self.assertEqual(goals["bonus_ap"], 0) + + def test_daily_goals_reflects_todays_completed_activities(self): + self.authenticate() + player = player_for(self.user) + PlayerCharacterLink.objects.create(player=player, character=self.character) + + PlayerActivity.objects.create( + player=player, + is_complete=True, + duration=1200, # 20 minutes + completed_at=datetime.now(timezone.utc), + ) + + res = self.client.get(reverse("me-daily-goals")) + + self.assertEqual(res.status_code, status.HTTP_200_OK) + goals = res.data["goals"] + self.assertTrue(goals["completed_activity_today"]) + self.assertEqual(goals["activity_minutes_today"], 20) + self.assertTrue(goals["minutes_goal_met"]) + + def test_daily_goals_all_met_awards_bonus_once(self): + from users.models import UserLogin + + self.authenticate() + player = player_for(self.user) + PlayerCharacterLink.objects.create(player=player, character=self.character) + UserLogin.objects.create(user=self.user) + PlayerActivity.objects.create( player=player, is_complete=True, - duration=1200, # 20 minutes -> 2 points + duration=1200, completed_at=datetime.now(timezone.utc), ) - res = self.client.get(reverse("me-today-points")) + res = self.client.get(reverse("me-daily-goals")) self.assertEqual(res.status_code, status.HTTP_200_OK) - self.assertEqual(res.data["points_today"], 2) + goals = res.data["goals"] + self.assertTrue(goals["all_goals_met"]) + # Reads never award the bonus themselves - only activity completion + # does (via check_and_award_daily_goals), so a plain GET here + # shouldn't have paid it out. + self.assertFalse(goals["bonus_awarded_today"]) + self.assertEqual(goals["bonus_ap"], 0) class CustomTokenObtainPairViewTests(APITestCase): diff --git a/api/views.py b/api/views.py index 65505a45..4d67fa11 100644 --- a/api/views.py +++ b/api/views.py @@ -363,24 +363,61 @@ def character(self, request): @extend_schema( responses=inline_serializer( - name="TodayPointsResponse", + name="DailyGoalsResponse", fields={ - "points_today": drf_serializers.IntegerField(allow_null=True), + "goals": inline_serializer( + name="DailyGoalsStateResponse", + fields={ + "logged_in_today": drf_serializers.BooleanField(), + "completed_activity_today": drf_serializers.BooleanField(), + "activity_minutes_today": drf_serializers.IntegerField(), + "minutes_goal_threshold": drf_serializers.IntegerField(), + "minutes_goal_met": drf_serializers.BooleanField(), + "all_goals_met": drf_serializers.BooleanField(), + "bonus_awarded_today": drf_serializers.BooleanField(), + "bonus_ap": drf_serializers.IntegerField(), + }, + allow_null=True, + ), }, ) ) @action(detail=False, methods=["get"]) - def today_points(self, request): + def daily_goals(self, request): """ - Personal "points earned today" for the map view's badge (issue #673). - `points_today` is null - not zero - when the player has no active + Daily goals for the map view's badge (issue #751, replacing the old + `today_points`/`points_today` mechanic from issue #673). `goals` is + null - not a set of all-false goals - when the player has no active PlayerCharacterLink, so the frontend can tell "no link" apart from - "linked but nothing earned yet today" and hide the badge entirely. + "linked but no goals cleared yet today" and hide the badge entirely. """ + from progression.daily_goals import ( + MINUTES_GOAL_THRESHOLD, + get_daily_goals_state, + ) + player = request.user.player link = player.active_link - return Response({"points_today": link.points_today if link else None}) + if not link: + return Response({"goals": None}) + + state = get_daily_goals_state(player) + + return Response( + { + "goals": { + "logged_in_today": state.logged_in_today, + "completed_activity_today": state.completed_activity_today, + "activity_minutes_today": state.activity_minutes_today, + "minutes_goal_threshold": MINUTES_GOAL_THRESHOLD, + "minutes_goal_met": state.minutes_goal_met, + "all_goals_met": state.all_goals_met, + "bonus_awarded_today": state.bonus_awarded_today, + "bonus_ap": state.bonus_ap, + } + } + ) @extend_schema( responses=inline_serializer( diff --git a/character/models/character.py b/character/models/character.py index fab8ffc6..74286ee5 100644 --- a/character/models/character.py +++ b/character/models/character.py @@ -520,34 +520,6 @@ def link_points(self): ) return int(base_points * multiplier) - @property - def player_time_today(self): - """ - Completed activity time for this link so far today (in minutes) - - a date-filtered variant of player_time, bounded to the current UTC - day instead of the whole link lifetime. - """ - start_of_day = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0) - start = max(start_of_day, self.linked_at) - - qs = self.player.activities.filter(is_complete=True, completed_at__gte=start) - if self.unlinked_at: - qs = qs.filter(completed_at__lte=self.unlinked_at) - - total_seconds = qs.aggregate(total=Sum("duration"))["total"] or 0 - return int(total_seconds // 60) - - @property - def points_today(self): - """ - Points earned today (issue #673's map-view "today" badge) - only the - activity-time component of link_points, date-filtered to today. - link_points' other terms (days_linked * 20, login_points) aren't - "earned today" in the same sense, so they're deliberately left out - here rather than prorated. - """ - return self.player_time_today // 10 - @classmethod def get_character(cls, player: Player) -> Character: return link_services.player_link_get_character(cls, player) diff --git a/character/tests/test_models.py b/character/tests/test_models.py index 45704cff..0f803f25 100644 --- a/character/tests/test_models.py +++ b/character/tests/test_models.py @@ -506,65 +506,6 @@ def test_has_available_all_linked(self): self.assertFalse(Character.has_available()) -class PlayerCharacterLinkPointsTodayTests(TestCase): - """Tests for PlayerCharacterLink.player_time_today/points_today (issue #673).""" - - def setUp(self): - from progression.models import PlayerActivity - - self.PlayerActivity = PlayerActivity - self.user = user_factory(with_player=True) - self.player = self.user.player - character = Character.objects.create(given_name="Hero") - self.link = PlayerCharacterLink.objects.create( - player=self.player, character=character - ) - # Backdated well before "today" so player_time_today's max(start_of_day, - # linked_at) resolves to start_of_day in these tests, rather than to - # whatever moment setUp happened to run at. - self.link.linked_at = now() - timedelta(days=30) - self.link.save(update_fields=["linked_at"]) - - def _complete_activity(self, *, duration_seconds, completed_at): - return self.PlayerActivity.objects.create( - player=self.player, - is_complete=True, - duration=duration_seconds, - completed_at=completed_at, - ) - - def test_points_today_counts_only_activities_completed_today(self): - today_start = now().replace(hour=0, minute=0, second=0, microsecond=0) - self._complete_activity( - duration_seconds=1800, completed_at=today_start + timedelta(hours=2) - ) # 30 min today - self._complete_activity( - duration_seconds=3600, completed_at=today_start - timedelta(hours=1) - ) # 60 min yesterday - excluded - - self.assertEqual(self.link.player_time_today, 30) - self.assertEqual(self.link.points_today, 3) - - def test_points_today_excludes_activity_before_link_started(self): - today_start = now().replace(hour=0, minute=0, second=0, microsecond=0) - self.link.linked_at = today_start + timedelta(hours=5) - self.link.save(update_fields=["linked_at"]) - - self._complete_activity( - duration_seconds=1800, completed_at=today_start + timedelta(hours=1) - ) # today, but before the link started - excluded - self._complete_activity( - duration_seconds=600, completed_at=today_start + timedelta(hours=6) - ) # 10 min, after linked_at - - self.assertEqual(self.link.player_time_today, 10) - self.assertEqual(self.link.points_today, 1) - - def test_points_today_zero_with_no_activities(self): - self.assertEqual(self.link.player_time_today, 0) - self.assertEqual(self.link.points_today, 0) - - class CharacterTotalLinkPointsTests(TestCase): """Tests for Character.total_link_points (the character-side counterpart to Player.total_link_points).""" diff --git a/core/migrations/0016_gamesettings_daily_goals_completion_bonus_ap.py b/core/migrations/0016_gamesettings_daily_goals_completion_bonus_ap.py new file mode 100644 index 00000000..b30f306a --- /dev/null +++ b/core/migrations/0016_gamesettings_daily_goals_completion_bonus_ap.py @@ -0,0 +1,21 @@ +# Generated by Django 5.2.16 on 2026-08-10 00:00 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("core", "0015_featureflag"), + ] + + operations = [ + migrations.AddField( + model_name="gamesettings", + name="daily_goals_completion_bonus_ap", + field=models.IntegerField( + default=50, + help_text="Lump-sum AP awarded once per day when a player clears all three daily goals (see progression.daily_goals).", + ), + ), + ] diff --git a/core/models.py b/core/models.py index 07e9204c..da57cd33 100644 --- a/core/models.py +++ b/core/models.py @@ -90,6 +90,13 @@ class GameSettings(models.Model): max_digits=5, decimal_places=2, default="1.25" ) task_completion_xp = models.IntegerField(default=100) + daily_goals_completion_bonus_ap = models.IntegerField( + default=50, + help_text=( + "Lump-sum AP awarded once per day when a player clears all " + "three daily goals (see progression.daily_goals)." + ), + ) xp_mastery_scale = models.DecimalField( max_digits=10, decimal_places=2, @@ -162,6 +169,8 @@ def clean(self): errors["task_activity_xp_multiplier"] = "Must be > 0." if self.task_completion_xp < 0: errors["task_completion_xp"] = "Must be non-negative." + if self.daily_goals_completion_bonus_ap < 0: + errors["daily_goals_completion_bonus_ap"] = "Must be non-negative." if self.xp_mastery_scale <= 0: errors["xp_mastery_scale"] = "Must be > 0." if self.xp_mastery_multiplier_cap < 1: diff --git a/frontend/src/api/player.ts b/frontend/src/api/player.ts index b4263b44..ba23d15b 100644 --- a/frontend/src/api/player.ts +++ b/frontend/src/api/player.ts @@ -41,13 +41,25 @@ export const deleteAccount = async (): Promise => { return response; }; -export interface TodayPointsResponse { - // null (not 0) when the player has no active PlayerCharacterLink - see - // MeViewSet.today_points in api/views.py. Callers use this to hide the - // map view's "today" badge entirely rather than showing a zero (issue #673). - points_today: number | null; +export interface DailyGoalsState { + logged_in_today: boolean; + completed_activity_today: boolean; + activity_minutes_today: number; + minutes_goal_threshold: number; + minutes_goal_met: boolean; + all_goals_met: boolean; + bonus_awarded_today: boolean; + bonus_ap: number; } -export const fetchTodayPoints = async (): Promise => { - return apiFetch("/me/today_points/"); +export interface DailyGoalsResponse { + // null (not a set of all-false goals) when the player has no active + // PlayerCharacterLink - see MeViewSet.daily_goals in api/views.py. + // Callers use this to hide the map view's badge entirely rather than + // showing an unearned "0 of 3" (issue #673, redesigned in #751). + goals: DailyGoalsState | null; +} + +export const fetchDailyGoals = async (): Promise => { + return apiFetch("/me/daily_goals/"); }; diff --git a/frontend/src/components/ActivityInput/useActivityInput.test.ts b/frontend/src/components/ActivityInput/useActivityInput.test.ts index cbf94b1f..137e83e1 100644 --- a/frontend/src/components/ActivityInput/useActivityInput.test.ts +++ b/frontend/src/components/ActivityInput/useActivityInput.test.ts @@ -344,7 +344,7 @@ describe('useActivityInput unified handlers', () => { }); }); - it('invalidates the today-points query after completing an activity, so the map badge updates immediately (#673)', async () => { + it('invalidates the daily-goals query after completing an activity, so the map badge updates immediately (#673, #751)', async () => { mockGame({ status: 'active', currentActivity: { name: 'Deep work' } }); stop.mockResolvedValue({ xp_gained: 10 }); @@ -355,7 +355,7 @@ describe('useActivityInput unified handlers', () => { }); expect(invalidateQueries).toHaveBeenCalledWith({ - queryKey: ['me', 'today-points'], + queryKey: ['me', 'daily-goals'], }); }); }); diff --git a/frontend/src/components/ActivityInput/useActivityInput.ts b/frontend/src/components/ActivityInput/useActivityInput.ts index c130ef24..5e8c3739 100644 --- a/frontend/src/components/ActivityInput/useActivityInput.ts +++ b/frontend/src/components/ActivityInput/useActivityInput.ts @@ -5,7 +5,7 @@ import { useGame } from "../../hooks/useGame"; import { useEntitySearchCache } from "../../hooks/useEntitySearchCache"; import { useSupportFlow } from "../../hooks/useSupportFlow"; import { useFeatureFlag } from "../../hooks/useFeatureFlag"; -import { TODAY_POINTS_QUERY_KEY } from "../../hooks/usePlayer"; +import { DAILY_GOALS_QUERY_KEY } from "../../hooks/usePlayer"; import type { PlayerActivity } from "../../types"; import { playLimitReachedSound, primeAudio } from "../../utils/sounds"; @@ -295,7 +295,7 @@ export function useActivityInput() { fetchPlayerAndCharacter(), fetchCharacterCurrent(), fetchActivities(), - queryClient.invalidateQueries({ queryKey: TODAY_POINTS_QUERY_KEY }), + queryClient.invalidateQueries({ queryKey: DAILY_GOALS_QUERY_KEY }), completedTaskId ? queryClient.invalidateQueries({ queryKey: ["tasks"] }) : Promise.resolve(), ]); } catch (err) { diff --git a/frontend/src/components/DailyGoalsBadge/DailyGoalsBadge.module.scss b/frontend/src/components/DailyGoalsBadge/DailyGoalsBadge.module.scss new file mode 100644 index 00000000..3bdf9ece --- /dev/null +++ b/frontend/src/components/DailyGoalsBadge/DailyGoalsBadge.module.scss @@ -0,0 +1,60 @@ +@use '../../styles/semantic/colors' as c; +@use '../../styles/semantic/typography' as t; +@use '../../styles/semantic/spacing' as sp; + +.badge { + background: c.$color-bg; + border: 1px solid c.$color-border-primary; + border-radius: sp.$border-radius; + padding: sp.$spacing-xs sp.$spacing-sm; + min-width: 11rem; + @include t.apply-text-style(t.$text-body); +} + +.title { + margin: 0 0 sp.$spacing-xs; + @include t.apply-text-style(t.$text-caption); + color: c.$color-text-muted; +} + +.goalList { + list-style: none; + margin: 0; + padding: 0; + display: flex; + flex-direction: column; + gap: 2px; +} + +.goal, +.goalMet { + display: flex; + align-items: center; + gap: sp.$spacing-xs; + @include t.apply-text-style(t.$text-list); +} + +.goal { + color: c.$color-text-muted; +} + +.goalMet { + color: c.$color-text-body; +} + +.goalMark { + color: c.$color-status-success; + width: 1em; + text-align: center; +} + +.goal .goalMark { + color: c.$color-text-disabled; +} + +.bonus { + margin: sp.$spacing-xs 0 0; + color: c.$color-status-success; + font-weight: bold; + @include t.apply-text-style(t.$text-list); +} diff --git a/frontend/src/components/DailyGoalsBadge/DailyGoalsBadge.test.tsx b/frontend/src/components/DailyGoalsBadge/DailyGoalsBadge.test.tsx new file mode 100644 index 00000000..3313ca75 --- /dev/null +++ b/frontend/src/components/DailyGoalsBadge/DailyGoalsBadge.test.tsx @@ -0,0 +1,96 @@ +import { render, screen } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; + +import DailyGoalsBadge from './DailyGoalsBadge'; +import type { DailyGoalsResponse } from '../../api/player'; + +const mockUseDailyGoals = vi.fn<() => { data: DailyGoalsResponse | undefined }>( + () => ({ data: { goals: null } }) +); + +vi.mock('../../hooks/usePlayer', () => ({ + useDailyGoals: () => mockUseDailyGoals(), +})); + +const baseGoals = { + logged_in_today: false, + completed_activity_today: false, + activity_minutes_today: 0, + minutes_goal_threshold: 3, + minutes_goal_met: false, + all_goals_met: false, + bonus_awarded_today: false, + bonus_ap: 0, +}; + +describe('DailyGoalsBadge', () => { + it('renders nothing when the player has no active character link', () => { + mockUseDailyGoals.mockReturnValue({ data: { goals: null } }); + + const { container } = render(); + + expect(container).toBeEmptyDOMElement(); + }); + + it('renders nothing while the query has no data yet', () => { + mockUseDailyGoals.mockReturnValue({ data: undefined }); + + const { container } = render(); + + expect(container).toBeEmptyDOMElement(); + }); + + it('shows all three goals as unmet', () => { + mockUseDailyGoals.mockReturnValue({ data: { goals: baseGoals } }); + + render(); + + expect(screen.getByText('Logged in today')).toBeInTheDocument(); + expect(screen.getByText('Completed an activity')).toBeInTheDocument(); + expect(screen.getByText('3+ minutes recorded')).toBeInTheDocument(); + expect(screen.queryByText(/AP bonus earned/)).not.toBeInTheDocument(); + expect(screen.queryByText('All goals cleared!')).not.toBeInTheDocument(); + }); + + it('shows the awarded bonus once all goals are cleared and the bonus has paid out', () => { + mockUseDailyGoals.mockReturnValue({ + data: { + goals: { + ...baseGoals, + logged_in_today: true, + completed_activity_today: true, + activity_minutes_today: 5, + minutes_goal_met: true, + all_goals_met: true, + bonus_awarded_today: true, + bonus_ap: 50, + }, + }, + }); + + render(); + + expect(screen.getByText('+50 AP bonus earned!')).toBeInTheDocument(); + }); + + it('shows a generic "cleared" message if goals are met but the bonus has not landed yet', () => { + mockUseDailyGoals.mockReturnValue({ + data: { + goals: { + ...baseGoals, + logged_in_today: true, + completed_activity_today: true, + activity_minutes_today: 5, + minutes_goal_met: true, + all_goals_met: true, + bonus_awarded_today: false, + bonus_ap: 0, + }, + }, + }); + + render(); + + expect(screen.getByText('All goals cleared!')).toBeInTheDocument(); + }); +}); diff --git a/frontend/src/components/DailyGoalsBadge/DailyGoalsBadge.tsx b/frontend/src/components/DailyGoalsBadge/DailyGoalsBadge.tsx new file mode 100644 index 00000000..ed841a59 --- /dev/null +++ b/frontend/src/components/DailyGoalsBadge/DailyGoalsBadge.tsx @@ -0,0 +1,65 @@ +import React from "react"; +import { useDailyGoals } from "../../hooks/usePlayer"; +import styles from "./DailyGoalsBadge.module.scss"; + +interface GoalItem { + key: string; + label: string; + met: boolean; +} + +// Personal "daily goals" badge for the map view (issue #751, replacing the +// old single-number "today" points badge from issue #673) - deliberately +// not attached to any village marker, and deliberately silent about the +// player/character link: framed as the player's own goals rather than +// naming a character, since players don't yet know about the link. +// Renders nothing (not an empty/all-unmet state) when the player has no +// active PlayerCharacterLink - see MeViewSet.daily_goals in api/views.py. +export default function DailyGoalsBadge(): React.ReactElement | null { + const { data } = useDailyGoals(); + const goals = data?.goals; + + if (goals == null) { + return null; + } + + const items: GoalItem[] = [ + { key: "login", label: "Logged in today", met: goals.logged_in_today }, + { + key: "activity", + label: "Completed an activity", + met: goals.completed_activity_today, + }, + { + key: "minutes", + label: `${goals.minutes_goal_threshold}+ minutes recorded`, + met: goals.minutes_goal_met, + }, + ]; + + return ( +
+

Today's goals

+
    + {items.map((item) => ( +
  • + + {item.label} +
    • + ))} +
+ {goals.all_goals_met && ( +

+ {goals.bonus_awarded_today + ? `+${goals.bonus_ap} AP bonus earned!` + : "All goals cleared!"} +

+ )} +
+ ); +} diff --git a/frontend/src/components/TodayPointsBadge/TodayPointsBadge.module.scss b/frontend/src/components/TodayPointsBadge/TodayPointsBadge.module.scss deleted file mode 100644 index 905ae3fe..00000000 --- a/frontend/src/components/TodayPointsBadge/TodayPointsBadge.module.scss +++ /dev/null @@ -1,12 +0,0 @@ -@use '../../styles/semantic/colors' as c; -@use '../../styles/semantic/typography' as t; -@use '../../styles/semantic/spacing' as sp; - -.badge { - background: c.$color-bg; - border: 1px solid c.$color-border-primary; - border-radius: sp.$border-radius; - padding: 0.25rem 0.6rem; - white-space: nowrap; - @include t.apply-text-style(t.$text-body); -} diff --git a/frontend/src/components/TodayPointsBadge/TodayPointsBadge.test.tsx b/frontend/src/components/TodayPointsBadge/TodayPointsBadge.test.tsx deleted file mode 100644 index c2c50430..00000000 --- a/frontend/src/components/TodayPointsBadge/TodayPointsBadge.test.tsx +++ /dev/null @@ -1,46 +0,0 @@ -import { render, screen } from '@testing-library/react'; -import { describe, expect, it, vi } from 'vitest'; - -import TodayPointsBadge from './TodayPointsBadge'; - -const mockUseTodayPoints = vi.fn<() => { data: { points_today: number | null } | undefined }>( - () => ({ data: { points_today: 12 } }) -); - -vi.mock('../../hooks/usePlayer', () => ({ - useTodayPoints: () => mockUseTodayPoints(), -})); - -describe('TodayPointsBadge', () => { - it("shows the player's points earned today", () => { - mockUseTodayPoints.mockReturnValue({ data: { points_today: 12 } }); - - render(); - - expect(screen.getByText('You contributed 12 today')).toBeInTheDocument(); - }); - - it('renders nothing when the player has no active character link', () => { - mockUseTodayPoints.mockReturnValue({ data: { points_today: null } }); - - const { container } = render(); - - expect(container).toBeEmptyDOMElement(); - }); - - it('renders nothing (not a zero) while the query has no data yet', () => { - mockUseTodayPoints.mockReturnValue({ data: undefined }); - - const { container } = render(); - - expect(container).toBeEmptyDOMElement(); - }); - - it('shows zero points as a real value, not treating it as "no link"', () => { - mockUseTodayPoints.mockReturnValue({ data: { points_today: 0 } }); - - render(); - - expect(screen.getByText('You contributed 0 today')).toBeInTheDocument(); - }); -}); diff --git a/frontend/src/components/TodayPointsBadge/TodayPointsBadge.tsx b/frontend/src/components/TodayPointsBadge/TodayPointsBadge.tsx deleted file mode 100644 index c2cb5cc4..00000000 --- a/frontend/src/components/TodayPointsBadge/TodayPointsBadge.tsx +++ /dev/null @@ -1,23 +0,0 @@ -import React from "react"; -import { useTodayPoints } from "../../hooks/usePlayer"; -import styles from "./TodayPointsBadge.module.scss"; - -// Personal "points earned today" badge for the map view (issue #673) - -// deliberately not attached to any village marker, and deliberately silent -// about the player/character link: framed as "you contributed today" rather -// than naming a character, since players don't yet know about the link. -// Renders nothing (not a zero) when the player has no active -// PlayerCharacterLink - see MeViewSet.today_points in api/views.py. -export default function TodayPointsBadge(): React.ReactElement | null { - const { data } = useTodayPoints(); - - if (data?.points_today == null) { - return null; - } - - return ( -
- You contributed {data.points_today} today -
- ); -} diff --git a/frontend/src/featureFlags.ts b/frontend/src/featureFlags.ts index a20f67b1..d3251e6d 100644 --- a/frontend/src/featureFlags.ts +++ b/frontend/src/featureFlags.ts @@ -16,7 +16,7 @@ const featureFlags: Record = { unified_homepage: [], results_mode: [], map: ['testers'], - todayPointsBadge: ['all'], + dailyGoalsBadge: ['all'], }; export default featureFlags; diff --git a/frontend/src/hooks/usePlayer.ts b/frontend/src/hooks/usePlayer.ts index 6773eedf..11e12ea2 100644 --- a/frontend/src/hooks/usePlayer.ts +++ b/frontend/src/hooks/usePlayer.ts @@ -1,27 +1,27 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { useNavigate } from "react-router"; -import { updatePlayer, downloadUserData, deleteAccount, fetchTodayPoints } from "../api/player"; +import { updatePlayer, downloadUserData, deleteAccount, fetchDailyGoals } from "../api/player"; import { useAuth } from "../context/AuthContext"; // Query key shared with useActivityInput's refreshAfterActivityChange, which -// invalidates it after every completed task/session so the map view's -// "today" badge (issue #673) updates immediately instead of waiting for the -// next poll. -export const TODAY_POINTS_QUERY_KEY = ["me", "today-points"]; +// invalidates it after every completed task/session so the map view's daily +// goals badge (issue #673, redesigned in #751) updates immediately instead +// of waiting for the next poll. +export const DAILY_GOALS_QUERY_KEY = ["me", "daily-goals"]; // Polled lightly (rather than one-shot like other /me/ data) so the badge // still rolls over to the new day's value on its own if the map view is left // open across midnight, without needing a dedicated push mechanism for that. -const TODAY_POINTS_POLL_INTERVAL_MS = 60_000; +const DAILY_GOALS_POLL_INTERVAL_MS = 60_000; -export function useTodayPoints() { +export function useDailyGoals() { const { isAuthenticated } = useAuth(); return useQuery({ - queryKey: TODAY_POINTS_QUERY_KEY, - queryFn: fetchTodayPoints, + queryKey: DAILY_GOALS_QUERY_KEY, + queryFn: fetchDailyGoals, enabled: isAuthenticated, - refetchInterval: TODAY_POINTS_POLL_INTERVAL_MS, + refetchInterval: DAILY_GOALS_POLL_INTERVAL_MS, }); } diff --git a/frontend/src/pages/MapPage/MapPage.tsx b/frontend/src/pages/MapPage/MapPage.tsx index 04a35d76..45e65503 100644 --- a/frontend/src/pages/MapPage/MapPage.tsx +++ b/frontend/src/pages/MapPage/MapPage.tsx @@ -5,7 +5,7 @@ import Button from "../../components/Button/Button"; import PopulationCentreMap, { type PopulationCentreMapHandle, } from "../../components/Map/Map"; -import TodayPointsBadge from "../../components/TodayPointsBadge/TodayPointsBadge"; +import DailyGoalsBadge from "../../components/DailyGoalsBadge/DailyGoalsBadge"; import { fetchPopulationCentreMap } from "../../api/map"; import FeatureToggle from "../../components/FeatureToggle"; import { @@ -136,8 +136,8 @@ export default function MapPage(): React.ReactElement { onViewportChange={handleViewportChange} worldBounds={worldBounds?.bbox} > - - + + {nextVillage && (
- {minutes}:{seconds.toString().padStart(2, "0")} + {formatDuration(elapsed)}
- ) : null} {canDelete ? ( @@ -197,12 +205,16 @@ export default function TasksPanel({ className={styles.parentSelect} disabled={hasSubtasks} defaultValue={taskItem.parent ?? ""} - onChange={(event) => - updateTask.mutate({ - id: taskItem.id, - data: { parent: event.target.value ? Number(event.target.value) : null }, - }) - } + onChange={(event) => { + saveHelpers.reportSaving(); + updateTask.mutate( + { + id: taskItem.id, + data: { parent: event.target.value ? Number(event.target.value) : null }, + }, + { onSuccess: saveHelpers.reportSaved, onError: saveHelpers.reportError }, + ); + }} > {parentOptions.map((option) => ( diff --git a/frontend/src/components/TasksPanel/useTasksPanel.tsx b/frontend/src/components/TasksPanel/useTasksPanel.tsx index 9da699f5..af963a66 100644 --- a/frontend/src/components/TasksPanel/useTasksPanel.tsx +++ b/frontend/src/components/TasksPanel/useTasksPanel.tsx @@ -245,15 +245,8 @@ export function useTasksPanel(openTaskId?: number | null, onOpenNote?: (noteId: }, []); const handleEdit = useCallback( - (task: ItemRecord, name: string, options?: { parent?: number | null; due_at?: string | null }) => { - updateTask.mutate({ - id: task.id, - data: { - name, - ...(options?.parent !== undefined ? { parent: options.parent } : {}), - ...(options?.due_at !== undefined ? { due_at: options.due_at } : {}), - }, - }); + (task: ItemRecord, name: string, callbacks?: { onSuccess?: () => void; onError?: () => void }) => { + updateTask.mutate({ id: task.id, data: { name } }, callbacks); }, [updateTask] ); diff --git a/frontend/src/hooks/useSimpleCrudPanel.ts b/frontend/src/hooks/useSimpleCrudPanel.ts index 9baa8297..e5437581 100644 --- a/frontend/src/hooks/useSimpleCrudPanel.ts +++ b/frontend/src/hooks/useSimpleCrudPanel.ts @@ -8,6 +8,11 @@ interface Entity { name: string; } +interface SaveCallbacks { + onSuccess?: () => void; + onError?: () => void; +} + interface UseSimpleCrudPanelOptions { useList: () => Pick, "data" | "isLoading">; useCreate: () => Pick>, "mutate">; @@ -41,8 +46,8 @@ export function useSimpleCrudPanel({ ); const handleEdit = useCallback( - (item: T, name: string) => { - update.mutate({ id: item.id, data: { name } as Partial }); + (item: T, name: string, callbacks?: SaveCallbacks) => { + update.mutate({ id: item.id, data: { name } as Partial }, callbacks); }, [update], ); From d14ea3088404e21113c5f90a33845c9754c6b67e Mon Sep 17 00:00:00 2001 From: Duncan Appleby Date: Thu, 13 Aug 2026 18:30:21 +0100 Subject: [PATCH 11/28] fix: keep parent-task field editable and render subtasks as indented rows - TasksPanel: the "Parent task" select was hidden entirely once a task had a parent, with no way to revert it. It now always renders (still disabled when the task itself has subtasks, to prevent nesting). - PlayerItemList: child tasks were rendered nested inside the parent's own
  • , structurally and visually boxed inside it. They now render as independent rows, siblings of other top-level items, indented via a width/margin modifier so the right edge still lines up with the parent's. - gameplay/tasks.py: fix a circular import (gameplay.utils -> gameplay.services.xp_modifiers -> gameplay.tasks -> gameplay.utils) that crashed web/celery on startup, by deferring the broadcast_activity_timer import into the two functions that use it. Fixes #765. --- .../PlayerItemList/PlayerItemList.module.scss | 17 ++--- .../PlayerItemList/PlayerItemList.test.tsx | 11 +++- .../PlayerItemList/PlayerItemList.tsx | 35 +++-------- .../components/TasksPanel/TasksPanel.test.tsx | 11 +++- .../src/components/TasksPanel/TasksPanel.tsx | 62 +++++++++---------- gameplay/tasks.py | 3 +- 6 files changed, 63 insertions(+), 76 deletions(-) diff --git a/frontend/src/components/PlayerItemList/PlayerItemList.module.scss b/frontend/src/components/PlayerItemList/PlayerItemList.module.scss index 87e9fd30..fc7a3e7e 100644 --- a/frontend/src/components/PlayerItemList/PlayerItemList.module.scss +++ b/frontend/src/components/PlayerItemList/PlayerItemList.module.scss @@ -156,19 +156,12 @@ } } -.childList { - list-style: none; - margin: sp.$spacing-xs 0 0; - padding: 0 0 0 sp.$spacing-lg; - display: flex; - flex-direction: column; - gap: sp.$spacing-xs; -} - .childItem { - background: transparent; - border-color: rgba(c.$color-border-primary, 0.12); - opacity: 0.9; + // Shrink width by the indent instead of just shifting it, so the right + // edge still lines up with the parent's โ€” .listItem's width: 100% would + // otherwise push the box that far past the parent's right edge. + width: calc(100% - #{sp.$spacing-lg}); + margin-left: sp.$spacing-lg; } .itemCompleted { diff --git a/frontend/src/components/PlayerItemList/PlayerItemList.test.tsx b/frontend/src/components/PlayerItemList/PlayerItemList.test.tsx index 682abdd2..7cd8e76b 100644 --- a/frontend/src/components/PlayerItemList/PlayerItemList.test.tsx +++ b/frontend/src/components/PlayerItemList/PlayerItemList.test.tsx @@ -154,7 +154,7 @@ describe("PlayerItemList", () => { const child = { id: 11, name: "Child task" }; const flatItems = [parent, child]; - it("renders children nested under their parent without a top-level row", () => { + it("renders children as independent rows, indented, directly after their parent", () => { render( { expect(screen.getByRole("button", { name: "Open task Parent task" })).toBeInTheDocument(); expect(screen.getByRole("button", { name: "Open task Child task" })).toBeInTheDocument(); - // The child is not rendered as its own top-level list item. + // The child is not duplicated as a second top-level entry sourced from `items`. expect(screen.getAllByRole("button", { name: /^Open task / })).toHaveLength(2); + + // Both render as siblings within the list, not one nested inside the other. + const rows = screen.getAllByRole("listitem"); + expect(rows).toHaveLength(2); + expect(rows[0]).toHaveTextContent("Parent task"); + expect(rows[1]).toHaveTextContent("Child task"); + expect(within(rows[0]).queryByText("Child task")).not.toBeInTheDocument(); }); it("is unaffected when getChildren is not passed (ProjectsPanel-style usage)", () => { diff --git a/frontend/src/components/PlayerItemList/PlayerItemList.tsx b/frontend/src/components/PlayerItemList/PlayerItemList.tsx index 53356cc3..398b2f81 100644 --- a/frontend/src/components/PlayerItemList/PlayerItemList.tsx +++ b/frontend/src/components/PlayerItemList/PlayerItemList.tsx @@ -3,7 +3,6 @@ import classNames from "classnames"; import Button from "../Button/Button"; import List from "../List/List"; -import Li from "../List/Li"; import Modal from "../Modal/Modal"; import { usePlayerItemListControls } from "./usePlayerItemListControls"; import { usePlayerItemModal } from "./usePlayerItemModal"; @@ -137,11 +136,15 @@ export default function PlayerItemList { + // Sort/filter controls only apply to top-level items; a child keeps its + // place directly after its parent (in `getChildren`'s order) rather than + // being reordered independently. + const flatDisplayItems = useMemo(() => { if (!getChildren) return displayItems; - return displayItems.filter( + const topLevel = displayItems.filter( (item) => item.id === undefined || !childIds.has(item.id) ); + return topLevel.flatMap((item) => [item, ...(getChildren(item) ?? [])]); }, [displayItems, getChildren, childIds]); const renderRow = (item: T): React.ReactNode => ( @@ -252,7 +255,7 @@ export default function PlayerItemList classNames(styles.item, { + [styles.childItem]: item.id !== undefined && childIds.has(item.id), [styles.itemCompleted]: isItemComplete?.(item), }) } - renderItem={(item) => { - const children = getChildren?.(item); - return ( - <> - {renderRow(item)} - {children?.length ? ( -
      - {children.map((child, index) => ( -
    • - {renderRow(child)} -
      • - ))} -
    - ) : null} - - ); - }} + renderItem={(item) => renderRow(item)} /> diff --git a/frontend/src/components/TasksPanel/TasksPanel.test.tsx b/frontend/src/components/TasksPanel/TasksPanel.test.tsx index 5671697c..34608e33 100644 --- a/frontend/src/components/TasksPanel/TasksPanel.test.tsx +++ b/frontend/src/components/TasksPanel/TasksPanel.test.tsx @@ -325,7 +325,7 @@ describe("TasksPanel", () => { total_records: 0, }; - it("renders a subtask nested under its parent", () => { + it("renders a subtask as an independent, indented row directly after its parent", () => { mockUseTasks.mockReturnValue({ isLoading: false, data: [parentTask, childTask], @@ -336,9 +336,14 @@ describe("TasksPanel", () => { const childButton = screen.getAllByRole("button", { name: "Edit task Child subtask" })[0]; expect(parentButton).toBeInTheDocument(); expect(childButton).toBeInTheDocument(); - // The subtask is nested inside the parent's
  • , not a sibling top-level row. + // The subtask is its own sibling row, not nested inside the parent's
  • . const parentListItem = parentButton.closest("li"); - expect(parentListItem).toContainElement(childButton); + const childListItem = childButton.closest("li"); + expect(parentListItem).not.toBe(childListItem); + expect(parentListItem).not.toContainElement(childButton); + + const rows = screen.getAllByRole("listitem"); + expect(rows.indexOf(childListItem!)).toBe(rows.indexOf(parentListItem!) + 1); }); it("hides a completed parent and its subtasks together", () => { diff --git a/frontend/src/components/TasksPanel/TasksPanel.tsx b/frontend/src/components/TasksPanel/TasksPanel.tsx index e8d64ca0..076a4a46 100644 --- a/frontend/src/components/TasksPanel/TasksPanel.tsx +++ b/frontend/src/components/TasksPanel/TasksPanel.tsx @@ -180,40 +180,38 @@ export default function TasksPanel({ )} - {taskItem.parent == null && ( -
    - - + + + - updateTask.mutate({ - id: taskItem.id, - data: { parent: event.target.value ? Number(event.target.value) : null }, - }) - } - > - - {parentOptions.map((option) => ( - - ))} - - -
    - )} + + {parentOptions.map((option) => ( + + ))} + + + ); }} diff --git a/gameplay/tasks.py b/gameplay/tasks.py index 52311e2e..e1b02306 100644 --- a/gameplay/tasks.py +++ b/gameplay/tasks.py @@ -7,7 +7,6 @@ from character.models import PlayerCharacterLink from .models import XpModifier -from .utils import broadcast_activity_timer DISCONNECT_TASK_CACHE_KEY = "disconnect_task:{player_id}" @@ -28,6 +27,7 @@ def auto_complete_timer_on_disconnect(self, player_id: int): Revoked by TimerConsumer.connect() if the player reconnects in time. """ from .models import ActivityTimer + from .utils import broadcast_activity_timer stored_task_id = cache.get(DISCONNECT_TASK_CACHE_KEY.format(player_id=player_id)) if stored_task_id != self.request.id: @@ -63,6 +63,7 @@ def auto_complete_timers_for_stale_players(): completes them the same way the disconnect grace period does. """ from .models import ActivityTimer + from .utils import broadcast_activity_timer cutoff = timezone.now() - STALE_TIMER_THRESHOLD stale_timers = ( From c49dacfa78669100f16814bd4e9741f7589c077c Mon Sep 17 00:00:00 2001 From: Duncan Appleby Date: Thu, 13 Aug 2026 18:53:48 +0100 Subject: [PATCH 12/28] fix: correct save-status indicator position, add saving-delay, align with modal actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - right: sp.$padding-base was a two-value shorthand (invalid for a single-value offset), so the browser dropped the declaration and the indicator fell back to its static (bottom-left) position, overlapping the Close button. Use $spacing-md instead. - Debounce "Savingโ€ฆ" by 150ms so fast autosaves go straight to "Saved" instead of flashing the interim state. - Move the indicator into the actions row and vertically center it with the Close/Delete buttons instead of anchoring to the modal's bottom edge; bump contrast (border, stronger shadow, bolder text) so it reads clearly without changing its size. Co-Authored-By: Claude Sonnet 5 --- .../PlayerItemList/PlayerItemList.module.scss | 10 ++++-- .../PlayerItemList/PlayerItemList.tsx | 2 +- .../PlayerItemList/useSaveStatus.ts | 35 +++++++++++++++---- 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/frontend/src/components/PlayerItemList/PlayerItemList.module.scss b/frontend/src/components/PlayerItemList/PlayerItemList.module.scss index 760a5243..97e5d4da 100644 --- a/frontend/src/components/PlayerItemList/PlayerItemList.module.scss +++ b/frontend/src/components/PlayerItemList/PlayerItemList.module.scss @@ -259,6 +259,7 @@ } .editConfirmActions { + position: relative; display: flex; justify-content: flex-start; align-items: center; @@ -293,14 +294,17 @@ .saveStatus { position: absolute; - right: sp.$padding-base; - bottom: sp.$spacing-sm; + right: 0; + top: 50%; + transform: translateY(-50%); padding: sp.$spacing-xs sp.$spacing-sm; border-radius: sp.$border-radius; + border: 1px solid rgba(c.$color-border-primary, 0.4); background: rgba(c.$color-bg, 0.95); - box-shadow: 0 2px 6px rgba(0, 0, 0, 0.15); + box-shadow: 0 2px 8px rgba(0, 0, 0, 0.25); pointer-events: none; z-index: 1; + font-weight: 600; @include t.apply-text-style(t.$text-caption); animation: saveStatusFadeIn 0.15s ease; } diff --git a/frontend/src/components/PlayerItemList/PlayerItemList.tsx b/frontend/src/components/PlayerItemList/PlayerItemList.tsx index 381830e1..549a542d 100644 --- a/frontend/src/components/PlayerItemList/PlayerItemList.tsx +++ b/frontend/src/components/PlayerItemList/PlayerItemList.tsx @@ -363,8 +363,8 @@ export default function PlayerItemList ) : null} + - )} diff --git a/frontend/src/components/PlayerItemList/useSaveStatus.ts b/frontend/src/components/PlayerItemList/useSaveStatus.ts index b51370a9..077f03da 100644 --- a/frontend/src/components/PlayerItemList/useSaveStatus.ts +++ b/frontend/src/components/PlayerItemList/useSaveStatus.ts @@ -9,6 +9,9 @@ export interface SaveStatusHelpers { } const SAVED_DISPLAY_MS = 2500; +// Saves usually resolve fast enough that "Savingโ€ฆ" would just flash on +// screen; only show it once a save has been pending this long. +const SAVING_DISPLAY_DELAY_MS = 150; /** * Tracks a single in-flight autosave's status so a modal can show a brief @@ -19,6 +22,7 @@ const SAVED_DISPLAY_MS = 2500; export function useSaveStatus() { const [saveStatus, setSaveStatus] = useState("idle"); const hideTimerRef = useRef | null>(null); + const savingTimerRef = useRef | null>(null); const clearHideTimer = useCallback(() => { if (hideTimerRef.current) { @@ -27,28 +31,47 @@ export function useSaveStatus() { } }, []); + const clearSavingTimer = useCallback(() => { + if (savingTimerRef.current) { + clearTimeout(savingTimerRef.current); + savingTimerRef.current = null; + } + }, []); + const reportSaving = useCallback(() => { clearHideTimer(); - setSaveStatus("saving"); - }, [clearHideTimer]); + clearSavingTimer(); + savingTimerRef.current = setTimeout(() => { + savingTimerRef.current = null; + setSaveStatus("saving"); + }, SAVING_DISPLAY_DELAY_MS); + }, [clearHideTimer, clearSavingTimer]); const reportSaved = useCallback(() => { clearHideTimer(); + clearSavingTimer(); setSaveStatus("saved"); hideTimerRef.current = setTimeout(() => setSaveStatus("idle"), SAVED_DISPLAY_MS); - }, [clearHideTimer]); + }, [clearHideTimer, clearSavingTimer]); const reportError = useCallback(() => { clearHideTimer(); + clearSavingTimer(); setSaveStatus("error"); - }, [clearHideTimer]); + }, [clearHideTimer, clearSavingTimer]); const resetSaveStatus = useCallback(() => { clearHideTimer(); + clearSavingTimer(); setSaveStatus("idle"); - }, [clearHideTimer]); + }, [clearHideTimer, clearSavingTimer]); - useEffect(() => clearHideTimer, [clearHideTimer]); + useEffect(() => { + return () => { + clearHideTimer(); + clearSavingTimer(); + }; + }, [clearHideTimer, clearSavingTimer]); return { saveStatus, reportSaving, reportSaved, reportError, resetSaveStatus }; } From c47dd00d81a7c93c37da8a6673f7dc46eb7a892b Mon Sep 17 00:00:00 2001 From: Duncan Appleby Date: Thu, 13 Aug 2026 20:54:31 +0100 Subject: [PATCH 13/28] feat: extend formatDueAt with weeks/months granularity Scale due-date formatting smoothly on both sides of now: days (2-6), then weeks[, leftover days] out to 8 weeks, then months, matching the pattern already used by "last worked on". Future dates beyond ~6 months still fall back to an absolute date; past dates keep counting months uncapped. Closes #760 Co-Authored-By: Claude Sonnet 5 --- frontend/src/utils/formatUtils.test.ts | 44 ++++++++++++++++++++------ frontend/src/utils/formatUtils.ts | 33 +++++++++++++++---- 2 files changed, 60 insertions(+), 17 deletions(-) diff --git a/frontend/src/utils/formatUtils.test.ts b/frontend/src/utils/formatUtils.test.ts index c4090fe3..474acf97 100644 --- a/frontend/src/utils/formatUtils.test.ts +++ b/frontend/src/utils/formatUtils.test.ts @@ -31,16 +31,16 @@ describe("formatDueAt", () => { expect(formatDueAt(null)).toBe("-"); }); - it("labels the current day as Today", () => { - expect(formatDueAt(atLocalMidnight(0))).toBe("Today"); + it("labels the current day as today", () => { + expect(formatDueAt(atLocalMidnight(0))).toBe("today"); }); - it("labels the next day as Tomorrow", () => { - expect(formatDueAt(atLocalMidnight(1))).toBe("Tomorrow"); + it("labels the next day as tomorrow", () => { + expect(formatDueAt(atLocalMidnight(1))).toBe("tomorrow"); }); - it("labels the previous day as Yesterday", () => { - expect(formatDueAt(atLocalMidnight(-1))).toBe("Yesterday"); + it("labels the previous day as yesterday", () => { + expect(formatDueAt(atLocalMidnight(-1))).toBe("yesterday"); }); it("labels a few days out as 'in N days'", () => { @@ -51,13 +51,37 @@ describe("formatDueAt", () => { expect(formatDueAt(atLocalMidnight(-3))).toBe("3 days ago"); }); - it("labels a further-back date in weeks", () => { + it("labels a further-back date in whole weeks", () => { expect(formatDueAt(atLocalMidnight(-14))).toBe("2 weeks ago"); }); - it("labels a further-out date as an absolute weekday/day/month", () => { - // 9 days out from Wed 15 Jul 2026 is Fri 24 Jul 2026. - expect(formatDueAt(atLocalMidnight(9))).toBe("Fri 24th Jul"); + it("labels a further-out date in weeks, with a leftover-days remainder", () => { + expect(formatDueAt(atLocalMidnight(9))).toBe("in 1 week, 2 days"); + }); + + it("labels a further-out whole-week date without a days remainder", () => { + expect(formatDueAt(atLocalMidnight(14))).toBe("in 2 weeks"); + }); + + it("labels a further-back date in weeks, with a leftover-days remainder", () => { + expect(formatDueAt(atLocalMidnight(-20))).toBe("2 weeks, 6 days ago"); + }); + + it("labels a date beyond the week cutoff in months (future)", () => { + expect(formatDueAt(atLocalMidnight(60))).toBe("in 2 months"); + }); + + it("labels a date beyond the week cutoff in months (past)", () => { + expect(formatDueAt(atLocalMidnight(-60))).toBe("2 months ago"); + }); + + it("keeps counting months uncapped on the past side", () => { + expect(formatDueAt(atLocalMidnight(-200))).toBe("7 months ago"); + }); + + it("falls back to an absolute date beyond the month cap (future)", () => { + // 200 days out from Wed 15 Jul 2026 is Sun 31 Jan 2027. + expect(formatDueAt(atLocalMidnight(200))).toBe("Sun 31st Jan"); }); }); diff --git a/frontend/src/utils/formatUtils.ts b/frontend/src/utils/formatUtils.ts index d7eed39c..3bbd83b4 100644 --- a/frontend/src/utils/formatUtils.ts +++ b/frontend/src/utils/formatUtils.ts @@ -75,6 +75,21 @@ function startOfDay(date: Date): Date { return new Date(date.getFullYear(), date.getMonth(), date.getDate()); } +const WEEK_CUTOFF_DAYS = 56; // 8 weeks: weeks[, days] granularity applies up to this many days out/back +const MONTH_CUTOFF_DAYS = 180; // 6 months: beyond this on the future side, fall back to an absolute date + +function formatWeeksAndDays(totalDays: number): string { + const weeks = Math.floor(totalDays / 7); + const days = totalDays % 7; + const weeksPart = `${weeks} ${pluralize(weeks, "week")}`; + return days > 0 ? `${weeksPart}, ${days} ${pluralize(days, "day")}` : weeksPart; +} + +function formatMonths(totalDays: number): string { + const months = Math.max(1, Math.round(totalDays / 30)); + return `${months} ${pluralize(months, "month")}`; +} + export function formatDueAt(dueAt: string | null): string { if (!dueAt) return "-"; const date = new Date(dueAt); @@ -84,19 +99,23 @@ export function formatDueAt(dueAt: string | null): string { (startOfDay(date).getTime() - startOfDay(new Date()).getTime()) / (24 * 60 * 60 * 1000) ); - if (diffDays === 0) return "Today"; - if (diffDays === 1) return "Tomorrow"; - if (diffDays === -1) return "Yesterday"; + if (diffDays === 0) return "today"; + if (diffDays === 1) return "tomorrow"; + if (diffDays === -1) return "yesterday"; if (diffDays > 1 && diffDays <= 6) return `in ${diffDays} days`; if (diffDays < -1 && diffDays >= -6) return `${-diffDays} days ago`; - if (diffDays < -6) { - const weeks = Math.round(-diffDays / 7); - return `${weeks} week${weeks === 1 ? "" : "s"} ago`; + if (diffDays > 6 && diffDays <= WEEK_CUTOFF_DAYS) return `in ${formatWeeksAndDays(diffDays)}`; + if (diffDays < -6 && diffDays >= -WEEK_CUTOFF_DAYS) return `${formatWeeksAndDays(-diffDays)} ago`; + + if (diffDays < -WEEK_CUTOFF_DAYS) return `${formatMonths(-diffDays)} ago`; + + if (diffDays > WEEK_CUTOFF_DAYS && diffDays <= MONTH_CUTOFF_DAYS) { + return `in ${formatMonths(diffDays)}`; } - // diffDays > 6: further out than a week, show an absolute date. + // diffDays > MONTH_CUTOFF_DAYS: further out than ~6 months, show an absolute date. const weekday = date.toLocaleDateString(undefined, { weekday: "short" }); const month = date.toLocaleDateString(undefined, { month: "short" }); const day = date.getDate(); From 37f09c668c94004f1494cb93fdba12f645f9fdd8 Mon Sep 17 00:00:00 2001 From: Duncan Appleby Date: Thu, 13 Aug 2026 21:08:13 +0100 Subject: [PATCH 14/28] fix: simplify resident line display by removing activity status --- frontend/src/components/BuildingDetail/BuildingDetail.tsx | 3 +-- frontend/src/components/List/List.module.scss | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/frontend/src/components/BuildingDetail/BuildingDetail.tsx b/frontend/src/components/BuildingDetail/BuildingDetail.tsx index c14c6aef..f114f6b7 100644 --- a/frontend/src/components/BuildingDetail/BuildingDetail.tsx +++ b/frontend/src/components/BuildingDetail/BuildingDetail.tsx @@ -35,8 +35,7 @@ function capitalize(word: string): string { function residentLine(resident: BuildingDetailResident): string { // "walking" overrides the scheduled activity while moving, same rule as // a character's own map tooltip (CharacterTooltipContent). - const activity = resident.isMoving ? "walking" : resident.currentActivity; - return activity ? `${resident.name} โ€” ${activity}` : resident.name; + return resident.name; } export default function BuildingDetail({ diff --git a/frontend/src/components/List/List.module.scss b/frontend/src/components/List/List.module.scss index 09ae1119..836525a9 100644 --- a/frontend/src/components/List/List.module.scss +++ b/frontend/src/components/List/List.module.scss @@ -61,7 +61,6 @@ .canHover .listItem:hover { background: rgba(c.$color-border-primary, 0.18); - border-color: rgba(c.$color-border-primary, 0.7); @include m.box-shadow(md); z-index: 1; cursor: default; From ca7adfd26147ce7c35b773d210e173f2d79bdd1f Mon Sep 17 00:00:00 2001 From: Duncan Appleby Date: Thu, 13 Aug 2026 21:40:13 +0100 Subject: [PATCH 15/28] style: standardize form control dimensions and radius across components --- .../ActivityInput/ActivityInput.module.scss | 18 +----------------- .../EntitySearchInput.module.scss | 3 ++- .../src/components/Input/Input.module.scss | 3 ++- .../PlayerItemList/PlayerItemList.module.scss | 3 ++- .../UnifiedTimerHome.module.scss | 14 +------------- frontend/src/styles/semantic/_spacing.scss | 3 +++ 6 files changed, 11 insertions(+), 33 deletions(-) diff --git a/frontend/src/components/ActivityInput/ActivityInput.module.scss b/frontend/src/components/ActivityInput/ActivityInput.module.scss index 6a5a3d63..f4725474 100644 --- a/frontend/src/components/ActivityInput/ActivityInput.module.scss +++ b/frontend/src/components/ActivityInput/ActivityInput.module.scss @@ -4,7 +4,7 @@ @use '../../styles/semantic/spacing' as sp; @use 'sass:map'; -$row-control-height: 44px; +$row-control-height: sp.$form-control-height; .control { height: $row-control-height; display: flex; @@ -216,14 +216,6 @@ $row-control-height: 44px; } @include m.respond-to(md, down) { - .control { - height: 40px; - } - - .grow { - min-height: 40px; - } - .inputText { font-size: 1rem; line-height: 1.1; @@ -242,14 +234,6 @@ $row-control-height: 44px; } @include m.respond-to(sm, down) { - .control { - height: 38px; - } - - .grow { - min-height: 38px; - } - .inputText { font-size: 0.9rem; line-height: 1.05; diff --git a/frontend/src/components/EntitySearchInput/EntitySearchInput.module.scss b/frontend/src/components/EntitySearchInput/EntitySearchInput.module.scss index 2a60608e..48c964ef 100644 --- a/frontend/src/components/EntitySearchInput/EntitySearchInput.module.scss +++ b/frontend/src/components/EntitySearchInput/EntitySearchInput.module.scss @@ -13,9 +13,10 @@ .input { width: 100%; min-width: 0; + height: sp.$form-control-height; padding: sp.$padding-base; box-sizing: border-box; - border-radius: sp.$border-radius; + border-radius: sp.$form-control-radius; @include t.apply-text-style(t.$text-body); @include m.border-interactive( rgba(c.$color-border-primary, 0.4), diff --git a/frontend/src/components/Input/Input.module.scss b/frontend/src/components/Input/Input.module.scss index a991bf0a..6441da5e 100644 --- a/frontend/src/components/Input/Input.module.scss +++ b/frontend/src/components/Input/Input.module.scss @@ -27,6 +27,7 @@ .inputField { padding: sp.$padding-base; + height: sp.$form-control-height; box-sizing: border-box; max-width: 30rem; @include m.border-interactive( @@ -34,7 +35,7 @@ c.$color-border-primary, c.$color-border-accent ); - border-radius: sp.$border-radius; + border-radius: sp.$form-control-radius; transition: border 0.2s, background 0.2s; @include t.apply-text-style(t.$text-body); diff --git a/frontend/src/components/PlayerItemList/PlayerItemList.module.scss b/frontend/src/components/PlayerItemList/PlayerItemList.module.scss index e1162d13..d577afcd 100644 --- a/frontend/src/components/PlayerItemList/PlayerItemList.module.scss +++ b/frontend/src/components/PlayerItemList/PlayerItemList.module.scss @@ -228,9 +228,10 @@ .editInput { flex: 1; min-width: 0; + height: sp.$form-control-height; padding: sp.$spacing-xs sp.$spacing-sm; border: 2px solid rgba(c.$color-status-success, 0.45); - border-radius: sp.$border-radius; + border-radius: sp.$form-control-radius; font-size: 1em; font-family: inherit; transition: border-color 0.2s ease; diff --git a/frontend/src/components/UnifiedTimerHome/UnifiedTimerHome.module.scss b/frontend/src/components/UnifiedTimerHome/UnifiedTimerHome.module.scss index 2793e6af..db2234e2 100644 --- a/frontend/src/components/UnifiedTimerHome/UnifiedTimerHome.module.scss +++ b/frontend/src/components/UnifiedTimerHome/UnifiedTimerHome.module.scss @@ -4,7 +4,7 @@ @use '../../styles/semantic/spacing' as sp; @use 'sass:map'; -$row-control-height: 44px; +$row-control-height: sp.$form-control-height; .wrapper { position: relative; @@ -180,25 +180,13 @@ $row-control-height: 44px; } @include m.respond-to(md, down) { - .timerPill, - .ctaButton { - height: 40px; - } - .inputText { - height: 40px; font-size: 1rem; } } @include m.respond-to(sm, down) { - .timerPill, - .ctaButton { - height: 38px; - } - .inputText { - height: 38px; font-size: 0.9rem; } } diff --git a/frontend/src/styles/semantic/_spacing.scss b/frontend/src/styles/semantic/_spacing.scss index d9a3c5a1..3e326fe5 100644 --- a/frontend/src/styles/semantic/_spacing.scss +++ b/frontend/src/styles/semantic/_spacing.scss @@ -38,6 +38,9 @@ $button-padding: $spacing-xs $spacing-md; $button-radius: $spacing-md; $content-padding: $spacing-lg; +$form-control-height: 2rem; +$form-control-radius: $border-radius; + // === Gap / radius / shadow collections === $gap: ( xs: map.get(s.$spacing, xs), From 44ce250174efead2195696852fac5b554ae75fc6 Mon Sep 17 00:00:00 2001 From: Duncan Appleby Date: Thu, 13 Aug 2026 22:46:23 +0100 Subject: [PATCH 16/28] test: update resident-display tests for name-only rendering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BuildingDetail.tsx's residentLine() was simplified in 37f09c66 to drop the " โ€” idle"/" โ€” walking" activity-status suffix, but the tests weren't updated to match, leaving BuildingDetail.test.tsx and Map.test.tsx failing on development. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Mur33yGv419VMfAaKxjaGS --- .../src/components/BuildingDetail/BuildingDetail.test.tsx | 6 +++--- frontend/src/components/Map/Map.test.tsx | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/frontend/src/components/BuildingDetail/BuildingDetail.test.tsx b/frontend/src/components/BuildingDetail/BuildingDetail.test.tsx index 85593148..6d986ec6 100644 --- a/frontend/src/components/BuildingDetail/BuildingDetail.test.tsx +++ b/frontend/src/components/BuildingDetail/BuildingDetail.test.tsx @@ -56,7 +56,7 @@ describe('BuildingDetail', () => { expect(screen.queryByText(/Wheat/)).not.toBeInTheDocument(); }); - it('shows each resident, with "walking" overriding their scheduled activity', () => { + it('shows each resident by name only', () => { render( { /> ); - expect(screen.getByText('Alice โ€” idle')).toBeInTheDocument(); - expect(screen.getByText('Bob โ€” walking')).toBeInTheDocument(); + expect(screen.getByText('Alice')).toBeInTheDocument(); + expect(screen.getByText('Bob')).toBeInTheDocument(); }); it('calls onSelectResident when a resident row is clicked', async () => { diff --git a/frontend/src/components/Map/Map.test.tsx b/frontend/src/components/Map/Map.test.tsx index 33f39f40..c17e6d9c 100644 --- a/frontend/src/components/Map/Map.test.tsx +++ b/frontend/src/components/Map/Map.test.tsx @@ -1165,7 +1165,6 @@ describe('PopulationCentreMap entity detail card', () => { const dialog = await screen.findByRole('dialog', { name: 'House 2' }); expect(dialog).toHaveTextContent('1 / 4'); expect(dialog).toHaveTextContent('Alice'); - expect(dialog).toHaveTextContent('idle'); }); it('switches to a resident\'s own detail card when clicked inside the building detail card', async () => { From c254fbbc937757103276e14d347dba439ae4321a Mon Sep 17 00:00:00 2001 From: Duncan Appleby Date: Thu, 13 Aug 2026 22:46:32 +0100 Subject: [PATCH 17/28] feat: add offline activity logging modal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a "Log activity" flow (issue #704) for recording work done outside a running timer: a "+" entry point next to the activities summary opens a modal to search/enter a task, split hours+minutes duration, and a completion date/time (capped at today via the native date picker). Submits to the existing backend endpoint and surfaces its XP-eligibility messaging โ€” the backend remains authoritative on whether XP is actually awarded; the client only does obvious sanity-checks (positive duration, required fields). Also fixes two things this surfaced along the way: - Input.tsx dropped the native `required` attribute (kept aria-required) โ€” it was silently blocking form submission in favor of the browser's own validation UI, overriding the app's styled error messages. - Modal.module.scss's .modalContent had overflow-x: hidden with no horizontal padding, clipping the focus ring on any full-width input flush against its edge. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Mur33yGv419VMfAaKxjaGS --- frontend/src/api/activities.ts | 20 ++ .../ActivitiesPanel.module.scss | 28 +- .../ActivitiesPanel/ActivitiesPanel.test.tsx | 13 +- .../ActivitiesPanel/ActivitiesPanel.tsx | 25 +- frontend/src/components/Input/Input.tsx | 10 +- .../LogOfflineActivityModal.module.scss | 126 +++++++++ .../LogOfflineActivityModal.test.tsx | 122 +++++++++ .../LogOfflineActivityModal.tsx | 171 +++++++++++++ .../useLogOfflineActivityForm.ts | 242 ++++++++++++++++++ .../src/components/Modal/Modal.module.scss | 3 +- frontend/src/hooks/useActivities.ts | 16 +- frontend/src/types/api.ts | 11 + frontend/src/types/index.ts | 1 + 13 files changed, 778 insertions(+), 10 deletions(-) create mode 100644 frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.module.scss create mode 100644 frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.test.tsx create mode 100644 frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.tsx create mode 100644 frontend/src/components/LogOfflineActivityModal/useLogOfflineActivityForm.ts diff --git a/frontend/src/api/activities.ts b/frontend/src/api/activities.ts index bbb97450..2d8a8019 100644 --- a/frontend/src/api/activities.ts +++ b/frontend/src/api/activities.ts @@ -1,7 +1,18 @@ // src/api/activities.ts import type { PlayerActivity } from "../types"; +import type { OfflineActivityLogResponse } from "../types/api"; import { apiFetch } from "../utils/api"; +export interface OfflineActivityLogPayload { + /** Name for a newly-created task; ignored (and optional) when `task` is set. */ + name?: string; + description?: string; + skill?: number; + task?: number; + started_at: string; + completed_at: string; +} + export function fetchActivities(): Promise { return (async () => { const allResults: PlayerActivity[] = []; @@ -45,3 +56,12 @@ export function deleteActivity(id: number): Promise { method: "DELETE", }); } + +export function logOfflineActivity( + data: OfflineActivityLogPayload +): Promise { + return apiFetch("/player-activities/log_offline/", { + method: "POST", + body: JSON.stringify(data), + }); +} diff --git a/frontend/src/components/ActivitiesPanel/ActivitiesPanel.module.scss b/frontend/src/components/ActivitiesPanel/ActivitiesPanel.module.scss index a1f3229b..520b000d 100644 --- a/frontend/src/components/ActivitiesPanel/ActivitiesPanel.module.scss +++ b/frontend/src/components/ActivitiesPanel/ActivitiesPanel.module.scss @@ -14,13 +14,37 @@ flex-direction: column; } -.dateTabs { +.toolbar { display: flex; + align-items: center; gap: sp.$spacing-sm; margin-bottom: sp.$spacing-md; + width: 100%; +} + +.dateTabs { + display: flex; + gap: sp.$spacing-sm; flex-wrap: wrap; justify-content: center; - width: 100%; + flex: 1; +} + +.addOfflineButton { + @include c.apply-button-variant(primary); + flex-shrink: 0; + border: none; + border-radius: 50%; + width: 2.25rem; + height: 2.25rem; + min-width: 2.25rem; + padding: 0; + font-size: 1.25rem; + line-height: 1; + display: inline-flex; + align-items: center; + justify-content: center; + cursor: pointer; } .dateButton { diff --git a/frontend/src/components/ActivitiesPanel/ActivitiesPanel.test.tsx b/frontend/src/components/ActivitiesPanel/ActivitiesPanel.test.tsx index f1c92d91..99bfd72f 100644 --- a/frontend/src/components/ActivitiesPanel/ActivitiesPanel.test.tsx +++ b/frontend/src/components/ActivitiesPanel/ActivitiesPanel.test.tsx @@ -2,8 +2,17 @@ import { render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import { TooltipProvider } from "../Tooltip/Tooltip"; import ActivitiesPanel from "./ActivitiesPanel"; +function renderActivitiesPanel() { + return render( + + + + ); +} + const mockUseActivities = vi.fn(); const mockUseDeleteActivity = vi.fn(); const mockUseUpdateActivity = vi.fn(); @@ -39,7 +48,7 @@ describe("ActivitiesPanel", () => { it("renders activities and delegates edit through PlayerItemList", async () => { const user = userEvent.setup(); - render(); + renderActivitiesPanel(); expect(screen.getByText("Write docs")).toBeInTheDocument(); @@ -59,7 +68,7 @@ describe("ActivitiesPanel", () => { it("delegates delete confirmation through PlayerItemList", async () => { const user = userEvent.setup(); - render(); + renderActivitiesPanel(); await user.click(screen.getByRole("button", { name: "Open activity Write docs" })); await user.click(within(screen.getByRole("dialog")).getByRole("button", { name: "Delete" })); diff --git a/frontend/src/components/ActivitiesPanel/ActivitiesPanel.tsx b/frontend/src/components/ActivitiesPanel/ActivitiesPanel.tsx index 39ef6a07..72e3c820 100644 --- a/frontend/src/components/ActivitiesPanel/ActivitiesPanel.tsx +++ b/frontend/src/components/ActivitiesPanel/ActivitiesPanel.tsx @@ -6,6 +6,8 @@ import { formatDurationShort, pluralize } from "../../utils/formatUtils"; import Button from "../Button/Button"; import PlayerItemList from "../PlayerItemList/PlayerItemList"; +import Tooltip from "../Tooltip/Tooltip"; +import LogOfflineActivityModal from "../LogOfflineActivityModal/LogOfflineActivityModal"; import styles from "./ActivitiesPanel.module.scss"; type DateCategory = "today" | "yesterday" | "older"; @@ -68,6 +70,7 @@ export default function ActivitiesPanel(): React.ReactElement | null { const deleteActivity = useDeleteActivity(); const updateActivity = useUpdateActivity(); const [activeTab, setActiveTab] = useState("today"); + const [isLogModalOpen, setIsLogModalOpen] = useState(false); const bucketed = useMemo(() => bucketActivities(activities ?? []), [activities]); @@ -127,8 +130,8 @@ export default function ActivitiesPanel(): React.ReactElement | null { return (
    - {hasActivities && ( - <> +
    + {hasActivities && (
    {dateTabs.map(({ key, label }) => (
    + )} + + + +
    + {isLogModalOpen && ( + setIsLogModalOpen(false)} /> + )} + + {hasActivities && ( + <> {hasTabActivities ? (
    {Object.entries(activitiesByDay).map(([dateKey, dayActivities]) => { diff --git a/frontend/src/components/Input/Input.tsx b/frontend/src/components/Input/Input.tsx index b95a9176..c0b9343b 100644 --- a/frontend/src/components/Input/Input.tsx +++ b/frontend/src/components/Input/Input.tsx @@ -23,6 +23,7 @@ function EyeOffIcon() { interface InputProps { id: string; label?: string; + ariaLabel?: string; type?: string; value?: string; onChange?: (value: string | boolean) => void; @@ -34,6 +35,8 @@ interface InputProps { checked?: boolean; minLength?: number; maxLength?: number; + min?: string | number; + max?: string | number; className?: string; inputClassName?: string; disabled?: boolean; @@ -44,6 +47,7 @@ interface InputProps { export default function Input({ id, label, + ariaLabel, type = 'text', value, onChange, @@ -55,6 +59,8 @@ export default function Input({ checked, minLength, maxLength, + min, + max, className, inputClassName, disabled = false, @@ -91,13 +97,15 @@ export default function Input({ onBlur={onBlur} onKeyDown={onKeyDown} placeholder={placeholder} + aria-label={!label ? ariaLabel : undefined} aria-invalid={!!error} aria-describedby={describedBy} aria-required={required} autoComplete={autoComplete} - required={required} minLength={minLength} maxLength={maxLength} + min={min} + max={max} disabled={disabled} /> ); diff --git a/frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.module.scss b/frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.module.scss new file mode 100644 index 00000000..8c7e4788 --- /dev/null +++ b/frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.module.scss @@ -0,0 +1,126 @@ +@use '../../styles/semantic/spacing' as sp; +@use '../../styles/semantic/typography' as t; +@use '../../styles/semantic/colors' as c; +@use '../../styles/utilities/mixins' as m; + +.form { + display: flex; + flex-direction: column; + gap: sp.$spacing-md; + width: 100%; +} + +.field { + display: flex; + flex-direction: column; + gap: sp.$spacing-xs; +} + +.label { + @include t.apply-text-style(t.$text-body); + font-weight: 600; +} + +.required { + color: c.$color-error; +} + +.row { + display: flex; + gap: sp.$spacing-sm; + width: 100%; + + > * { + flex: 1; + min-width: 0; + } +} + +.durationPartField { + flex: 0 0 auto; + width: 4.5rem; + max-width: 4.5rem; + + input[type='number'] { + appearance: textfield; + -moz-appearance: textfield; + + &::-webkit-outer-spin-button, + &::-webkit-inner-spin-button { + appearance: none; + -webkit-appearance: none; + margin: 0; + } + } +} + +.taskInput { + width: 100%; +} + +.durationPreview { + @include t.apply-text-style(t.$text-body); + opacity: 0.75; + margin: 0; +} + +.errorText { + @include t.apply-text-style(t.$text-body); + color: c.$color-error; + margin: 0; +} + +.eligibilityBanner { + @include t.apply-text-style(t.$text-body); + margin: 0; + padding: sp.$spacing-sm sp.$spacing-md; + border-radius: sp.$border-radius; + border: 1px solid transparent; + + &.info { + background: rgba(c.$color-status-info, 0.1); + border-color: rgba(c.$color-status-info, 0.4); + } + + &.success { + background: rgba(c.$color-status-success, 0.1); + border-color: rgba(c.$color-status-success, 0.4); + } + + &.warning { + background: rgba(c.$color-warning, 0.12); + border-color: rgba(c.$color-warning, 0.4); + } +} + +.actions { + display: flex; + justify-content: flex-end; + gap: sp.$spacing-sm; + margin-top: sp.$spacing-sm; +} + +.confirmation { + display: flex; + flex-direction: column; + gap: sp.$spacing-md; + width: 100%; + text-align: center; +} + +.confirmationHeadline { + @include t.apply-text-style(t.$text-body); + margin: 0; +} + +.confirmationActions { + display: flex; + justify-content: center; + gap: sp.$spacing-sm; +} + +@include m.respond-to(sm, down) { + .row { + flex-direction: column; + } +} diff --git a/frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.test.tsx b/frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.test.tsx new file mode 100644 index 00000000..b8e3763b --- /dev/null +++ b/frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.test.tsx @@ -0,0 +1,122 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { TooltipProvider } from "../Tooltip/Tooltip"; +import LogOfflineActivityModal from "./LogOfflineActivityModal"; + +const logMutate = vi.fn(); +const fetchPlayerAndCharacter = vi.fn(); +let gameValue: Record; + +vi.mock("../../hooks/useActivities", () => ({ + useLogOfflineActivity: () => ({ mutate: logMutate, isPending: false }), +})); + +vi.mock("../../hooks/useGame", () => ({ + useGame: () => gameValue, +})); + +// Stub the autocomplete input so these tests don't depend on the search cache. +vi.mock("../EntitySearchInput/EntitySearchInput", () => ({ + default: ({ + value, + onChange, + placeholder, + }: { + value: string; + onChange?: (v: string) => void; + placeholder?: string; + }) => ( + onChange?.(event.target.value)} + /> + ), +})); + +function renderModal(onClose = vi.fn()) { + return render( + + + + ); +} + +describe("LogOfflineActivityModal", () => { + beforeEach(() => { + logMutate.mockReset(); + fetchPlayerAndCharacter.mockReset(); + gameValue = { player: { is_premium: true }, fetchPlayerAndCharacter }; + }); + + it("blocks submission and shows errors when required fields are missing", async () => { + const user = userEvent.setup(); + renderModal(); + + await user.click(screen.getByRole("button", { name: "Log activity" })); + + expect(await screen.findByText("Enter or select a task.")).toBeInTheDocument(); + expect(screen.getByText("Enter a duration greater than zero.")).toBeInTheDocument(); + expect(logMutate).not.toHaveBeenCalled(); + }); + + it("submits a valid entry and shows the XP confirmation", async () => { + const user = userEvent.setup(); + renderModal(); + + await user.type(screen.getByLabelText("Task"), "Write docs"); + await user.type(screen.getByLabelText("Minutes"), "30"); + + await user.click(screen.getByRole("button", { name: "Log activity" })); + + await waitFor(() => expect(logMutate).toHaveBeenCalledTimes(1)); + const [payload, callbacks] = logMutate.mock.calls[0]; + expect(payload.name).toBe("Write docs"); + expect(payload.started_at < payload.completed_at).toBe(true); + + callbacks.onSuccess({ + success: true, + message: "Activity logged", + activity: { name: "Write docs" }, + xp_gained: 42, + xp_eligible_seconds: 1800, + level_ups: [], + }); + + expect(await screen.findByText("+42 XP awarded")).toBeInTheDocument(); + expect(fetchPlayerAndCharacter).toHaveBeenCalled(); + }); + + it("surfaces the backend error message on failure", async () => { + const user = userEvent.setup(); + renderModal(); + + await user.type(screen.getByLabelText("Task"), "Write docs"); + await user.type(screen.getByLabelText("Minutes"), "30"); + await user.click(screen.getByRole("button", { name: "Log activity" })); + + await waitFor(() => expect(logMutate).toHaveBeenCalledTimes(1)); + const [, callbacks] = logMutate.mock.calls[0]; + + callbacks.onError(new Error(JSON.stringify({ success: false, message: "Daily limit reached." }))); + + expect(await screen.findByText("Daily limit reached.")).toBeInTheDocument(); + }); + + it("warns free-tier users that XP won't be awarded", async () => { + gameValue = { player: { is_premium: false }, fetchPlayerAndCharacter }; + const user = userEvent.setup(); + renderModal(); + + await user.type(screen.getByLabelText("Minutes"), "10"); + + expect( + await screen.findByText( + "This will be recorded, but offline activities only earn XP for Premium accounts." + ) + ).toBeInTheDocument(); + }); +}); diff --git a/frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.tsx b/frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.tsx new file mode 100644 index 00000000..bd3f6c83 --- /dev/null +++ b/frontend/src/components/LogOfflineActivityModal/LogOfflineActivityModal.tsx @@ -0,0 +1,171 @@ +import classNames from "classnames"; + +import Modal from "../Modal/Modal"; +import Button from "../Button/Button"; +import Input from "../Input/Input"; +import EntitySearchInput from "../EntitySearchInput/EntitySearchInput"; +import { formatDurationShort } from "../../utils/formatUtils"; +import { useLogOfflineActivityForm } from "./useLogOfflineActivityForm"; +import styles from "./LogOfflineActivityModal.module.scss"; + +interface LogOfflineActivityModalProps { + onClose: () => void; +} + +export default function LogOfflineActivityModal({ onClose }: LogOfflineActivityModalProps) { + const { + taskName, + handleTaskNameChange, + handleTaskSelect, + durationHours, + setDurationHours, + durationMinutes, + setDurationMinutes, + totalDurationMinutes, + completionDate, + handleCompletionDateChange, + completionTime, + handleCompletionTimeChange, + maxCompletionDate, + fieldErrors, + submitError, + result, + eligibility, + isSubmitting, + handleSubmit, + reset, + } = useLogOfflineActivityForm(); + + if (result) { + return ( + +
    +

    + “{result.activity.name}” has been recorded. +

    + {result.xp_gained > 0 ? ( +

    + +{result.xp_gained} XP awarded +

    + ) : ( +

    + No XP was awarded for this activity. +

    + )} +
    + + +
    +
    +     + ); + } + + return ( + +
    +
    + + Task * + + + {fieldErrors.task && ( +

    {fieldErrors.task}

    + )} +
    + +
    + + Duration * + +
    + setDurationHours(value as string)} + className={styles.durationPartField} + /> + setDurationMinutes((value as string).slice(0, 2))} + maxLength={2} + className={styles.durationPartField} + /> +
    + {fieldErrors.duration && ( +

    {fieldErrors.duration}

    + )} +
    + +
    + Completed +
    + handleCompletionDateChange(value as string)} + max={maxCompletionDate} + /> + handleCompletionTimeChange(value as string)} + /> +
    + {fieldErrors.completedAt && ( +

    {fieldErrors.completedAt}

    + )} +
    + + {!Number.isNaN(totalDurationMinutes) && totalDurationMinutes > 0 && ( +

    + Logging {formatDurationShort(Math.round(totalDurationMinutes * 60))} +

    + )} + + {eligibility && ( +

    + {eligibility.message} +

    + )} + + {submitError && ( +

    {submitError}

    + )} + +
    + + +
    +
    +     + ); +} diff --git a/frontend/src/components/LogOfflineActivityModal/useLogOfflineActivityForm.ts b/frontend/src/components/LogOfflineActivityModal/useLogOfflineActivityForm.ts new file mode 100644 index 00000000..03d24d1d --- /dev/null +++ b/frontend/src/components/LogOfflineActivityModal/useLogOfflineActivityForm.ts @@ -0,0 +1,242 @@ +import { useCallback, useMemo, useState } from "react"; + +import { useLogOfflineActivity } from "../../hooks/useActivities"; +import { useGame } from "../../hooks/useGame"; +import type { SearchEntity } from "../EntitySearchInput/useEntitySearchInput"; +import type { OfflineActivityLogResponse } from "../../types"; + +// Must match OFFLINE_XP_ELIGIBLE_BACKDATE_WINDOW in progression/services.py โ€” +// duplicated here only for the client-side "will this earn XP" hint; the +// backend remains the authority on whether XP is actually awarded. +const XP_ELIGIBLE_BACKDATE_DAYS = 7; + +function pad(n: number): string { + return String(n).padStart(2, "0"); +} + +function todayDateValue(): string { + const now = new Date(); + return `${now.getFullYear()}-${pad(now.getMonth() + 1)}-${pad(now.getDate())}`; +} + +function nowTimeValue(): string { + const now = new Date(); + return `${pad(now.getHours())}:${pad(now.getMinutes())}`; +} + +function extractErrorMessage(error: unknown): string { + if (error instanceof Error && error.message) { + try { + const parsed = JSON.parse(error.message) as Record; + if (typeof parsed.message === "string") return parsed.message; + + const firstFieldError = Object.values(parsed).find( + (value): value is string[] => + Array.isArray(value) && typeof value[0] === "string" + ); + if (firstFieldError) return firstFieldError[0]; + } catch { + // Not a JSON error body โ€” fall back to the raw message below. + } + return error.message; + } + return "Something went wrong logging this activity. Please try again."; +} + +export interface FieldErrors { + task?: string; + duration?: string; + completedAt?: string; +} + +export function useLogOfflineActivityForm(onLogged?: () => void) { + const { player, fetchPlayerAndCharacter } = useGame(); + const isPremium = Boolean(player?.is_premium); + const logOfflineActivity = useLogOfflineActivity(); + + const [taskName, setTaskName] = useState(""); + const [selectedTaskId, setSelectedTaskId] = useState(null); + const [durationHours, setDurationHours] = useState(""); + const [durationMinutes, setDurationMinutes] = useState(""); + const [completionDate, setCompletionDate] = useState(todayDateValue); + const [completionTime, setCompletionTime] = useState(nowTimeValue); + // Fixed at mount: caps the native date picker so future dates can't be + // selected at all, rather than allowing them and erroring afterwards. + const [maxCompletionDate] = useState(todayDateValue); + const [fieldErrors, setFieldErrors] = useState({}); + const [submitError, setSubmitError] = useState(null); + const [result, setResult] = useState(null); + // Read once at mount (a lazy initializer, not a render-phase call) and + // refreshed by the date/time change handlers below โ€” those are event + // handlers, where reading the clock is fine. + const [nowMs, setNowMs] = useState(() => Date.now()); + + const handleCompletionDateChange = useCallback((value: string) => { + setCompletionDate(value); + setNowMs(Date.now()); + }, []); + + const handleCompletionTimeChange = useCallback((value: string) => { + setCompletionTime(value); + setNowMs(Date.now()); + }, []); + + const handleTaskNameChange = useCallback((value: string) => { + setTaskName(value); + setSelectedTaskId(null); + }, []); + + const handleTaskSelect = useCallback((entity: SearchEntity) => { + setTaskName(entity.name); + setSelectedTaskId(typeof entity.id === "number" ? entity.id : Number(entity.id)); + }, []); + + const totalDurationMinutes = useMemo(() => { + const hours = Number(durationHours || 0); + const minutes = Number(durationMinutes || 0); + if (Number.isNaN(hours) || Number.isNaN(minutes)) return NaN; + return hours * 60 + minutes; + }, [durationHours, durationMinutes]); + + const completedAt = useMemo(() => { + if (!completionDate || !completionTime) return null; + const date = new Date(`${completionDate}T${completionTime}`); + return Number.isNaN(date.getTime()) ? null : date; + }, [completionDate, completionTime]); + + const eligibility = useMemo(() => { + if (!completedAt) return null; + + if (!isPremium) { + return { + tone: "info" as const, + message: "This will be recorded, but offline activities only earn XP for Premium accounts.", + }; + } + + const ageDays = (nowMs - completedAt.getTime()) / (24 * 60 * 60 * 1000); + if (ageDays > XP_ELIGIBLE_BACKDATE_DAYS) { + return { + tone: "warning" as const, + message: `This is more than ${XP_ELIGIBLE_BACKDATE_DAYS} days old, so it will be recorded but won't earn XP.`, + }; + } + + return { + tone: "success" as const, + message: "This activity is eligible for XP, subject to your daily logging limits.", + }; + }, [completedAt, isPremium, nowMs]); + + const validate = useCallback((): FieldErrors => { + const errors: FieldErrors = {}; + + if (!taskName.trim()) { + errors.task = "Enter or select a task."; + } + + const hours = durationHours ? Number(durationHours) : 0; + const minutes = durationMinutes ? Number(durationMinutes) : 0; + if (Number.isNaN(hours) || hours < 0) { + errors.duration = "Hours must be a positive number."; + } else if (Number.isNaN(minutes) || minutes < 0 || minutes > 59) { + errors.duration = "Minutes must be between 0 and 59."; + } else if (totalDurationMinutes <= 0) { + errors.duration = "Enter a duration greater than zero."; + } + + if (!completionDate || !completionTime) { + errors.completedAt = "Enter a completion date and time."; + } else if (!completedAt) { + errors.completedAt = "Enter a valid completion date and time."; + } else if (completedAt.getTime() > Date.now()) { + errors.completedAt = "Completion date/time can't be in the future."; + } + + return errors; + }, [ + completedAt, + completionDate, + completionTime, + durationHours, + durationMinutes, + taskName, + totalDurationMinutes, + ]); + + const reset = useCallback(() => { + setTaskName(""); + setSelectedTaskId(null); + setDurationHours(""); + setDurationMinutes(""); + setCompletionDate(todayDateValue()); + setCompletionTime(nowTimeValue()); + setNowMs(Date.now()); + setFieldErrors({}); + setSubmitError(null); + setResult(null); + }, []); + + const handleSubmit = useCallback( + (event: React.FormEvent) => { + event.preventDefault(); + setSubmitError(null); + + const errors = validate(); + setFieldErrors(errors); + if (Object.keys(errors).length > 0 || !completedAt) return; + + const durationSeconds = Math.round(totalDurationMinutes * 60); + const startedAt = new Date(completedAt.getTime() - durationSeconds * 1000); + + logOfflineActivity.mutate( + { + ...(selectedTaskId ? { task: selectedTaskId } : { name: taskName.trim() }), + started_at: startedAt.toISOString(), + completed_at: completedAt.toISOString(), + }, + { + onSuccess: (data) => { + setResult(data); + if (data.xp_gained > 0) fetchPlayerAndCharacter(); + onLogged?.(); + }, + onError: (error) => setSubmitError(extractErrorMessage(error)), + } + ); + }, + [ + completedAt, + fetchPlayerAndCharacter, + logOfflineActivity, + onLogged, + selectedTaskId, + taskName, + totalDurationMinutes, + validate, + ] + ); + + return { + taskName, + handleTaskNameChange, + handleTaskSelect, + durationHours, + setDurationHours, + durationMinutes, + setDurationMinutes, + totalDurationMinutes, + completionDate, + handleCompletionDateChange, + completionTime, + handleCompletionTimeChange, + maxCompletionDate, + fieldErrors, + submitError, + result, + eligibility, + isSubmitting: logOfflineActivity.isPending, + handleSubmit, + reset, + }; +} diff --git a/frontend/src/components/Modal/Modal.module.scss b/frontend/src/components/Modal/Modal.module.scss index 0ca97f77..6022ecb9 100644 --- a/frontend/src/components/Modal/Modal.module.scss +++ b/frontend/src/components/Modal/Modal.module.scss @@ -53,8 +53,7 @@ overflow-y: auto; overflow-x: hidden; min-height: 0; - padding-top: sp.$spacing-md; - padding-bottom: sp.$spacing-md; + padding: sp.$spacing-md sp.$spacing-sm; @include tc.two-column-layout; justify-content: flex-start; diff --git a/frontend/src/hooks/useActivities.ts b/frontend/src/hooks/useActivities.ts index 17f6718c..0e05fc4d 100644 --- a/frontend/src/hooks/useActivities.ts +++ b/frontend/src/hooks/useActivities.ts @@ -1,7 +1,7 @@ // src/hooks/useActivities.ts import { useMutation, useQueryClient, useQuery } from "@tanstack/react-query"; -import { updateActivity, deleteActivity, fetchActivities, createActivity } from "../api/activities"; +import { updateActivity, deleteActivity, fetchActivities, createActivity, logOfflineActivity } from "../api/activities"; import type { PlayerActivity } from "../types"; @@ -26,6 +26,20 @@ export function useCreateActivity() { } +export function useLogOfflineActivity() { + const queryClient = useQueryClient(); + + return useMutation({ + mutationFn: logOfflineActivity, + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ["activities"] }); + // A task may have been auto-created (or its total time updated) by the log. + queryClient.invalidateQueries({ queryKey: ["tasks"] }); + }, + }); +} + + export function useUpdateActivity() { const queryClient = useQueryClient(); diff --git a/frontend/src/types/api.ts b/frontend/src/types/api.ts index cb76f9a5..839d9e45 100644 --- a/frontend/src/types/api.ts +++ b/frontend/src/types/api.ts @@ -97,6 +97,16 @@ export interface AnnouncementReadMutationResponse { unread_count: number; } +/** Response from POST /player-activities/log_offline/ */ +export interface OfflineActivityLogResponse { + success: boolean; + message: string; + activity: PlayerActivity; + xp_gained: number; + xp_eligible_seconds: number; + level_ups: number[]; +} + // Forward references resolved in domain.ts import type { Player } from "./domain"; import type { Character } from "./domain"; @@ -104,4 +114,5 @@ import type { Announcement } from "./domain"; import type { ActivityTimerApiData } from "./timers"; import type { PopulationCentre } from "./domain"; import type { XpModifier } from "./domain"; +import type { PlayerActivity } from "./domain"; import type { LoginState } from "./enums"; diff --git a/frontend/src/types/index.ts b/frontend/src/types/index.ts index 2a2ff69e..35676180 100644 --- a/frontend/src/types/index.ts +++ b/frontend/src/types/index.ts @@ -19,6 +19,7 @@ export type { AnnouncementListResponse, AnnouncementUnreadCountResponse, AnnouncementReadMutationResponse, + OfflineActivityLogResponse, } from "./api"; // Enums and literal union types From 2133d2e03957d9069c95119382e1eb90cb1100b3 Mon Sep 17 00:00:00 2001 From: Duncan Appleby Date: Fri, 14 Aug 2026 10:10:44 +0100 Subject: [PATCH 18/28] feat: parent task chip, due-date split, timestamp tooltip, subtask creation - Display a parent task chip in the task edit modal (with remove/select), and split the due-date input into separate date/time fields (#762). - Move task timestamp display (created/modified/completed) out of the inline summary into a clock-icon tooltip on the edit modal's title row (#764). - Rework "Add a subtask": clicking it now opens the same task edit/detail modal used everywhere else, seeded with a blank, unsaved draft, instead of adding a "Subtask of X" chip before the task input (#774). The draft is only actually created once its name has been genuinely edited (via usePlayerItemModal's existing no-op-edit guard); closing the modal without editing the name discards the draft with no API call. - Add PlayerItemList `hiddenItemIds` (keep an item deep-linkable without rendering it as a row) and `onModalClose` (notify the caller which item's modal just closed) to support the draft-subtask flow. --- .../PlayerItemList/PlayerItemList.tsx | 40 +++- .../TasksPanel/TasksPanel.module.scss | 21 ++ .../components/TasksPanel/TasksPanel.test.tsx | 110 ++++++++- .../src/components/TasksPanel/TasksPanel.tsx | 224 ++++++++++++------ .../components/TasksPanel/useTasksPanel.tsx | 98 +++++++- frontend/src/utils/formatUtils.test.ts | 69 ++++-- frontend/src/utils/formatUtils.ts | 42 +++- 7 files changed, 478 insertions(+), 126 deletions(-) diff --git a/frontend/src/components/PlayerItemList/PlayerItemList.tsx b/frontend/src/components/PlayerItemList/PlayerItemList.tsx index 9d0f2e9c..0617d707 100644 --- a/frontend/src/components/PlayerItemList/PlayerItemList.tsx +++ b/frontend/src/components/PlayerItemList/PlayerItemList.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useMemo } from "react"; +import React, { useCallback, useEffect, useMemo } from "react"; import classNames from "classnames"; import Button from "../Button/Button"; @@ -33,6 +33,8 @@ interface PlayerItemListProps getItemKey?: (item: T, index: number) => string | number; renderItemMeta?: (item: T) => React.ReactNode; renderEditSummary?: (item: T, saveHelpers: SaveStatusHelpers) => React.ReactNode; + /** Rendered next to the name input in the edit modal's title row (e.g. an icon button). */ + renderTitleRowActions?: (item: T) => React.ReactNode; onEdit?: (item: T, name: string, callbacks?: SaveCallbacks) => void; onDelete?: (item: T) => void; hoverEdit?: boolean; @@ -47,6 +49,10 @@ interface PlayerItemListProps /** Called once the requested `openItemId` has been opened, so the caller can clear it. */ onOpenItemHandled?: () => void; getChildren?: (item: T) => T[] | undefined; + /** Ids of items present in `items` (e.g. for the deep-link lookup) that should not be rendered as rows. */ + hiddenItemIds?: Set; + /** Called with the item whose edit modal just closed (via Close, backdrop, or Escape). */ + onModalClose?: (item: T) => void; } export default function PlayerItemList({ @@ -59,6 +65,7 @@ export default function PlayerItemList) { const { activeFilterKey, @@ -125,6 +134,13 @@ export default function PlayerItemList { + if (activeItem) onModalClose?.(activeItem); + handleModalClose(); + }, [activeItem, onModalClose, handleModalClose]); + const canToggleComplete = typeof onToggleComplete === "function"; const canEdit = typeof onEdit === "function"; const canDelete = typeof onDelete === "function"; @@ -140,16 +156,23 @@ export default function PlayerItemList { + if (!hiddenItemIds || hiddenItemIds.size === 0) return displayItems; + return displayItems.filter((item) => item.id === undefined || !hiddenItemIds.has(item.id)); + }, [displayItems, hiddenItemIds]); + // Sort/filter controls only apply to top-level items; a child keeps its // place directly after its parent (in `getChildren`'s order) rather than // being reordered independently. const flatDisplayItems = useMemo(() => { - if (!getChildren) return displayItems; - const topLevel = displayItems.filter( + if (!getChildren) return visibleDisplayItems; + const topLevel = visibleDisplayItems.filter( (item) => item.id === undefined || !childIds.has(item.id) ); return topLevel.flatMap((item) => [item, ...(getChildren(item) ?? [])]); - }, [displayItems, getChildren, childIds]); + }, [visibleDisplayItems, getChildren, childIds]); const renderRow = (item: T): React.ReactNode => ( <> @@ -283,7 +306,7 @@ export default function PlayerItemList setConfirmingDelete(false) : undefined} backLabel="Back" > @@ -328,17 +351,20 @@ export default function PlayerItemList { if (event.key === "Enter") handleEditSave(); - if (event.key === "Escape") handleModalClose(); + if (event.key === "Escape") closeModal(); }} /> ) : null} + {renderTitleRowActions && liveActiveItem + ? renderTitleRowActions(liveActiveItem) + : null}
    ) : null} {modalSummary ? (
    {modalSummary}
    ) : null}
    - {canDelete ? ( diff --git a/frontend/src/components/TasksPanel/TasksPanel.module.scss b/frontend/src/components/TasksPanel/TasksPanel.module.scss index ca842fd1..b38d1f24 100644 --- a/frontend/src/components/TasksPanel/TasksPanel.module.scss +++ b/frontend/src/components/TasksPanel/TasksPanel.module.scss @@ -99,6 +99,27 @@ gap: sp.$spacing-sm; } +.timestampButton { + flex-shrink: 0; + height: sp.$form-control-height; + width: sp.$form-control-height; + padding: 0; + border: 1px solid rgba(c.$color-border-primary, 0.35); + border-radius: sp.$form-control-radius; + background: transparent; + color: inherit; + font-size: 1rem; + line-height: 1; + display: inline-flex; + align-items: center; + justify-content: center; + cursor: pointer; + + &:hover { + border-color: rgba(c.$color-border-primary, 0.55); + } +} + .timestampLabel { font-weight: 600; margin-bottom: 2px; diff --git a/frontend/src/components/TasksPanel/TasksPanel.test.tsx b/frontend/src/components/TasksPanel/TasksPanel.test.tsx index 6ddb5e8f..a503b5cf 100644 --- a/frontend/src/components/TasksPanel/TasksPanel.test.tsx +++ b/frontend/src/components/TasksPanel/TasksPanel.test.tsx @@ -359,7 +359,7 @@ describe("TasksPanel", () => { expect(screen.queryByText("Child subtask")).not.toBeInTheDocument(); }); - it("pre-fills the add-task form with a parent chip via the add-subtask row action", async () => { + it("opens the task detail modal for a blank draft subtask without creating one yet", async () => { const user = userEvent.setup({ pointerEventsCheck: 0 }); mockUseTasks.mockReturnValue({ isLoading: false, @@ -369,15 +369,65 @@ describe("TasksPanel", () => { await user.click(screen.getByRole("button", { name: "Add subtask to Parent project task" })); - expect(screen.getByText(/Subtask of Parent project task/)).toBeInTheDocument(); + const dialog = await screen.findByRole("dialog"); + expect(within(dialog).getByLabelText("task name")).toHaveValue(""); + expect(createMutate).not.toHaveBeenCalled(); + }); + + it("creates the subtask only once its draft name has actually been edited, then opens the persisted task", async () => { + const user = userEvent.setup({ pointerEventsCheck: 0 }); + const newSubtask = { ...childTask, id: 7, name: "New task" }; + createMutate.mockImplementation((_data, callbacks) => { + callbacks?.onSuccess?.(newSubtask); + }); + mockUseTasks.mockReturnValue({ + isLoading: false, + data: [parentTask], + }); + const { rerender } = renderTasksPanel(); + + await user.click(screen.getByRole("button", { name: "Add subtask to Parent project task" })); + const dialog = await screen.findByRole("dialog"); + const input = within(dialog).getByLabelText("task name"); + await user.type(input, "New task"); + await user.tab(); + + expect(createMutate).toHaveBeenCalledWith( + { name: "New task", parent: 3 }, + expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), + ); + + // The new subtask isn't in `items` until the tasks query refetches with it included. + mockUseTasks.mockReturnValue({ + isLoading: false, + data: [parentTask, newSubtask], + }); + rerender( + + + , + ); - const input = screen.getByLabelText("new task"); - await user.type(input, "Buy groceries"); - await user.click(screen.getByRole("button", { name: "Add subtask" })); + const reopenedDialog = await screen.findByRole("dialog"); + expect(within(reopenedDialog).getByDisplayValue("New task")).toBeInTheDocument(); + }); + + it("discards the draft subtask, without creating anything, when its modal is closed unedited", async () => { + const user = userEvent.setup({ pointerEventsCheck: 0 }); + mockUseTasks.mockReturnValue({ + isLoading: false, + data: [parentTask], + }); + renderTasksPanel(); + + await user.click(screen.getByRole("button", { name: "Add subtask to Parent project task" })); + const dialog = await screen.findByRole("dialog"); + await user.click(within(dialog).getByRole("button", { name: "Close" })); await waitFor(() => { - expect(createMutate).toHaveBeenCalledWith({ name: "Buy groceries", parent: 3 }); + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); }); + expect(createMutate).not.toHaveBeenCalled(); }); it("disables the parent picker for a task that already has subtasks", async () => { @@ -406,7 +456,7 @@ describe("TasksPanel", () => { ); const dueDateInput = screen.getByLabelText("Due date"); - await user.type(dueDateInput, "2026-06-01T09:00"); + await user.type(dueDateInput, "2026-06-01"); await user.tab(); await waitFor(() => { @@ -416,5 +466,51 @@ describe("TasksPanel", () => { ); }); }); + + it("defaults the date to today when only a time is set", async () => { + const user = userEvent.setup(); + renderTasksPanel(); + + await user.click( + screen.getAllByRole("button", { name: "Edit task Morning routine" })[0], + ); + + const dueTimeInput = screen.getByLabelText("Due time"); + await user.type(dueTimeInput, "0900"); + await user.tab(); + + await waitFor(() => { + expect(updateMutate).toHaveBeenCalledWith( + { id: 1, data: { due_at: expect.any(String) } }, + expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), + ); + }); + + const lastCall = updateMutate.mock.calls.at(-1) as [{ data: { due_at: string } }, unknown]; + const committedDate = new Date(lastCall[0].data.due_at); + const today = new Date(); + expect(committedDate.getFullYear()).toBe(today.getFullYear()); + expect(committedDate.getMonth()).toBe(today.getMonth()); + expect(committedDate.getDate()).toBe(today.getDate()); + }); + }); + + describe("timestamps tooltip", () => { + it("shows Created/Modified/Completed on click of the clock button", async () => { + const user = userEvent.setup(); + renderTasksPanel(); + + await user.click( + screen.getAllByRole("button", { name: "Edit task Morning routine" })[0], + ); + + expect(screen.queryByText("Created", { selector: "div" })).not.toBeInTheDocument(); + + await user.click(screen.getByRole("button", { name: "View task timestamps" })); + + expect(screen.getByText("Created", { selector: "div" })).toBeInTheDocument(); + expect(screen.getByText("Modified", { selector: "div" })).toBeInTheDocument(); + expect(screen.getByText("Completed", { selector: "div" })).toBeInTheDocument(); + }); }); }); diff --git a/frontend/src/components/TasksPanel/TasksPanel.tsx b/frontend/src/components/TasksPanel/TasksPanel.tsx index 3da46e7a..148ea471 100644 --- a/frontend/src/components/TasksPanel/TasksPanel.tsx +++ b/frontend/src/components/TasksPanel/TasksPanel.tsx @@ -1,4 +1,4 @@ -import React from "react"; +import React, { useRef } from "react"; import classNames from "classnames"; import EntitySearchInput from "../EntitySearchInput/EntitySearchInput"; @@ -6,7 +6,7 @@ import Button from "../Button/Button"; import PlayerItemList from "../PlayerItemList/PlayerItemList"; import Tooltip from "../Tooltip/Tooltip"; import { isTaskComplete, taskSortOptions, useTasksPanel, type ItemRecord } from "./useTasksPanel"; -import { toDatetimeLocalValue, fromDatetimeLocalValue } from "../../utils/formatUtils"; +import { toDateInputValue, toTimeInputValue, fromDateAndTimeInputValues } from "../../utils/formatUtils"; import styles from "./TasksPanel.module.scss"; interface TasksPanelProps { @@ -31,9 +31,11 @@ export default function TasksPanel({ visibleTasks, getChildren, topLevelTasks, - addSubtaskParent, + pendingOpenTaskId, + hiddenItemIds, startAddSubtask, - clearAddSubtaskParent, + clearPendingOpenTaskId, + discardDraftTask, handleCreateTask, handleSubmitForm, handleEdit, @@ -48,35 +50,27 @@ export default function TasksPanel({ updateTask, } = useTasksPanel(openTaskId, onOpenNote); + // Only one task's edit summary is ever open at a time (it renders inside a modal), so a + // single pair of refs is enough to read the sibling input's value when committing due_at. + const dueDateInputRef = useRef(null); + const dueTimeInputRef = useRef(null); + if (isLoading) return

    Loading tasks...

    ; return (
    - {addSubtaskParent && ( - - Subtask of {addSubtaskParent.name} - - - )} setNewName(v)} - onCreate={(name) => handleCreateTask(name, { parent: addSubtaskParent?.id ?? undefined })} - placeholder={addSubtaskParent ? "New subtask name" : "New task name"} + onCreate={(name) => handleCreateTask(name)} + placeholder="New task name" className={styles.addTaskInput} /> @@ -108,27 +102,19 @@ export default function TasksPanel({ ); }} renderEditSummary={(taskItem, saveHelpers) => { + if (taskItem.id < 0) { + // An unsaved draft subtask: nothing to show or edit here yet + // (due date, parent, notes) until it's actually been created. + return
    Type a name to create this subtask.
    ; + } + const summary = getTaskEditSummary(taskItem); const hasSubtasks = (taskItem.subtask_count ?? 0) > 0; const parentOptions = topLevelTasks.filter((t) => t.id !== taskItem.id); + const parentTask = topLevelTasks.find((t) => t.id === taskItem.parent) ?? null; return ( <> -
    -
    -
    Created
    -
    {summary.created}
    -
    -
    -
    Modified
    -
    {summary.modified}
    -
    -
    -
    Completed
    -
    {summary.completed}
    -
    -
    -
    Total time: {summary.totalTime}
    @@ -154,20 +140,51 @@ export default function TasksPanel({ })() ) : null}
    -
    ); }} + renderTitleRowActions={(task) => { + if (task.id < 0) return null; + const summary = getTaskEditSummary(task); + return ( + +
    +
    Created
    +
    {summary.created}
    +
    +
    +
    Modified
    +
    {summary.modified}
    +
    +
    +
    Completed
    +
    {summary.completed}
    +
    +
    + } + > + + + ); + }} hoverEdit renderRowActions={(task) => ( <> @@ -262,8 +335,13 @@ export default function TasksPanel({ )} onEdit={handleEdit} onDelete={handleDelete} - openItemId={openTaskId} - onOpenItemHandled={onOpenTaskHandled} + openItemId={openTaskId ?? pendingOpenTaskId} + onOpenItemHandled={() => { + onOpenTaskHandled?.(); + clearPendingOpenTaskId(); + }} + hiddenItemIds={hiddenItemIds} + onModalClose={discardDraftTask} sortOptions={taskSortOptions} controls={