From db8d799798eff16cda25e8d5bc4d28b75ccf274f Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Mon, 28 Sep 2026 15:01:41 +0700 Subject: [PATCH] fix(client): remember refused storage writes, clear the pin between rounds Blocked storage kept a refused write nowhere, so the name prompt never closed and every round scored under a new random name; storage.js now holds it in memory for the visit. The guess marker is controlled by the current guess and the map reframes on each round. Reloading /daily after playing reads the stored record instead of fetching a round, and the mounted flag survives StrictMode so result sounds play and no prefetch starts after leaving. GameClient's load paths share one helper and stop threading session ids. ShareButton, PlayLink and ThemeSync replace duplicated share, play-link and theme-apply code; the coverage page orphans a pano lookup on region switch. --- src/app/components/DailyCard.js | 58 +++----- src/app/components/GameClient.js | 117 ++++++++--------- src/app/components/GuessMapPanel.js | 6 +- src/app/components/LeafletMap.js | 167 +++++++++++------------- src/app/components/PlayLink.js | 26 ++++ src/app/components/RegionPicker.js | 14 +- src/app/components/RoundResultDialog.js | 45 ++----- src/app/components/ShareButton.js | 43 ++++++ src/app/components/ThemeSync.js | 26 ++++ src/app/components/ThemeToggle.js | 14 +- src/app/components/UsernameModal.js | 95 +++++++------- src/app/debug/coverage/page.js | 8 +- src/app/layout.js | 11 +- src/app/page.js | 2 +- src/lib/share.js | 18 ++- src/lib/storage.js | 10 +- tests/storage.test.js | 23 +++- 17 files changed, 371 insertions(+), 312 deletions(-) create mode 100644 src/app/components/PlayLink.js create mode 100644 src/app/components/ShareButton.js create mode 100644 src/app/components/ThemeSync.js diff --git a/src/app/components/DailyCard.js b/src/app/components/DailyCard.js index c055a55..f13fa63 100644 --- a/src/app/components/DailyCard.js +++ b/src/app/components/DailyCard.js @@ -1,17 +1,16 @@ "use client"; -import { useState } from 'react'; import Link from 'next/link'; -import { AlertCircle, ArrowRight, Calendar, Check, Share2 } from 'lucide-react'; +import { ArrowRight, Calendar } from 'lucide-react'; import { Button } from '@/components/ui/button'; import { Card, CardContent } from '@/components/ui/card'; import { dailyDay, dailyNumber } from '../../lib/daily-calendar'; import { getDailyProgress, watchDailyProgress, currentStreak } from '../../lib/daily-progress'; import { useStoredValue } from '../../lib/use-stored-value'; -import { buildDailyShareText, shareText } from '../../lib/share'; +import { buildDailyShareText, scoreLine } from '../../lib/share'; import { formatDistance } from '../../lib/game'; - -const MAX_POINTS = 5; +import ShareButton from './ShareButton'; +import PlayLink from './PlayLink'; // The day never changes under an open page in a way worth subscribing to; a // reload at midnight is the refresh. @@ -27,8 +26,6 @@ const subscribeNothing = () => () => {}; * @param {Function} props.onPlayClick Same interception hook as the region rows. */ export default function DailyCard({ onPlayClick }) { - const [shareState, setShareState] = useState(null); - // Today's day is browser-only knowledge: the page is prerendered, and a day // computed at build time would be frozen into the HTML and disagree with // the client on every later day. Read through the same store hook as the @@ -40,30 +37,14 @@ export default function DailyCard({ onPlayClick }) { const number = today ? dailyNumber(today) : null; const played = today && progress?.day === today ? progress : null; const streak = today ? currentStreak(progress, today) : 0; - const shareLabel = shareState === 'copied' ? 'Copied' : shareState === 'failed' ? 'Retry' : 'Share'; - const ShareIcon = shareState === 'copied' ? Check : shareState === 'failed' ? AlertCircle : Share2; - const shareStatus = - shareState === 'copied' ? 'Copied to the clipboard.' - : shareState === 'shared' ? 'Shared.' - : shareState === 'failed' ? 'Sharing failed. Try again.' - : ''; - - const handleShare = async () => { - const outcome = await shareText( - buildDailyShareText( - played.number, - played.result.score, - formatDistance(played.result.distance), - played.streak, - `${window.location.origin}/daily` - ) + const getShareText = () => + buildDailyShareText( + played.number, + played.result.score, + formatDistance(played.result.distance), + played.streak, + `${window.location.origin}/daily` ); - setShareState(outcome); - }; - - const squares = played - ? '🟩'.repeat(played.result.score) + '⬜'.repeat(MAX_POINTS - played.result.score) - : ''; return ( @@ -84,7 +65,7 @@ export default function DailyCard({ onPlayClick }) {

