From fabe2595c7c0dd5b03e5efec2eeba6e7f8ff090e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 00:44:37 +0000 Subject: [PATCH 01/47] Extract beforeunload guard out of useActivityTimer useActivityTimer had one browser-only dependency: a beforeunload listener warning the user before closing the tab mid-activity. Move it into a new useUnloadWarning(active) hook and call it from GameContext, the component that owns the timer, so the timer hook itself has zero window references and stays platform-neutral. Fixes #572 --- frontend/src/context/GameContext.tsx | 3 ++ frontend/src/hooks/useActivityTimer.ts | 8 ---- frontend/src/hooks/useUnloadWarning.test.tsx | 45 ++++++++++++++++++++ frontend/src/hooks/useUnloadWarning.ts | 18 ++++++++ 4 files changed, 66 insertions(+), 8 deletions(-) create mode 100644 frontend/src/hooks/useUnloadWarning.test.tsx create mode 100644 frontend/src/hooks/useUnloadWarning.ts diff --git a/frontend/src/context/GameContext.tsx b/frontend/src/context/GameContext.tsx index 16d15a7b..88a7e5a5 100644 --- a/frontend/src/context/GameContext.tsx +++ b/frontend/src/context/GameContext.tsx @@ -5,6 +5,7 @@ import type { ReactElement, ReactNode } from 'react'; import { useBootstrapGameData } from '../hooks/useBootstrapGameData'; import { apiFetch } from "../utils/api"; import useActivityTimer from '../hooks/useActivityTimer'; +import useUnloadWarning from '../hooks/useUnloadWarning'; import { useAuth } from './AuthContext'; import { GameContext, type GameContextValue } from './gameContext'; import type { @@ -81,6 +82,8 @@ export const GameProvider = ({ children }: ProviderProps): ReactElement => { const activityTimer = useActivityTimer(); const { loadFromServer } = activityTimer; + useUnloadWarning(activityTimer.status === 'active'); + // ---------------------------------------- // STABLE CALLBACKS diff --git a/frontend/src/hooks/useActivityTimer.ts b/frontend/src/hooks/useActivityTimer.ts index f4c501c7..2b51b787 100644 --- a/frontend/src/hooks/useActivityTimer.ts +++ b/frontend/src/hooks/useActivityTimer.ts @@ -333,14 +333,6 @@ export default function useActivityTimer(): ActivityTimerReturn { // ---------------------------- - // Block tab close / refresh / external navigation while timer is active - useEffect(() => { - if (status !== 'active') return; - const handler = (e: BeforeUnloadEvent): void => { e.preventDefault(); e.returnValue = ''; }; - window.addEventListener('beforeunload', handler); - return () => window.removeEventListener('beforeunload', handler); - }, [status]); - // Cleanup on unmount useEffect(() => { //console.log(`[useActivityTimer] mounted`); diff --git a/frontend/src/hooks/useUnloadWarning.test.tsx b/frontend/src/hooks/useUnloadWarning.test.tsx new file mode 100644 index 00000000..ac88f633 --- /dev/null +++ b/frontend/src/hooks/useUnloadWarning.test.tsx @@ -0,0 +1,45 @@ +import { renderHook } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import useUnloadWarning from './useUnloadWarning'; + +describe('useUnloadWarning', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('registers a beforeunload handler while active and removes it on deactivation', () => { + const addSpy = vi.spyOn(window, 'addEventListener'); + const removeSpy = vi.spyOn(window, 'removeEventListener'); + + const { rerender } = renderHook(({ active }) => useUnloadWarning(active), { + initialProps: { active: true }, + }); + + expect(addSpy).toHaveBeenCalledWith('beforeunload', expect.any(Function)); + + rerender({ active: false }); + + expect(removeSpy).toHaveBeenCalledWith('beforeunload', expect.any(Function)); + }); + + it('does not register a handler when inactive', () => { + const addSpy = vi.spyOn(window, 'addEventListener'); + + renderHook(() => useUnloadWarning(false)); + + expect(addSpy).not.toHaveBeenCalledWith('beforeunload', expect.any(Function)); + }); + + it('prevents default and clears returnValue on the beforeunload event', () => { + renderHook(() => useUnloadWarning(true)); + + const event = new Event('beforeunload') as BeforeUnloadEvent; + const preventDefaultSpy = vi.spyOn(event, 'preventDefault'); + + window.dispatchEvent(event); + + expect(preventDefaultSpy).toHaveBeenCalled(); + expect(event.returnValue).toBe(''); + }); +}); diff --git a/frontend/src/hooks/useUnloadWarning.ts b/frontend/src/hooks/useUnloadWarning.ts new file mode 100644 index 00000000..abe82a21 --- /dev/null +++ b/frontend/src/hooks/useUnloadWarning.ts @@ -0,0 +1,18 @@ +// hooks/useUnloadWarning.ts +import { useEffect } from "react"; + +// Warns the user before closing/refreshing/navigating away from the tab +// while `active` is true. No-op when `window` isn't available (e.g. native). +export default function useUnloadWarning(active: boolean): void { + useEffect(() => { + if (!active) return; + if (typeof window === "undefined") return; + + const handler = (e: BeforeUnloadEvent): void => { + e.preventDefault(); + e.returnValue = ""; + }; + window.addEventListener("beforeunload", handler); + return () => window.removeEventListener("beforeunload", handler); + }, [active]); +} From 47edbce8f0586e181443b2becb1ad8d671ff326c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 13:15:22 +0000 Subject: [PATCH 02/47] Move DOM event handling out of useEntitySearchInput into its component useEntitySearchInput reached into the DOM directly for click-outside dismissal and typed its keydown handler against KeyboardEvent. This splits it along the platform boundary, per #573: - Hook now exposes intent-shaped actions (onSelectNext, onSelectPrevious, onDismiss, onCommit) instead of a DOM-typed handleKeyDown. - The document mousedown listener for click-outside dismissal moves to EntitySearchInput.tsx, which owns the rootRef and translates the gesture into onDismiss(). - EntitySearchInput.tsx's handleKeyDown translates raw KeyboardEvents into the hook's semantic actions. - window.setTimeout/window.clearTimeout -> bare setTimeout/clearTimeout. Fixes #573 --- .../EntitySearchInput/EntitySearchInput.tsx | 48 +++++- .../EntitySearchInput/useEntitySearchInput.ts | 148 +++++++----------- 2 files changed, 103 insertions(+), 93 deletions(-) diff --git a/frontend/src/components/EntitySearchInput/EntitySearchInput.tsx b/frontend/src/components/EntitySearchInput/EntitySearchInput.tsx index 29db7906..9feef0b4 100644 --- a/frontend/src/components/EntitySearchInput/EntitySearchInput.tsx +++ b/frontend/src/components/EntitySearchInput/EntitySearchInput.tsx @@ -1,4 +1,5 @@ -import type { CSSProperties } from "react"; +import { useEffect, useRef } from "react"; +import type { CSSProperties, KeyboardEvent } from "react"; import classNames from "classnames"; import { useEntitySearchInput, type SearchEntity } from "./useEntitySearchInput"; @@ -44,7 +45,6 @@ export default function EntitySearchInput({ emptyMessage, }: EntitySearchInputProps) { const { - rootRef, canSearch, taskItems, activityItems, @@ -53,8 +53,11 @@ export default function EntitySearchInput({ activeHighlightedIndex, handleInputFocus, handleInputChange, - handleKeyDown, commitSelection, + onSelectNext, + onSelectPrevious, + onDismiss, + onCommit, } = useEntitySearchInput({ type, value, @@ -68,6 +71,45 @@ export default function EntitySearchInput({ maxVisibleRows, }); + const rootRef = useRef(null); + + // Dismiss on outside click — the hook exposes the semantic action, this + // component owns the DOM listener that detects the gesture. + useEffect(() => { + function handleClickOutside(event: MouseEvent) { + if (!rootRef.current?.contains(event.target as Node)) { + onDismiss(); + } + } + + document.addEventListener("mousedown", handleClickOutside); + return () => document.removeEventListener("mousedown", handleClickOutside); + }, [onDismiss]); + + const handleKeyDown = (event: KeyboardEvent) => { + if (disabled) return; + + switch (event.key) { + case "ArrowDown": + if (isDropdownOpen) event.preventDefault(); + onSelectNext(); + return; + case "ArrowUp": + if (isDropdownOpen) event.preventDefault(); + onSelectPrevious(); + return; + case "Escape": + if (isDropdownOpen) event.preventDefault(); + onDismiss(); + return; + case "Enter": + if (onCommit()) event.preventDefault(); + return; + default: + return; + } + }; + const renderOption = (entity: SearchEntity, index: number) => { const isHighlighted = index === activeHighlightedIndex; return ( diff --git a/frontend/src/components/EntitySearchInput/useEntitySearchInput.ts b/frontend/src/components/EntitySearchInput/useEntitySearchInput.ts index 13b146bf..7856b952 100644 --- a/frontend/src/components/EntitySearchInput/useEntitySearchInput.ts +++ b/frontend/src/components/EntitySearchInput/useEntitySearchInput.ts @@ -1,5 +1,4 @@ -import { useCallback, useEffect, useMemo, useRef, useState } from "react"; -import type { KeyboardEvent } from "react"; +import { useCallback, useEffect, useMemo, useState } from "react"; import Fuse from "fuse.js"; import { useEntitySearchCache } from "../../hooks/useEntitySearchCache"; @@ -63,11 +62,11 @@ function useEntitySearchResults({ ); useEffect(() => { - const timeoutId = window.setTimeout(() => { + const timeoutId = setTimeout(() => { setDebouncedQuery(value); }, DEBOUNCE_MS); - return () => window.clearTimeout(timeoutId); + return () => clearTimeout(timeoutId); }, [value]); const fuse = useMemo( @@ -162,27 +161,21 @@ function useEntitySearchResults({ }; } -function useEntitySearchDropdown(rootRef: React.RefObject) { +function useEntitySearchDropdown() { const [isFocused, setIsFocused] = useState(false); const [highlightedIndex, setHighlightedIndex] = useState(-1); - useEffect(() => { - function handleClickOutside(event: MouseEvent) { - if (!rootRef.current?.contains(event.target as Node)) { - setIsFocused(false); - setHighlightedIndex(-1); - } - } - - document.addEventListener("mousedown", handleClickOutside); - return () => document.removeEventListener("mousedown", handleClickOutside); - }, [rootRef]); + const dismiss = useCallback(() => { + setIsFocused(false); + setHighlightedIndex(-1); + }, []); return { isFocused, setIsFocused, highlightedIndex, setHighlightedIndex, + dismiss, }; } @@ -198,13 +191,12 @@ export function useEntitySearchInput({ alwaysOpen = false, maxVisibleRows, }: UseEntitySearchInputProps) { - const rootRef = useRef(null); const { entities, addEntityToCache } = useEntitySearchCache(type); const canSearch = searchEnabled && !disabled; - const { isFocused, setIsFocused, highlightedIndex, setHighlightedIndex } = - useEntitySearchDropdown(rootRef); + const { isFocused, setIsFocused, highlightedIndex, setHighlightedIndex, dismiss } = + useEntitySearchDropdown(); const { results, taskItems, activityItems } = useEntitySearchResults({ entities, @@ -225,10 +217,9 @@ export function useEntitySearchInput({ (entity: SearchEntity) => { onChange?.(entity.name); onSelect?.(entity); - setIsFocused(false); - setHighlightedIndex(-1); + dismiss(); }, - [onChange, onSelect, setHighlightedIndex, setIsFocused] + [onChange, onSelect, dismiss] ); const commitCreate = useCallback(async () => { @@ -238,9 +229,8 @@ export function useEntitySearchInput({ addEntityToCache(nextName); onChange?.(nextName); await onCreate?.(nextName); - setIsFocused(false); - setHighlightedIndex(-1); - }, [addEntityToCache, onChange, onCreate, setHighlightedIndex, setIsFocused, value]); + dismiss(); + }, [addEntityToCache, onChange, onCreate, dismiss, value]); const handleInputFocus = useCallback(() => { setIsFocused(true); @@ -253,73 +243,49 @@ export function useEntitySearchInput({ [onChange] ); - const handleKeyDown = useCallback( - async (event: KeyboardEvent) => { - if (disabled) return; - - if (event.key === "ArrowDown" && isDropdownOpen) { - event.preventDefault(); - setHighlightedIndex( - activeHighlightedIndex < results.length - 1 ? activeHighlightedIndex + 1 : 0 - ); - return; - } - - if (event.key === "ArrowUp" && isDropdownOpen) { - event.preventDefault(); - setHighlightedIndex( - activeHighlightedIndex > 0 ? activeHighlightedIndex - 1 : results.length - 1 - ); - return; - } - - if (event.key === "Escape") { - if (isDropdownOpen) { - event.preventDefault(); - } - setIsFocused(false); - setHighlightedIndex(-1); - return; - } - - if (event.key !== "Enter") return; - - if (!canSearch) { - return; - } - - const hasHighlightedResult = - isDropdownOpen && - activeHighlightedIndex >= 0 && - activeHighlightedIndex < results.length; + /** Move the highlight to the next result, wrapping to the top. No-op while closed. */ + const onSelectNext = useCallback(() => { + if (!isDropdownOpen) return; + setHighlightedIndex(activeHighlightedIndex < results.length - 1 ? activeHighlightedIndex + 1 : 0); + }, [activeHighlightedIndex, isDropdownOpen, results.length, setHighlightedIndex]); + + /** Move the highlight to the previous result, wrapping to the bottom. No-op while closed. */ + const onSelectPrevious = useCallback(() => { + if (!isDropdownOpen) return; + setHighlightedIndex(activeHighlightedIndex > 0 ? activeHighlightedIndex - 1 : results.length - 1); + }, [activeHighlightedIndex, isDropdownOpen, results.length, setHighlightedIndex]); + + /** Close the dropdown and clear the highlight, e.g. on Escape or an outside click/tap. */ + const onDismiss = useCallback(() => { + dismiss(); + }, [dismiss]); + + /** + * Commit the highlighted result, or create a new entity from the typed + * value when nothing is highlighted. Returns whether it took action, so + * callers translating a "commit" gesture (e.g. Enter) know whether to + * suppress its default behaviour. + */ + const onCommit = useCallback(() => { + if (!canSearch) return false; + + const hasHighlightedResult = + isDropdownOpen && activeHighlightedIndex >= 0 && activeHighlightedIndex < results.length; + + if (hasHighlightedResult) { + commitSelection(results[activeHighlightedIndex]); + return true; + } - if (hasHighlightedResult) { - event.preventDefault(); - commitSelection(results[activeHighlightedIndex]); - return; - } + if (normalizeQuery(value)) { + void commitCreate(); + return true; + } - if (normalizeQuery(value)) { - event.preventDefault(); - await commitCreate(); - } - }, - [ - activeHighlightedIndex, - canSearch, - commitCreate, - commitSelection, - disabled, - isDropdownOpen, - results, - setHighlightedIndex, - setIsFocused, - value, - ] - ); + return false; + }, [activeHighlightedIndex, canSearch, commitCreate, commitSelection, isDropdownOpen, results, value]); return { - rootRef, canSearch, results, taskItems, @@ -327,10 +293,12 @@ export function useEntitySearchInput({ showGroupLabels, isDropdownOpen, activeHighlightedIndex, - highlightedIndex, handleInputFocus, handleInputChange, - handleKeyDown, commitSelection, + onSelectNext, + onSelectPrevious, + onDismiss, + onCommit, }; } From 1c8a880b854ebed659f8c6971714f0edfd8c4e9b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 13:38:12 +0000 Subject: [PATCH 03/47] Extract XP/multiplier reward breakdown out of ActivityRewardScreen into a pure util MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ActivityRewardScreen computed the post-activity reward breakdown — premium bonus, task bonus, total XP, and the four summary-line phrasings — inline in its render path. Extracts that into a pure buildActivityRewardBreakdown() function in utils/, matching the existing buildActivityRewardToastMessage precedent. - Added frontend/src/utils/activityRewardBreakdown.ts: takes the raw reward fields (activityName, xpGained, baseXp, xpMultiplier, taskXpMultiplier, levelUps, elapsedSeconds) and returns the breakdown rows, the premium-multiplier gate (isLikelyPremiumUser), and the assembled summary line. No React, no side effects. - ActivityRewardScreen.tsx now calls the util and just renders the result; the inline calc block is gone. - Added activityRewardBreakdown.test.ts covering zero XP, no multipliers, premium-only, task-only, both multipliers, all four summary phrasings, level-up normalization, and missing XP. - Deliberately not merged with buildActivityRewardToastMessage — documented why in the new file's doc comment (different output shapes, mostly-disjoint inputs). Fixes #575 --- .../screens/ActivityRewardScreen.tsx | 78 +++-------- .../src/utils/activityRewardBreakdown.test.ts | 120 ++++++++++++++++ frontend/src/utils/activityRewardBreakdown.ts | 131 ++++++++++++++++++ 3 files changed, 272 insertions(+), 57 deletions(-) create mode 100644 frontend/src/utils/activityRewardBreakdown.test.ts create mode 100644 frontend/src/utils/activityRewardBreakdown.ts diff --git a/frontend/src/components/SupportFlow/screens/ActivityRewardScreen.tsx b/frontend/src/components/SupportFlow/screens/ActivityRewardScreen.tsx index 102f18f2..344aa84e 100644 --- a/frontend/src/components/SupportFlow/screens/ActivityRewardScreen.tsx +++ b/frontend/src/components/SupportFlow/screens/ActivityRewardScreen.tsx @@ -2,7 +2,8 @@ import React, { useEffect, useRef, useState } from "react"; import Button from "../../Button/Button"; import ButtonFrame from "../../Button/ButtonFrame"; -import { formatDuration, formatRewardDuration } from "../../../utils/formatUtils"; +import { formatRewardDuration } from "../../../utils/formatUtils"; +import { buildActivityRewardBreakdown } from "../../../utils/activityRewardBreakdown"; import { useTasks, useUpdateTask } from "../../../hooks/useTasks"; import { useGame } from "../../../hooks/useGame"; import styles from "../SupportFlowModal.module.scss"; @@ -98,70 +99,33 @@ export default function ActivityRewardScreen({ const continueButtonLabel = shouldEnableCountdown ? `Continue with support in ${countdownSeconds}..` : "Continue with support"; - const hasActivityName = typeof activityName === "string" && activityName.trim(); - const parsedXp = Number(xpGained); - const hasXp = Number.isFinite(parsedXp); - const parsedBaseXp = Number(baseXp); - const parsedMultiplier = Number(xpMultiplier); - const hasRewardBreakdown = - Number.isFinite(parsedBaseXp) && - parsedBaseXp >= 0 && - Number.isFinite(parsedMultiplier) && - parsedMultiplier > 0 && - hasXp; - const parsedElapsedSeconds = Number(elapsedSeconds); - const hasElapsedSeconds = - Number.isFinite(parsedElapsedSeconds) && parsedElapsedSeconds >= 0; - const formattedElapsed = hasElapsedSeconds - ? formatRewardDuration(parsedElapsedSeconds) - : null; - const condensedElapsed = hasElapsedSeconds - ? formatDuration(parsedElapsedSeconds) - : null; - const parsedTaskXpMultiplier = Number(taskXpMultiplier); - const hasTaskBonus = - Number.isFinite(parsedTaskXpMultiplier) && parsedTaskXpMultiplier > 1; - // Infer premium component: combined / task (or combined if no task bonus) - const premiumMultiplier = - hasRewardBreakdown && hasTaskBonus && parsedTaskXpMultiplier > 0 - ? parsedMultiplier / parsedTaskXpMultiplier - : parsedMultiplier; - function fmtMult(m: number): string { - return Number.isInteger(m) ? String(m) : m.toFixed(2).replace(/\.?0+$/, ""); - } + const { + hasXp, + xpGained: parsedXp, + hasElapsedSeconds, + condensedElapsed, + rewardSummaryLine, + multiplierLines, + isLikelyPremiumUser, + normalizedLevelUps, + hasActivityName, + } = buildActivityRewardBreakdown({ + activityName, + xpGained, + baseXp, + xpMultiplier, + taskXpMultiplier, + levelUps, + elapsedSeconds, + }); - const normalizedLevelUps = Array.isArray(levelUps) - ? levelUps - .map((level) => Number(level)) - .filter((level) => Number.isInteger(level) && level > 0) - : []; - const isLikelyPremiumUser = premiumMultiplier >= 2; const shouldShowUpgradePrompt = Boolean(showUpgradePrompt) && !isLikelyPremiumUser; const upgradeMessage = shouldShowUpgradePrompt ? isAutoStopped ? "Need more time? Upgrade to Premium for unlimited timer sessions." : "Want even more rewards? Upgrade to Premium for double XP on activities." : null; - const multiplierLines: Array<{ label: string; value: string }> = []; - let rewardSummaryLine = "Nice work ⚔️ You completed an activity."; - - if (formattedElapsed && hasActivityName) { - rewardSummaryLine = `Nice work ⚔️ You spent ${formattedElapsed} on "${activityName!.trim()}".`; - } else if (hasActivityName) { - rewardSummaryLine = `Nice work ⚔️ You completed "${activityName!.trim()}".`; - } else if (formattedElapsed) { - rewardSummaryLine = `Nice work ⚔️ You spent ${formattedElapsed} focused.`; - } - - if (hasRewardBreakdown) { - if (premiumMultiplier > 1) { - multiplierLines.push({ label: "Premium bonus", value: `x${fmtMult(premiumMultiplier)}` }); - } - if (hasTaskBonus) { - multiplierLines.push({ label: "Task bonus", value: `x${fmtMult(parsedTaskXpMultiplier)}` }); - } - } return (
diff --git a/frontend/src/utils/activityRewardBreakdown.test.ts b/frontend/src/utils/activityRewardBreakdown.test.ts new file mode 100644 index 00000000..a9f613fa --- /dev/null +++ b/frontend/src/utils/activityRewardBreakdown.test.ts @@ -0,0 +1,120 @@ +import { describe, expect, it } from "vitest"; +import { buildActivityRewardBreakdown } from "./activityRewardBreakdown"; + +describe("buildActivityRewardBreakdown", () => { + it("renders a zero-XP completion without crashing and with an empty multiplier list", () => { + const breakdown = buildActivityRewardBreakdown({ + activityName: "Write tests", + xpGained: 0, + baseXp: 0, + xpMultiplier: 1, + elapsedSeconds: 60, + }); + + expect(breakdown.hasXp).toBe(true); + expect(breakdown.xpGained).toBe(0); + expect(breakdown.multiplierLines).toEqual([]); + expect(breakdown.isLikelyPremiumUser).toBe(false); + }); + + it("has no multiplier lines when neither premium nor task bonus applies", () => { + const breakdown = buildActivityRewardBreakdown({ + xpGained: 27, + baseXp: 27, + xpMultiplier: 1, + }); + + expect(breakdown.multiplierLines).toEqual([]); + expect(breakdown.isLikelyPremiumUser).toBe(false); + }); + + it("shows only the premium bonus when there is no task multiplier", () => { + const breakdown = buildActivityRewardBreakdown({ + xpGained: 54, + baseXp: 27, + xpMultiplier: 2, + }); + + expect(breakdown.multiplierLines).toEqual([{ label: "Premium bonus", value: "x2" }]); + expect(breakdown.isLikelyPremiumUser).toBe(true); + }); + + it("shows only the task bonus when the combined multiplier is entirely the task bonus", () => { + const breakdown = buildActivityRewardBreakdown({ + xpGained: 40, + baseXp: 27, + xpMultiplier: 1.5, + taskXpMultiplier: 1.5, + }); + + expect(breakdown.multiplierLines).toEqual([{ label: "Task bonus", value: "x1.5" }]); + expect(breakdown.isLikelyPremiumUser).toBe(false); + }); + + it("shows both bonuses and infers the premium component from the combined multiplier", () => { + const breakdown = buildActivityRewardBreakdown({ + xpGained: 81, + baseXp: 27, + xpMultiplier: 3, + taskXpMultiplier: 1.5, + }); + + expect(breakdown.multiplierLines).toEqual([ + { label: "Premium bonus", value: "x2" }, + { label: "Task bonus", value: "x1.5" }, + ]); + expect(breakdown.isLikelyPremiumUser).toBe(true); + }); + + it("uses the 'spent
); From f6cfd9e8e40ab926b43b2fdb73571cb22be07154 Mon Sep 17 00:00:00 2001 From: Duncan Appleby Date: Sun, 9 Aug 2026 22:58:01 +0100 Subject: [PATCH 44/47] Keep body/heading/button/link/caption text at their mobile sizes above 768px Most body text read as too large on wider viewports; drop the md/lg font-size escalation from the shared type scale so text stays the same size at every breakpoint instead of growing past 768px. --- frontend/src/styles/semantic/_typography.scss | 36 ------------------- 1 file changed, 36 deletions(-) diff --git a/frontend/src/styles/semantic/_typography.scss b/frontend/src/styles/semantic/_typography.scss index 9ff46ca1..595cf4b5 100644 --- a/frontend/src/styles/semantic/_typography.scss +++ b/frontend/src/styles/semantic/_typography.scss @@ -19,12 +19,6 @@ $text-body: ( letter-spacing: token(t.$letter-spacing, normal), text-transform: token(t.$text-transform, none) ), - md: ( - font-size: 1.125rem - ), - lg: ( - //font-size: 1.25rem - ), ); // Body text styles @@ -37,9 +31,6 @@ $text-list: ( letter-spacing: token(t.$letter-spacing, normal), text-transform: token(t.$text-transform, none) ), - md: ( - font-size: token(t.$font-size, base) - ) ); // Heading 1 styles @@ -52,12 +43,6 @@ $text-heading-1: ( letter-spacing: token(t.$letter-spacing, tight), text-transform: token(t.$text-transform, none) ), - md: ( - font-size: 2.25rem - ), - lg: ( - font-size: 2.5rem - ) ); // Heading 2 styles @@ -70,12 +55,6 @@ $text-heading-2: ( letter-spacing: token(t.$letter-spacing, tight), text-transform: token(t.$text-transform, none) ), - md: ( - font-size: 1.75rem - ), - lg: ( - font-size: 2rem - ) ); $text-heading-3: ( @@ -87,12 +66,6 @@ $text-heading-3: ( letter-spacing: token(t.$letter-spacing, normal), text-transform: token(t.$text-transform, none) ), - md: ( - font-size: 1.5rem - ), - lg: ( - font-size: 1.75rem - ) ); @@ -106,9 +79,6 @@ $text-button: ( letter-spacing: token(t.$letter-spacing, normal), text-transform: token(t.$text-transform, none) ), - md: ( - font-size: 1.125rem - ) ); // Link styles @@ -121,9 +91,6 @@ $text-link : ( letter-spacing: token(t.$letter-spacing, normal), text-transform: token(t.$text-transform, none), ), - md: ( - font-size: 1.125rem - ) ); // Caption styles @@ -136,9 +103,6 @@ $text-caption: ( letter-spacing: token(t.$letter-spacing, normal), text-transform: token(t.$text-transform, none), ), - md: ( - font-size: 0.75rem - ), ); // Label styles From b1dffbe0bdee39e7397ce4043db8ea6b61cc941c Mon Sep 17 00:00:00 2001 From: Duncan Appleby Date: Sun, 9 Aug 2026 23:25:18 +0100 Subject: [PATCH 45/47] Dock the map detail panel inside the map and add list hover feedback Extracts MapDetailCard from DetailCard so the docked panel portals into Map's own wrapper (via a new DetailSurface `container` prop) and positions relative to the map instead of the viewport, fixing it sitting far too high on the page. It stays docked top-right through a wider range of narrow viewports before falling back to the mobile bottom sheet, and sits closer to the zoom controls. DetailCard itself is now always a centered modal. Also gives the character/building lists in the detail card a stronger hover effect (List's existing background/border hover rule was nearly invisible). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_011H8vdMQa7b5tHiz32seC6v --- .../BuildingDetail/BuildingDetail.tsx | 2 + .../CharacterDetail/CharacterDetail.tsx | 1 + .../DetailCard/DetailCard.module.scss | 22 ----- .../DetailCard/DetailCard.stories.tsx | 30 ------ .../src/components/DetailCard/DetailCard.tsx | 96 ++++++++++++------- .../DetailSurface/DetailSurface.tsx | 8 +- frontend/src/components/List/List.module.scss | 6 +- frontend/src/components/Map/Map.tsx | 21 ++-- .../MapDetailCard/MapDetailCard.module.scss | 48 ++++++++++ .../MapDetailCard/MapDetailCard.stories.tsx | 52 ++++++++++ .../MapDetailCard/MapDetailCard.tsx | 47 +++++++++ 11 files changed, 233 insertions(+), 100 deletions(-) create mode 100644 frontend/src/components/MapDetailCard/MapDetailCard.module.scss create mode 100644 frontend/src/components/MapDetailCard/MapDetailCard.stories.tsx create mode 100644 frontend/src/components/MapDetailCard/MapDetailCard.tsx diff --git a/frontend/src/components/BuildingDetail/BuildingDetail.tsx b/frontend/src/components/BuildingDetail/BuildingDetail.tsx index 15a9264f..c14c6aef 100644 --- a/frontend/src/components/BuildingDetail/BuildingDetail.tsx +++ b/frontend/src/components/BuildingDetail/BuildingDetail.tsx @@ -84,6 +84,7 @@ export default function BuildingDetail({ className={styles.residentsList} compact canSelect={Boolean(onSelectResident)} + canHover={Boolean(onSelectResident)} onSelect={(resident) => onSelectResident?.(resident.id)} renderItem={(resident) => {residentLine(resident)}} /> @@ -102,6 +103,7 @@ export default function BuildingDetail({ className={styles.residentsList} compact canSelect={Boolean(onSelectWorker)} + canHover={Boolean(onSelectWorker)} onSelect={(worker) => onSelectWorker?.(worker.id)} renderItem={(worker) => {residentLine(worker)}} /> diff --git a/frontend/src/components/CharacterDetail/CharacterDetail.tsx b/frontend/src/components/CharacterDetail/CharacterDetail.tsx index 979d2b53..76ef7151 100644 --- a/frontend/src/components/CharacterDetail/CharacterDetail.tsx +++ b/frontend/src/components/CharacterDetail/CharacterDetail.tsx @@ -102,6 +102,7 @@ export default function CharacterDetail({ items={relationshipItems} className={styles.relationshipsList} canSelect={Boolean(onSelectRelationship)} + canHover={Boolean(onSelectRelationship)} onSelect={(relationship) => onSelectRelationship?.(relationship.character_id)} renderItem={(relationship) => ( diff --git a/frontend/src/components/DetailCard/DetailCard.module.scss b/frontend/src/components/DetailCard/DetailCard.module.scss index 33d26537..399bcc2c 100644 --- a/frontend/src/components/DetailCard/DetailCard.module.scss +++ b/frontend/src/components/DetailCard/DetailCard.module.scss @@ -36,28 +36,6 @@ } } -// A portrait side panel docked to the right edge, instead of a -// floating centered card - see the `placement` prop comment in -// DetailCard.tsx for why (Map covers itself otherwise). Only takes effect at -// `sm` and up; below that it's still the full-width bottom sheet, where a -// side panel has no room to mean anything. -.right { - @include m.respond-to(sm) { - left: auto; - right: 0; - top: 50%; - bottom: auto; - transform: translateY(-50%); - width: 90%; - max-width: min(90vw, 200px); - max-height: 75vh; - // No dimming overlay behind this variant (it's non-modal - see the - // `modal` prop comment in DetailSurface.tsx), so a border is the only - // thing separating it from the map underneath. - border: 1px solid c.$color-border-primary; - } -} - .header { display: flex; align-items: center; diff --git a/frontend/src/components/DetailCard/DetailCard.stories.tsx b/frontend/src/components/DetailCard/DetailCard.stories.tsx index f4d47e7d..282c958e 100644 --- a/frontend/src/components/DetailCard/DetailCard.stories.tsx +++ b/frontend/src/components/DetailCard/DetailCard.stories.tsx @@ -57,33 +57,3 @@ export const WithLongerContent: Story = { ), }, }; - -/** - * `placement="right"` - the docked side-panel variant Map uses, so a click - * on a character/building stays open and browsable alongside the map - * instead of covering it. Non-modal (no dimming overlay, map underneath - * stays interactive), narrower, capped at 75vh with its own scroll. - */ -export const RightPlacement: Story = { - args: { - placement: 'right', - title: 'Rose Cottage', - children: ( - <> -

House

-

Residents: 4

-
    -
  • Alice (idle)
    • -
  • Thomas (delivering goods to neighbours)
    • -
  • Emily (idle)
    • -
  • James (idle)
    • -
- - ), - }, - play: async ({ canvasElement }) => { - const body = within(canvasElement.ownerDocument.body); - const dialog = await body.findByRole('dialog', { name: 'Rose Cottage' }); - await expect(dialog).toBeVisible(); - }, -}; diff --git a/frontend/src/components/DetailCard/DetailCard.tsx b/frontend/src/components/DetailCard/DetailCard.tsx index 5c07d5db..8afc8026 100644 --- a/frontend/src/components/DetailCard/DetailCard.tsx +++ b/frontend/src/components/DetailCard/DetailCard.tsx @@ -23,6 +23,58 @@ function TargetIcon() { ); } +interface DetailCardBodyProps { + title: string; + onClose: () => void; + children: React.ReactNode; + className: string; + // Omit to render the header with no fly-to affordance (e.g. entities + // with no map position to fly to yet). + onFlyTo?: () => void; +} + +// Header/content layout shared by DetailCard (centered modal) and Map's +// MapDetailCard (docked panel, positioned relative to the map itself) - only +// the outer wrapper's positioning differs between the two, so this owns +// everything else: title, fly-to/close buttons, scrollable content area. +export function DetailCardBody({ + title, + onClose, + children, + className, + onFlyTo, +}: DetailCardBodyProps) { + return ( +
+
+ {/* Plain text, not a heading element - DetailSurface already + renders an sr-only

with the same text as the dialog's + accessible name/heading; a second real heading here would + duplicate it for screen reader users navigating by heading. */} +
{title}
+ {onFlyTo && ( + +

+
{children}
+
+ ); +} + // Reusable, entity-agnostic detail-card shell (see Map's "click a tooltip to // open a richer detail card" flow) - provides the header/title/close/content // layout every entity type shares; CharacterDetail/BuildingDetail (and later @@ -30,18 +82,16 @@ function TargetIcon() { // Deliberately has no Radix/Tamagui import of its own - DetailSurface is the // only piece of this feature that talks to a UI library primitive, so // swapping it out later doesn't touch this file or its callers. +// Always a centered floating modal (matching Modal's own layout), falling +// back to the same full-width bottom sheet on mobile. Map's docked side +// panel is MapDetailCard, not a variant of this component - it needs +// different positioning (relative to the map, not the viewport) and portal +// target, not just different CSS. interface DetailCardProps { open: boolean; title: string; onClose: () => void; children: React.ReactNode; - // "center" (default) matches Modal's floating-card layout; "right" docks - // the card to the viewport's right edge instead (e.g. Map, where a - // centered card would sit on top of the very content it describes). - // Both still fall back to the same full-width bottom sheet on mobile. - placement?: "center" | "right"; - // Omit to render the header with no fly-to affordance (e.g. entities - // with no map position to fly to yet). onFlyTo?: () => void; } @@ -50,7 +100,6 @@ export default function DetailCard({ title, onClose, children, - placement = "center", onFlyTo, }: DetailCardProps) { return ( @@ -60,35 +109,10 @@ export default function DetailCard({ if (!next) onClose(); }} title={title} - modal={placement !== "right"} > -
-
- {/* Plain text, not a heading element - DetailSurface already - renders an sr-only

with the same text as the dialog's - accessible name/heading; a second real heading here would - duplicate it for screen reader users navigating by heading. */} -
{title}
- {onFlyTo && ( - -

-
{children}
-
+ + {children} + ); } diff --git a/frontend/src/components/DetailSurface/DetailSurface.tsx b/frontend/src/components/DetailSurface/DetailSurface.tsx index 657507a2..2af53ed9 100644 --- a/frontend/src/components/DetailSurface/DetailSurface.tsx +++ b/frontend/src/components/DetailSurface/DetailSurface.tsx @@ -23,6 +23,11 @@ interface DetailSurfaceProps { * Map) stays fully interactive - for a docked side panel, which is meant * to keep browsing alongside rather than block it. */ modal?: boolean; + /** DOM node to portal into instead of document.body (Radix's default) - + * e.g. Map's own wrapper element, so a non-modal docked panel (see + * MapDetailCard) positions relative to the map rather than the + * viewport. */ + container?: HTMLElement | null; } export default function DetailSurface({ @@ -31,10 +36,11 @@ export default function DetailSurface({ title, children, modal = true, + container, }: DetailSurfaceProps) { return ( - + {modal && } (null); + // Passed to MapDetailCard as DetailSurface's portal `container` so the + // docked detail panel positions relative to the map itself, not the + // viewport (see MapDetailCard.module.scss). State (not a plain ref) so + // the value is available for render once the wrapper mounts. + const [mapWrapperEl, setMapWrapperEl] = useState(null); const mapRef = useRef(null); const sourceRef = useRef(null); const [mapReady, setMapReady] = useState(false); @@ -825,29 +830,28 @@ export default function PopulationCentreMap({ }, [detail, characterFeatures, idleCharacterPositions, selectedBuildingFeature]); return ( -
+
{children &&
{children}
} {detail?.type === "character" && ( - setDetail(null)} onFlyTo={handleFlyToDetail} + container={mapWrapperEl} > openDetail({ type: "building", id: buildingId })} onSelectRelationship={(characterId) => openDetail({ type: "character", id: characterId })} /> - + )} {detail?.type === "building" && selectedBuildingFeature && ( - setDetail(null)} onFlyTo={handleFlyToDetail} + container={mapWrapperEl} > openDetail({ type: "character", id: characterId })} onSelectWorker={(characterId) => openDetail({ type: "character", id: characterId })} /> - + )}
); diff --git a/frontend/src/components/MapDetailCard/MapDetailCard.module.scss b/frontend/src/components/MapDetailCard/MapDetailCard.module.scss new file mode 100644 index 00000000..95b8ac83 --- /dev/null +++ b/frontend/src/components/MapDetailCard/MapDetailCard.module.scss @@ -0,0 +1,48 @@ +// components/MapDetailCard/MapDetailCard.module.scss +@use '../../styles/base/variables' as v; +@use '../../styles/semantic/spacing' as sp; +@use '../../styles/semantic/colors' as c; +@use '../../styles/utilities/mixins' as m; + +// Mobile-first: full-width bottom sheet fallback (same as DetailCard's +// centered variant - the map fills the screen at that size, so +// viewport-fixed is fine there too), but only below a custom, narrower +// breakpoint than the shared `sm` (576px) token - a docked side panel still +// has room to mean something down to a fairly small phone width, so this +// keeps the top-right dock through more of the "mobile" range and reserves +// the bottom sheet for genuinely narrow screens. +$dock-breakpoint: 420px; + +.card { + position: fixed; + z-index: v.$z-index-modal; + display: flex; + flex-direction: column; + background: c.$color-bg; + box-shadow: 0 sp.$spacing-sm sp.$spacing-md rgba(0, 0, 0, 0.2); + + left: 0; + right: 0; + bottom: 0; + max-height: 80vh; + border-radius: sp.$border-radius sp.$border-radius 0 0; + + @include m.respond-to($dock-breakpoint) { + position: absolute; + left: auto; + // Clear of MapLibre's own NavigationControl (zoom buttons, ~58px tall + // with the compass hidden - see Map.tsx's addControl call) plus its + // own 10px margin, with only a small extra gap above it. + top: 78px; + right: 10px; + bottom: auto; + width: 90%; + max-width: min(90vw, 200px); + max-height: 75vh; + border-radius: sp.$border-radius; + // No dimming overlay behind this panel (it's non-modal - see the + // `modal` prop comment in DetailSurface.tsx), so a border is the only + // thing separating it from the map underneath. + border: 1px solid c.$color-border-primary; + } +} diff --git a/frontend/src/components/MapDetailCard/MapDetailCard.stories.tsx b/frontend/src/components/MapDetailCard/MapDetailCard.stories.tsx new file mode 100644 index 00000000..5f9202d2 --- /dev/null +++ b/frontend/src/components/MapDetailCard/MapDetailCard.stories.tsx @@ -0,0 +1,52 @@ +import type { Meta, StoryObj } from '@storybook/react-vite'; +import { expect, within } from 'storybook/test'; +import MapDetailCard from './MapDetailCard'; + +/** + * `MapDetailCard` is the map's docked side-panel variant of DetailCard's + * header/content layout - a click on a character/building stays open and + * browsable alongside the map instead of covering it. Non-modal (no dimming + * overlay, the map underneath stays interactive), narrower than DetailCard, + * and positions relative to the map itself (via the `container` prop) + * rather than the viewport. + */ +const meta: Meta = { + title: 'Shared/MapDetailCard', + component: MapDetailCard, + tags: ['autodocs'], + // Renders via a Radix Portal into document.body - escapes the story + // canvas with inline docs rendering, so use an iframe like AlertDialog. + parameters: { + docs: { + story: { inline: false, iframeHeight: 320 }, + }, + }, + args: { + open: true, + title: 'Rose Cottage', + onClose: () => {}, + children: ( + <> +