{played - ? `Played today: ${squares} ${played.result.score}/${MAX_POINTS} · ${formatDistance(played.result.distance)} away` + ? `Played today: ${scoreLine(played.result.score)} · ${formatDistance(played.result.distance)} away` : 'One street view, one guess, the same for everyone. New at midnight, Vietnam time.'}

@@ -92,27 +73,20 @@ export default function DailyCard({ onPlayClick }) { {played ? (
- + -

{shareStatus}

) : ( - { - if (e.metaKey || e.ctrlKey || e.shiftKey || e.altKey || e.button !== 0) return; - if (onPlayClick && onPlayClick('/daily')) e.preventDefault(); - }} + onPlayClick={onPlayClick} className="inline-flex min-h-11 items-center gap-1.5 rounded-lg bg-brand px-4 py-2 text-sm font-semibold text-brand-foreground transition-colors hover:bg-brand/90 focus-visible:outline-none focus-visible:ring-[3px] focus-visible:ring-ring" > Play today's challenge diff --git a/src/app/components/GameClient.js b/src/app/components/GameClient.js index 09207b2..fc51142 100644 --- a/src/app/components/GameClient.js +++ b/src/app/components/GameClient.js @@ -48,19 +48,11 @@ function resultSound(score) { * Ask the server for a new round. Throws on failure; touches no state, so the * result-screen prefetch can call it without disturbing the round on screen. * @param {string} locationCode Region to play. - * @param {string|null} currentSessionId Session id to reuse, or null for a new one. * @param {boolean} daily True for today's daily challenge instead of a region draw. * @returns {Promise} The /api/new-game or /api/daily payload. */ -async function fetchNewRound(locationCode, currentSessionId, daily) { - let url = '/api/daily'; - if (!daily) { - // Encoded: an unencoded value carrying its own '&sessionId=' would let a - // shared link choose the session key a victim's round is stored under. - const params = new URLSearchParams({ region: locationCode }); - if (currentSessionId) params.set('sessionId', currentSessionId); - url = `/api/new-game?${params.toString()}`; - } +async function fetchNewRound(locationCode, daily) { + const url = daily ? '/api/daily' : `/api/new-game?${new URLSearchParams({ region: locationCode })}`; // A ceiling on the wait: the server gives itself eight seconds to draw a // round, so a request still open after fifteen is not coming back, and the @@ -112,7 +104,6 @@ export default function GameClient({ region, daily = false }) { // A failed round fetch, shown in place with a retry. Never an alert(): that // left an empty screen whose only way out was the browser back button. const [loadError, setLoadError] = useState(null); - const [initialized, setInitialized] = useState(false); const [guessCoordinatesState, setGuessCoordinates] = useState(null); const [showResultState, setShowResult] = useState(false); // Everything a submitted round produced, in one object set in exactly one @@ -163,7 +154,10 @@ export default function GameClient({ region, daily = false }) { // in state painted one frame of Ho Chi Minh on every other region. const mapCenter = pickedRegion?.center ?? [10.8231, 106.6297]; - const initializingRef = useRef(false); + // Set once the first round fetch has been started. A ref, not state: + // StrictMode replays the mount effect before any state update lands, and a + // second first-load would mint a second session. + const startedRef = useRef(false); // The next round, fetched while the result dialog is open so Next Round can // swap it in without a wait. Holds a promise; consumed exactly once. const prefetchRef = useRef(null); @@ -177,9 +171,14 @@ export default function GameClient({ region, daily = false }) { // Back stays live while a guess is in flight, so a submit can resolve after // the player has already left for the menu. State updates after that are // harmless no-ops; a victory jingle over the home screen is not. + // Set in the effect as well as cleared in its cleanup: StrictMode unmounts + // and remounts once, and a flag only ever cleared stays false for good. const mountedRef = useRef(true); - useEffect(() => () => { mountedRef.current = false; }, []); + useEffect(() => { + mountedRef.current = true; + return () => { mountedRef.current = false; }; + }, []); const applyRound = useCallback((data) => { if (data.day) { @@ -206,9 +205,11 @@ export default function GameClient({ region, daily = false }) { setLoadError(null); }, []); - const loadRound = useCallback(async (locationCode, currentSessionId, epoch) => { + // Never throws. On failure it shows the error panel and ends roundLoading + // itself, so every caller is just "start a load under this epoch". + const loadRound = useCallback(async (locationCode, epoch) => { try { - const data = await fetchNewRound(locationCode, currentSessionId, daily); + const data = await fetchNewRound(locationCode, daily); // Superseded while in flight: the newer action owns the screen and the // roundLoading flag, so touch nothing and report nothing to clear. if (epoch !== roundEpochRef.current) return true; @@ -220,6 +221,7 @@ export default function GameClient({ region, daily = false }) { setImageData(null); setSessionId(null); setLoadError(error.message || 'No images found'); + setRoundLoading(false); return false; } }, [applyRound, daily]); @@ -227,31 +229,27 @@ export default function GameClient({ region, daily = false }) { // initialLoading starts true and nothing here touches it before the first // await, so this can run from the mount effect without a synchronous // setState. - const loadLibrariesAndInitialize = useCallback(async (locationCode) => { - if (initializingRef.current) return; - initializingRef.current = true; - - try { - const code = locationCode.toUpperCase(); - roundEpochRef.current += 1; - const loaded = await loadRound(locationCode, null, roundEpochRef.current); - // Only a region that actually served a round is worth offering as - // "Continue in ..." on the home page. - if (loaded && !daily && isRegion(code)) setLastRegion(code); - setInitialized(true); - } catch (error) { - console.error('Failed to initialize:', error); - } finally { - initializingRef.current = false; - setInitialLoading(false); - } + const loadFirstRound = useCallback(async (locationCode) => { + roundEpochRef.current += 1; + const loaded = await loadRound(locationCode, roundEpochRef.current); + // Only a region that actually served a round is worth offering as + // "Continue in ..." on the home page. + if (loaded && !daily && isRegion(locationCode)) setLastRegion(locationCode); + setInitialLoading(false); }, [loadRound, daily]); useEffect(() => { + if (startedRef.current) return; // Already played today: the stored round is on screen, nothing to fetch. - if (initialized || replay) return; - loadLibrariesAndInitialize(region); - }, [region, loadLibrariesAndInitialize, initialized, replay]); + // Read from storage, not from `replay`: on a hydrated load this first + // commit still sees the server snapshot (null), and its effects run + // before the re-render that would show the stored record. + if (daily && getDailyProgress()?.day === dailyDay()) return; + startedRef.current = true; + loadFirstRound(region); + // `replay` is a dependency so a record cleared from another tab starts a + // round instead of leaving the loading screen up. + }, [region, daily, loadFirstRound, replay]); const submitGameResult = async (guessCoords) => { if (!guessCoords || !sessionId) return null; @@ -323,8 +321,8 @@ export default function GameClient({ region, daily = false }) { // Start fetching the next round while the player reads the result, and warm // the browser cache with its image. The wasted lookup when they leave from // the dialog is one API call and one self-expiring session. - const startPrefetch = (locationCode, currentSessionId) => { - const promise = fetchNewRound(locationCode, currentSessionId).then((data) => { + const startPrefetch = (locationCode) => { + const promise = fetchNewRound(locationCode, false).then((data) => { if (typeof window !== 'undefined' && data.imageData?.url) { const image = new window.Image(); image.src = data.imageData.url; @@ -342,7 +340,6 @@ export default function GameClient({ region, daily = false }) { // After the guard: a blocked submit stays silent. playSound('submit'); setSubmitting(true); - const currentSession = sessionId; try { const submitted = await submitGameResult(guessCoordinates); @@ -391,8 +388,9 @@ export default function GameClient({ region, daily = false }) { setSubmitting(false); setShowResult(true); - // One round a day: there is no next one to fetch. - if (!daily) startPrefetch(region, currentSession); + // One round a day: there is no next one to fetch. Nor is there once the + // player has left for the menu. + if (!daily && mountedRef.current) startPrefetch(region); }; // Everything a round accumulates. Both Next Round and Skip come through here, @@ -418,7 +416,6 @@ export default function GameClient({ region, daily = false }) { const epoch = roundEpochRef.current; setShowResult(false); resetRoundState(); - const currentSession = sessionId; setSessionId(null); const prefetched = prefetchRef.current; prefetchRef.current = null; @@ -439,8 +436,15 @@ export default function GameClient({ region, daily = false }) { } } - const loaded = await loadRound(region, currentSession, epoch); - if (!loaded) setRoundLoading(false); + await loadRound(region, epoch); + }; + + // Start a fresh round load that supersedes whatever is in flight. + const reloadRound = () => { + setLoadError(null); + setRoundLoading(true); + roundEpochRef.current += 1; + return loadRound(region, roundEpochRef.current); }; const handleSkipGuess = async () => { @@ -461,26 +465,14 @@ export default function GameClient({ region, daily = false }) { resetRoundState(); prefetchRef.current = null; - roundEpochRef.current += 1; setSessionId(null); - // Skip is also the way out of the error panel; leaving the error up would - // suppress the spinner and read as a hang while the new round loads. - setLoadError(null); - setRoundLoading(true); - // A fresh id, never the one just skipped: the DEL above is still in - // flight, and reusing the id let it land after the new round's write and - // delete a live session. - const loaded = await loadRound(region, null, roundEpochRef.current); - if (!loaded) setRoundLoading(false); + // Skip is also the way out of the error panel (reloadRound clears it). + // The server mints a fresh session id every round, so the DEL above can + // land after the new round's write without touching it. + await reloadRound(); }; - const handleRetryLoad = async () => { - setLoadError(null); - roundEpochRef.current += 1; - setRoundLoading(true); - const loaded = await loadRound(region, null, roundEpochRef.current); - if (!loaded) setRoundLoading(false); - }; + const handleRetryLoad = reloadRound; const handleGoBack = () => { playSound('click'); @@ -579,7 +571,8 @@ export default function GameClient({ region, daily = false }) { regionCode={pickedRegion?.code ?? 'VN'} expanded={mapExpanded} onExpandedChange={setMapExpanded} - hasGuess={Boolean(guessCoordinates)} + guess={guessCoordinates} + viewKey={roundKey} onMapClick={handleMapClick} /> diff --git a/src/app/components/GuessMapPanel.js b/src/app/components/GuessMapPanel.js index 6393928..1bc6ce7 100644 --- a/src/app/components/GuessMapPanel.js +++ b/src/app/components/GuessMapPanel.js @@ -21,9 +21,11 @@ export default function GuessMapPanel({ regionCode, expanded, onExpandedChange, - hasGuess, + guess, + viewKey, onMapClick, }) { + const hasGuess = Boolean(guess); const mapRef = useRef(null); // Mirrored into state so the search box renders once the map exists. const [mapInstance, setMapInstance] = useState(null); @@ -113,6 +115,8 @@ export default function GuessMapPanel({ zoom={10} // The search box owns the top-left corner on every breakpoint. zoomPosition="bottomleft" + marker={guess} + viewKey={viewKey} onMapClick={onMapClick} onReady={handleMapReady} className="w-full h-full" diff --git a/src/app/components/LeafletMap.js b/src/app/components/LeafletMap.js index c292df1..14dfe62 100644 --- a/src/app/components/LeafletMap.js +++ b/src/app/components/LeafletMap.js @@ -14,10 +14,33 @@ import { getTileConfig } from '../../lib/map-tiles'; // under Turbopack dev; Leaflet needs the string either way. const imageUrl = (image) => (typeof image === 'string' ? image : image.src); +/** + * Frame the map on a bbox when there is one, else on a centre and zoom. + * @param {L.Map} map + * @param {number[]|null} bbox [west, south, east, north]. + * @param {number[]} center [lat, lng]. + * @param {number} zoom + * @returns {void} + */ +function frameView(map, bbox, center, zoom) { + if (bbox) { + map.fitBounds(L.latLngBounds([bbox[1], bbox[0]], [bbox[3], bbox[2]]), { padding: [20, 20] }); + } else { + map.setView(center, zoom); + } +} + +// A click map with at most one pin. The pin is controlled: `marker` says where +// it is, and a click only reports coordinates, so a parent clearing its guess +// between rounds clears the pin too. Changing `viewKey` re-frames the map on +// the region, which is how a new round starts from the whole region again +// rather than wherever the last guess was zoomed to. export default function LeafletMap({ center, zoom = 10, bbox = null, + marker = null, + viewKey = 0, onMapClick, onReady, // Consumers that overlay UI on the top-left corner (the guess map's search @@ -27,109 +50,77 @@ export default function LeafletMap({ }) { const mapRef = useRef(null); const leafletMapRef = useRef(null); - const markersRef = useRef([]); + const markerRef = useRef(null); // Effect events: always call the latest callback props without making them // effect dependencies, so a parent re-rendering with a new function identity // does not tear the map down. const emitMapClick = useEffectEvent((coords) => onMapClick?.(coords)); const emitReady = useEffectEvent((map) => onReady?.(map)); + const placeMarker = useEffectEvent(() => { + const map = leafletMapRef.current; + markerRef.current?.remove(); + markerRef.current = map && marker ? L.marker(marker).addTo(map) : null; + }); + const reframe = useEffectEvent(() => { + if (leafletMapRef.current) frameView(leafletMapRef.current, bbox, center, zoom); + }); useEffect(() => { - const initMap = async () => { - if (!mapRef.current || leafletMapRef.current) return; + if (!mapRef.current || leafletMapRef.current) return undefined; - try { + try { + // Fix default marker icons issue + delete L.Icon.Default.prototype._getIconUrl; + L.Icon.Default.mergeOptions({ + iconRetinaUrl: imageUrl(markerIcon2x), + iconUrl: imageUrl(markerIcon), + shadowUrl: imageUrl(markerShadow), + }); - // Fix default marker icons issue - delete L.Icon.Default.prototype._getIconUrl; - L.Icon.Default.mergeOptions({ - iconRetinaUrl: imageUrl(markerIcon2x), - iconUrl: imageUrl(markerIcon), - shadowUrl: imageUrl(markerShadow), - }); + const map = L.map(mapRef.current, { zoomControl: false }); + L.control.zoom({ position: zoomPosition }).addTo(map); + frameView(map, bbox, center, zoom); - // Create map - const map = L.map(mapRef.current, { zoomControl: false }); - L.control.zoom({ position: zoomPosition }).addTo(map); + const tiles = getTileConfig(); + L.tileLayer(tiles.url, tiles.options).addTo(map); - // Set initial view - use bbox if provided, otherwise center/zoom - if (bbox) { - // bbox format: [west, south, east, north] - const bounds = L.latLngBounds( - L.latLng(bbox[1], bbox[0]), // southwest corner - L.latLng(bbox[3], bbox[2]) // northeast corner - ); - map.fitBounds(bounds, { padding: [20, 20] }); - } else { - map.setView(center, zoom); - } + map.on('click', (e) => emitMapClick({ lat: e.latlng.lat, lng: e.latlng.lng })); - // Add tile layer - const tiles = getTileConfig(); - L.tileLayer(tiles.url, tiles.options).addTo(map); - - // Handle click events - map.on('click', (e) => { - // Clear existing markers - markersRef.current.forEach(marker => { - map.removeLayer(marker); - }); - markersRef.current = []; - - // Add new marker - const marker = L.marker([e.latlng.lat, e.latlng.lng]).addTo(map); - markersRef.current.push(marker); - - emitMapClick({ lat: e.latlng.lat, lng: e.latlng.lng }); - }); - - leafletMapRef.current = map; - emitReady(map); - - } catch (error) { - console.error('Error initializing Leaflet map:', error); - } - }; - - initMap(); - - // Cleanup function - return () => { - if (leafletMapRef.current) { - try { - leafletMapRef.current.remove(); - } catch (error) { - console.warn('Error removing map:', error); - } - leafletMapRef.current = null; - markersRef.current = []; - // The handle given out by onReady is now a destroyed map; a parent - // still holding it would crash on the first pan/zoom call. - emitReady(null); - } - }; - }, [bbox, center, zoom, zoomPosition]); // Include props used in initialization - - // Update view when props change - useEffect(() => { - if (leafletMapRef.current) { - if (bbox) { - // bbox format: [west, south, east, north] - const bounds = L.latLngBounds( - L.latLng(bbox[1], bbox[0]), // southwest corner - L.latLng(bbox[3], bbox[2]) // northeast corner - ); - leafletMapRef.current.fitBounds(bounds, { padding: [20, 20] }); - } else if (center) { - leafletMapRef.current.setView(center, zoom); - } + leafletMapRef.current = map; + // A rebuilt map starts empty; put back the pin the parent still holds. + placeMarker(); + emitReady(map); + } catch (error) { + console.error('Error initializing Leaflet map:', error); } - }, [center, zoom, bbox]); + + return () => { + if (!leafletMapRef.current) return; + try { + leafletMapRef.current.remove(); + } catch (error) { + console.warn('Error removing map:', error); + } + leafletMapRef.current = null; + markerRef.current = null; + // The handle given out by onReady is now a destroyed map; a parent + // still holding it would crash on the first pan/zoom call. + emitReady(null); + }; + }, [bbox, center, zoom, zoomPosition]); + + useEffect(() => { + placeMarker(); + }, [marker]); + + useEffect(() => { + reframe(); + }, [viewKey]); return ( -
); -} \ No newline at end of file +} diff --git a/src/app/components/PlayLink.js b/src/app/components/PlayLink.js new file mode 100644 index 0000000..b16b34d --- /dev/null +++ b/src/app/components/PlayLink.js @@ -0,0 +1,26 @@ +"use client"; + +import Link from 'next/link'; + +/** + * A link into a game that lets the home page intercept a plain click (to ask + * for a name first). Modified clicks (new tab, new window) keep native + * behaviour: the deep-linked game page can name the player itself, so + * hijacking the gesture would cost more than the prompt is worth. + * @param {Object} props + * @param {string} props.href Game URL. + * @param {(href: string) => boolean} [props.onPlayClick] Returning true cancels + * the navigation (the caller resumes it itself). + */ +export default function PlayLink({ href, onPlayClick, ...linkProps }) { + return ( + { + if (e.metaKey || e.ctrlKey || e.shiftKey || e.altKey || e.button !== 0) return; + if (onPlayClick?.(href)) e.preventDefault(); + }} + /> + ); +} diff --git a/src/app/components/RegionPicker.js b/src/app/components/RegionPicker.js index aaa144a..c3375ed 100644 --- a/src/app/components/RegionPicker.js +++ b/src/app/components/RegionPicker.js @@ -1,6 +1,6 @@ "use client"; -import Link from 'next/link'; +import PlayLink from './PlayLink'; import { ArrowRight } from 'lucide-react'; import { getLastRegion, watchLastRegion } from '../../lib/last-region'; @@ -56,15 +56,9 @@ function PlayRow({ code, label, emphasis, onPlayClick }) { const href = `/game/${regionSlug(code)}`; return ( - { - // Modified clicks (new tab, new window) keep native behavior: the - // deep-linked game page can name the player itself, so hijacking the - // gesture would cost more than the prompt is worth. - if (e.metaKey || e.ctrlKey || e.shiftKey || e.altKey || e.button !== 0) return; - if (onPlayClick && onPlayClick(href)) e.preventDefault(); - }} + onPlayClick={onPlayClick} className={`group flex min-h-14 items-center justify-between gap-3 rounded-xl border p-4 shadow-xs transition-all duration-150 hover:border-brand/40 hover:shadow-md focus-visible:outline-none focus-visible:ring-[3px] focus-visible:ring-ring focus-visible:ring-offset-2 focus-visible:ring-offset-background ${ emphasis ? 'city-card-accent border-border bg-card' : 'border-border/60 bg-card/60' }`} @@ -94,7 +88,7 @@ function PlayRow({ code, label, emphasis, onPlayClick }) {
- {/* Announced once per outcome; visually the button label already says it. */} -

- {SHARE_STATUS[shareState] ?? ''} -

); diff --git a/src/app/components/ShareButton.js b/src/app/components/ShareButton.js new file mode 100644 index 0000000..a8bec48 --- /dev/null +++ b/src/app/components/ShareButton.js @@ -0,0 +1,43 @@ +"use client"; + +import { useState } from 'react'; +import { AlertCircle, Check, Share2 } from 'lucide-react'; +import { Button } from '@/components/ui/button'; +import { shareText } from '../../lib/share'; + +// What the share did, for the screen reader; the visible label carries the +// same word so nothing depends on colour or an icon alone. +const STATUS = { + shared: 'Shared.', + copied: 'Copied to the clipboard.', + failed: 'Sharing failed. Try again.', +}; + +/** + * A share button that remembers what the last press did. Unmount it (or give + * it a new key) to start fresh for a new result. + * @param {Object} props + * @param {() => string} props.getText Builds the text at press time. + * @param {string} props.title Tooltip. + * @param {string} [props.labelClassName] Classes for the visible word. + */ +export default function ShareButton({ getText, title, labelClassName, ...buttonProps }) { + const [outcome, setOutcome] = useState(null); + const label = outcome === 'copied' ? 'Copied' : outcome === 'failed' ? 'Retry' : 'Share'; + const Icon = outcome === 'copied' ? Check : outcome === 'failed' ? AlertCircle : Share2; + + return ( + <> + +

{STATUS[outcome] ?? ''}

+ + ); +} diff --git a/src/app/components/ThemeSync.js b/src/app/components/ThemeSync.js new file mode 100644 index 0000000..af17748 --- /dev/null +++ b/src/app/components/ThemeSync.js @@ -0,0 +1,26 @@ +"use client"; + +import { useEffect } from 'react'; +import { getStoredTheme, watchStoredTheme, applyTheme, watchSystemTheme } from '../../lib/theme'; +import { useStoredValue } from '../../lib/use-stored-value'; + +/** + * Keeps the document's theme in step with the stored choice and, while that + * choice is 'system', with the OS. Mounted once in the root layout, so the + * theme follows on every page -- not only where a toggle happens to be -- and + * an OS flip is applied once rather than once per mounted toggle. The toggles + * only write the choice. The first paint is the inline script in layout.js. + * @returns {null} + */ +export default function ThemeSync() { + const theme = useStoredValue(getStoredTheme, watchStoredTheme, null); + + useEffect(() => { + if (theme === null) return undefined; + applyTheme(theme); + if (theme !== 'system') return undefined; + return watchSystemTheme(() => applyTheme('system')); + }, [theme]); + + return null; +} diff --git a/src/app/components/ThemeToggle.js b/src/app/components/ThemeToggle.js index f2a5e10..6846123 100644 --- a/src/app/components/ThemeToggle.js +++ b/src/app/components/ThemeToggle.js @@ -1,6 +1,5 @@ "use client"; -import { useEffect } from 'react'; import { Monitor, Moon, Sun } from 'lucide-react'; import { THEMES, @@ -8,8 +7,6 @@ import { getStoredTheme, setStoredTheme, watchStoredTheme, - applyTheme, - watchSystemTheme, } from '../../lib/theme'; import { useStoredValue } from '../../lib/use-stored-value'; @@ -35,15 +32,8 @@ export default function ThemeToggle({ className = '', compact = false }) { // server marks nothing selected rather than guessing. const theme = useStoredValue(getStoredTheme, watchStoredTheme, null); - useEffect(() => { - if (theme === null) return undefined; - applyTheme(theme); - if (theme !== 'system') return undefined; - // Only while following the system does an OS change mean anything. - return watchSystemTheme(() => applyTheme('system')); - }, [theme]); - - // Writing storage is the whole update: every mounted toggle re-reads. + // Writing storage is the whole update: every mounted toggle re-reads, and + // ThemeSync in the root layout applies it to the document. const handleSelect = (choice) => setStoredTheme(choice); const groupClass = `inline-flex h-11 items-center rounded-lg border border-border bg-card ${className}`; diff --git a/src/app/components/UsernameModal.js b/src/app/components/UsernameModal.js index 2647524..b980051 100644 --- a/src/app/components/UsernameModal.js +++ b/src/app/components/UsernameModal.js @@ -73,56 +73,55 @@ function UsernameForm({ initialValue, hasExistingName, onSubmit, onSecondary }) return ( <> +
+
+ + setUsername(e.target.value)} + placeholder="Your username" + maxLength={20} + autoFocus + required + autoComplete="username" + aria-describedby="username-help" + aria-invalid={error ? true : undefined} + className="h-11" + /> +

+ 2-20 characters. Letters, numbers, hyphens and underscores. +

+ {error && ( + + {error} + + )} +
- -
- - setUsername(e.target.value)} - placeholder="Your username" - maxLength={20} - autoFocus - required - autoComplete="username" - aria-describedby="username-help" - aria-invalid={error ? true : undefined} - className="h-11" - /> -