House

+

Residents: 4

+
    +
  • Alice (idle)
    • +
  • Thomas (delivering goods to neighbours)
    • +
  • Emily (idle)
    • +
  • James (idle)
    • +
+ + ), + }, +}; + +export default meta; +type Story = StoryObj; + +export const Default: Story = { + play: async ({ canvasElement }) => { + const body = within(canvasElement.ownerDocument.body); + const dialog = await body.findByRole('dialog', { name: 'Rose Cottage' }); + await expect(dialog).toBeVisible(); + }, +}; diff --git a/frontend/src/components/MapDetailCard/MapDetailCard.tsx b/frontend/src/components/MapDetailCard/MapDetailCard.tsx new file mode 100644 index 00000000..5a4bd4f6 --- /dev/null +++ b/frontend/src/components/MapDetailCard/MapDetailCard.tsx @@ -0,0 +1,47 @@ +import type React from "react"; +import DetailSurface from "../DetailSurface/DetailSurface"; +import { DetailCardBody } from "../DetailCard/DetailCard"; +import styles from "./MapDetailCard.module.scss"; + +interface MapDetailCardProps { + open: boolean; + title: string; + onClose: () => void; + children: React.ReactNode; + onFlyTo?: () => void; + /** Map's own wrapper element, passed through to DetailSurface so this + * portals (and positions) relative to the map instead of the viewport - + * see the `container` prop comment on DetailSurface. */ + container?: HTMLElement | null; +} + +// Map's docked side panel (see Map's "click a tooltip -> detail card" flow) - +// reuses DetailCard's header/content layout (DetailCardBody) but, unlike +// DetailCard's centered viewport-fixed modal, docks to the map's own corner +// and stays non-modal so the map underneath stays interactive. Kept as its +// own component rather than a DetailCard variant because it needs a +// different portal target and positioning scheme, not just different CSS. +export default function MapDetailCard({ + open, + title, + onClose, + children, + onFlyTo, + container, +}: MapDetailCardProps) { + return ( + { + if (!next) onClose(); + }} + title={title} + modal={false} + container={container} + > + + {children} + + + ); +} From 88caabaff67966aacdf0e78573e64677777e2017 Mon Sep 17 00:00:00 2001 From: Duncan Appleby Date: Sun, 9 Aug 2026 23:35:42 +0100 Subject: [PATCH 46/47] fix: make character outlines smaller radius --- frontend/src/components/Map/layers.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frontend/src/components/Map/layers.ts b/frontend/src/components/Map/layers.ts index a401fef8..a7961143 100644 --- a/frontend/src/components/Map/layers.ts +++ b/frontend/src/components/Map/layers.ts @@ -312,8 +312,8 @@ export function addVillageLayers(map: MapLibreMap): void { "interpolate", ["linear"], ["zoom"], - 12, 8, - 16, 26, + 12, 6, + 16, 18, ], "circle-color": "transparent", "circle-stroke-color": SELECTION_HIGHLIGHT_COLOR, @@ -338,8 +338,8 @@ export function addVillageLayers(map: MapLibreMap): void { "interpolate", ["linear"], ["zoom"], - 12, 8, - 16, 26, + 12, 6, + 16, 18, ], "circle-color": "transparent", "circle-stroke-color": SELECTION_HIGHLIGHT_COLOR, From e12199f961297784fc6fb3aafed9caa70e6d4392 Mon Sep 17 00:00:00 2001 From: Duncan Appleby Date: Sun, 9 Aug 2026 21:52:47 +0100 Subject: [PATCH 47/47] Fix character activity schedule to respect the assigned work building's hours generate_day hardcoded a fixed 8:00-17:00 work window regardless of the character's actual work building, while movement (target_role_for) already read the building's real open_time/close_time. For a late-closing building like the inn (06:00-23:00), this meant a worker was still physically at the inn well into the evening but their scheduled activity had already fallen through to the fixed leisure block, showing "Relaxing" instead of a work activity. Extracted the building-hours lookup into a shared work_hours_for helper and use it to size the work blocks (and push dinner/leisure/wind-down after work actually ends) in generate_day too. Co-Authored-By: Claude Sonnet 5 --- character/services/behaviour_services.py | 32 ++++++++++++-------- character/tests/test_behaviour_services.py | 34 +++++++++++++++++++-- locations/services/schedule.py | 35 +++++++++++++++------- 3 files changed, 77 insertions(+), 24 deletions(-) diff --git a/character/services/behaviour_services.py b/character/services/behaviour_services.py index 5f441594..8e2b07ed 100644 --- a/character/services/behaviour_services.py +++ b/character/services/behaviour_services.py @@ -7,6 +7,7 @@ from django.utils import timezone from character.utils import window_for_date, work_activities_for +from locations.services.schedule import work_hours_for from progression.models import ActivityDefinition, CharacterActivity _FIXED_KINDS = [ @@ -54,32 +55,39 @@ def aware(dt_date, t: time): def jitter_minutes(base_dt, minutes): return base_dt + timedelta(minutes=rng.randint(-minutes, minutes)) - sleep_start = aware(date, time(23, 0)) wake = aware(date, time(7, 0)) wake = jitter_minutes(wake, 15) morning_start = wake morning_end = morning_start + timedelta(hours=1) - work1_start = morning_end - work1_end = aware(date, time(12, 0)) - - lunch_start = work1_end - lunch_start = jitter_minutes(lunch_start, 10) + # The work window comes from the character's actual assigned work + # building's hours (same source movement uses - see + # locations.services.schedule.target_role_for) rather than a fixed + # 8-17 assumption, so e.g. an inn open until 23:00 keeps its workers' + # scheduled activity as "working" that late instead of falling through + # to the fixed evening leisure block. + default_work_start, default_work_end = work_hours_for(behaviour.character) + work_start = max(morning_end, aware(date, default_work_start)) + work_end = aware(date, default_work_end) + + lunch_midpoint = work_start + (work_end - work_start) / 2 + lunch_start = jitter_minutes(lunch_midpoint, 10) lunch_end = lunch_start + timedelta(hours=1) + work1_start = work_start + work1_end = lunch_start work2_start = lunch_end - work2_end = aware(date, time(17, 0)) + work2_end = work_end - dinner_start = aware(date, time(17, 30)) - dinner_start = jitter_minutes(dinner_start, 10) + dinner_start = jitter_minutes(max(work_end, aware(date, time(17, 30))), 10) dinner_end = dinner_start + timedelta(hours=1) leisure_start = dinner_end - leisure_end = aware(date, time(22, 30)) + leisure_end = max(leisure_start, aware(date, time(22, 30))) wind_start = leisure_end - wind_end = aware(date, time(23, 0)) + wind_end = max(wind_start, aware(date, time(23, 0))) day_window(behaviour, date) @@ -87,7 +95,7 @@ def jitter_minutes(base_dt, minutes): next_wake = aware(next_day, time(7, 0)) next_wake = jitter_minutes(next_wake, 15) - sleep_start = aware(date, time(23, 0)) + sleep_start = wind_end sleep_end = next_wake fixed = _fixed_activity_definitions() diff --git a/character/tests/test_behaviour_services.py b/character/tests/test_behaviour_services.py index 4138d3c5..cdbeb2d8 100644 --- a/character/tests/test_behaviour_services.py +++ b/character/tests/test_behaviour_services.py @@ -1,11 +1,13 @@ -from datetime import date +from datetime import date, datetime, time from django.contrib.gis.geos import Point from django.test import TestCase +from django.utils import timezone -from character.models import Character +from character.models import Character, CharacterLocation from character.services.behaviour_services import _FIXED_KINDS from character.utils import work_activities_for +from locations.models import Building from progression.models import ( ActivityDefinition, CharacterActivity, @@ -129,6 +131,34 @@ def test_generating_the_same_day_twice_is_deterministic(self): self.assertEqual(first_ids, second_ids) + def test_late_building_hours_extend_the_work_block_past_the_default_workday(self): + # Inn hours run 06:00-23:00 (see Building.BUILDING_TYPE_HOURS) - well + # past generate_day's old fixed 17:00 work cutoff. An inn worker + # should still be scheduled as "working" in the evening instead of + # falling through to the fixed leisure block (issue: characters + # assigned to the inn showed as "Relaxing" during their shift). + inn = Building.objects.create( + name="The Tipsy Griffin", + building_type="inn", + location=Point(0, 0, srid=3857), + ) + CharacterLocation.objects.create( + character=self.character, + location=inn, + role=CharacterLocation.Role.WORK, + is_primary=True, + ) + + self.character.behaviour.generate_day(date(2026, 1, 5)) + + evening = timezone.make_aware(datetime.combine(date(2026, 1, 5), time(21, 0))) + activity_at_evening = CharacterActivity.objects.get( + character=self.character, + scheduled_start__lte=evening, + scheduled_end__gt=evening, + ) + self.assertEqual(activity_at_evening.activity_definition.kind, "work") + class DeleteDayTests(TestCase): def setUp(self): diff --git a/locations/services/schedule.py b/locations/services/schedule.py index 8e990d39..c2fffbba 100644 --- a/locations/services/schedule.py +++ b/locations/services/schedule.py @@ -21,18 +21,17 @@ def _stagger_offset_seconds(character_id: int) -> int: return (character_id % span) - MAX_STAGGER_SECONDS -def target_role_for(character, now=None) -> str: - """Which role (home/work) a character should currently be at. - - The work window comes from the character's assigned work building's - open_time/close_time if set, else falls back to the fixed WORK_START/ - WORK_END constants. A per-character stagger is applied to whichever - window is resolved, so the whole village doesn't flip in lockstep.""" +def work_hours_for(character) -> tuple[time, time]: + """The (open, close) window a character should be at work, from their + assigned work building's open_time/close_time if set, else the fixed + WORK_START/WORK_END constants. Shared by target_role_for (drives + physical movement) and behaviour_services.generate_day (drives the + scheduled CharacterActivity blocks), so a character's actual work + building's hours - e.g. an inn open until 23:00 - govern both rather + than generate_day assuming a fixed 8-17 workday that leaves late + building hours showing as an unrelated leisure/"Relaxing" block.""" from character.models import CharacterLocation - now = now or timezone.localtime() - seconds_since_midnight = now.hour * 3600 + now.minute * 60 + now.second - work_start, work_end = WORK_START, WORK_END work_location = ( CharacterLocation.objects.filter( @@ -45,6 +44,22 @@ def target_role_for(character, now=None) -> str: building = work_location.location if building.open_time is not None and building.close_time is not None: work_start, work_end = building.open_time, building.close_time + return work_start, work_end + + +def target_role_for(character, now=None) -> str: + """Which role (home/work) a character should currently be at. + + The work window comes from the character's assigned work building's + open_time/close_time if set, else falls back to the fixed WORK_START/ + WORK_END constants. A per-character stagger is applied to whichever + window is resolved, so the whole village doesn't flip in lockstep.""" + from character.models import CharacterLocation + + now = now or timezone.localtime() + seconds_since_midnight = now.hour * 3600 + now.minute * 60 + now.second + + work_start, work_end = work_hours_for(character) offset = _stagger_offset_seconds(character.id) work_start_seconds = work_start.hour * 3600 + work_start.minute * 60 + offset