- 2-20 characters. Letters, numbers, hyphens and underscores. -

- {error && ( - - {error} - - )} -
+
+ + +
+
-
- - -
- - -

- Displayed on the leaderboard -

+

+ Displayed on the leaderboard +

); } diff --git a/src/app/debug/coverage/page.js b/src/app/debug/coverage/page.js index f4a1c0b..0292ed0 100644 --- a/src/app/debug/coverage/page.js +++ b/src/app/debug/coverage/page.js @@ -51,8 +51,8 @@ export default function CoveragePage() { // than they come back, and an older one landing last would redraw stale dots. const requestIdRef = useRef(0); // Which region a response is allowed to draw for. The boundary is applied on - // this rather than on request order -- see load(). Kept current by the effect - // that switches region, which is the only thing that changes it. + // this rather than on request order -- see load(). Kept current by + // selectRegion, which is the only thing that changes it. const regionRef = useRef(region); const load = useCallback(async (regionCode, viewport) => { @@ -111,7 +111,11 @@ export default function CoveragePage() { setCounts(null); setGeneratedAt(null); setSelected(null); + // Orphan any pano lookup still in flight for the old region. + panoRequestRef.current += 1; setPano(null); + setPanoError(null); + setPanoLoading(false); load(code, null); }, [load]); diff --git a/src/app/layout.js b/src/app/layout.js index 3348199..2a854f6 100644 --- a/src/app/layout.js +++ b/src/app/layout.js @@ -1,6 +1,7 @@ import { THEME_STORAGE_KEY } from '../lib/theme'; import AppBackground from './components/AppBackground'; import MusicPlayer from './components/MusicPlayer'; +import ThemeSync from './components/ThemeSync'; import InlineScript from './components/InlineScript'; import DebugFooter from './components/DebugFooter'; import { Analytics } from '@vercel/analytics/next'; @@ -86,10 +87,12 @@ export default function RootLayout({ children }) { paints over the body background and under every page. */} - {/* Renders nothing, and its position is load-bearing: inside - but outside the flex column below, so the loop survives every - client navigation between the menu and a round, and a null- - rendering component can never become a row in that column. */} + {/* Both render nothing, and their position is load-bearing: inside + but outside the flex column below, so the music loop and + the theme watcher survive every client navigation between the + menu and a round, and a null-rendering component can never become + a row in that column. */} + {/* Sticky-footer column: pages fill the viewport via flex-1 and the diff --git a/src/app/page.js b/src/app/page.js index 558d9fe..b170e5e 100644 --- a/src/app/page.js +++ b/src/app/page.js @@ -3,7 +3,7 @@ import Link from "next/link"; import { useRouter } from "next/navigation"; import { Beer, Wrench } from "lucide-react"; -import React, { useState } from "react"; +import { useState } from "react"; import { Button } from '@/components/ui/button'; import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'; import ThemeToggle from './components/ThemeToggle'; diff --git a/src/lib/share.js b/src/lib/share.js index 1445038..bf88db0 100644 --- a/src/lib/share.js +++ b/src/lib/share.js @@ -7,6 +7,16 @@ const MAX_POINTS = 5; +/** + * A score as squares and a fraction, clamped to the ladder: '🟩🟩🟩⬜⬜ 3/5'. + * @param {number} score Points earned, 0-5. + * @returns {string} + */ +export function scoreLine(score) { + const points = Math.min(Math.max(Math.trunc(score) || 0, 0), MAX_POINTS); + return `${'🟩'.repeat(points)}${'⬜'.repeat(MAX_POINTS - points)} ${points}/${MAX_POINTS}`; +} + /** * Build the share text for one round. * @param {string} regionName Region the player picked, e.g. 'Ha Noi'. @@ -16,9 +26,7 @@ const MAX_POINTS = 5; * @returns {string} */ export function buildShareText(regionName, score, distanceLabel, url) { - const points = Math.min(Math.max(Math.trunc(score) || 0, 0), MAX_POINTS); - const squares = '🟩'.repeat(points) + '⬜'.repeat(MAX_POINTS - points); - return `VNGeoGuessr · ${regionName}\n${squares} ${points}/${MAX_POINTS} · ${distanceLabel} away\n${url}`; + return `VNGeoGuessr · ${regionName}\n${scoreLine(score)} · ${distanceLabel} away\n${url}`; } /** @@ -31,10 +39,8 @@ export function buildShareText(regionName, score, distanceLabel, url) { * @returns {string} */ export function buildDailyShareText(number, score, distanceLabel, streak, url) { - const points = Math.min(Math.max(Math.trunc(score) || 0, 0), MAX_POINTS); - const squares = '🟩'.repeat(points) + '⬜'.repeat(MAX_POINTS - points); const streakLine = streak > 1 ? ` · 🔥 ${streak} days` : ''; - return `VNGeoGuessr Daily #${number}\n${squares} ${points}/${MAX_POINTS} · ${distanceLabel} away${streakLine}\n${url}`; + return `VNGeoGuessr Daily #${number}\n${scoreLine(score)} · ${distanceLabel} away${streakLine}\n${url}`; } /** diff --git a/src/lib/storage.js b/src/lib/storage.js index b8c8bd0..56b75cc 100644 --- a/src/lib/storage.js +++ b/src/lib/storage.js @@ -9,6 +9,10 @@ // Reads and writes are guarded because storage is not always there: private // windows, blocked site data and thumbnail renderers all throw on access, and // a preference that cannot be remembered must never take the page down. +// A write the browser refuses is kept in memory instead, so it still holds +// for the rest of the visit: every value is read back through this module, +// and a name or theme that silently reverts leaves the page unusable (the +// name prompt would reopen forever). // // Change notification covers both tabs and this one. The browser's `storage` // event fires only in OTHER tabs, so a same-tab write also dispatches on a @@ -16,6 +20,8 @@ // pair, one hidden) stay in step that way. const local = typeof EventTarget === 'undefined' ? null : new EventTarget(); +// key -> value (or null for removed) for writes storage refused. +const refused = new Map(); /** * Read a raw string, or null when absent or unreadable. @@ -24,6 +30,7 @@ const local = typeof EventTarget === 'undefined' ? null : new EventTarget(); */ export function readItem(key) { if (typeof window === 'undefined') return null; + if (refused.has(key)) return refused.get(key); try { return localStorage.getItem(key); } catch { @@ -42,8 +49,9 @@ export function writeItem(key, value) { try { if (value === null) localStorage.removeItem(key); else localStorage.setItem(key, value); + refused.delete(key); } catch { - // Best effort: the caller's in-memory state still applies for this visit. + refused.set(key, value); } local?.dispatchEvent(new CustomEvent('change', { detail: key })); } diff --git a/tests/storage.test.js b/tests/storage.test.js index f0a443c..dd63e92 100644 --- a/tests/storage.test.js +++ b/tests/storage.test.js @@ -1,4 +1,4 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { readItem, writeItem, watchItem } from '../src/lib/storage.js'; import { getUsername, setUsername } from '../src/lib/username.js'; import { getStoredTheme, DEFAULT_THEME } from '../src/lib/theme.js'; @@ -20,3 +20,24 @@ describe('browser storage without a browser', () => { expect(getDailyProgress()).toBeNull(); }); }); + +// A private window or blocked site data: every access throws. +describe('browser storage that refuses access', () => { + it('keeps a refused write for the rest of the visit', async () => { + vi.resetModules(); + const blocked = () => { throw new Error('blocked'); }; + vi.stubGlobal('window', {}); + vi.stubGlobal('localStorage', { getItem: blocked, setItem: blocked, removeItem: blocked }); + try { + const storage = await import('../src/lib/storage.js'); + expect(storage.readItem('k')).toBeNull(); + storage.writeItem('k', 'v'); + expect(storage.readItem('k')).toBe('v'); + storage.writeItem('k', null); + expect(storage.readItem('k')).toBeNull(); + } finally { + vi.unstubAllGlobals(); + vi.resetModules(); + } + }); +});