From a66f70031cf5f21ba7d0fa75d7ca62a85b8d56c7 Mon Sep 17 00:00:00 2001 From: Renzo Beux Date: Mon, 3 Aug 2026 21:32:34 -0300 Subject: [PATCH] Wire the current-location button into the mobile entry sheet - MJourneyEntrySheet renders the same crosshair button in its location field, with the coordinate fallback and reverse-geocode race guard - Quick-capture auto-locate now goes through getCurrentPositionOnce(), replacing the raw navigator.geolocation call and the untranslated error.message it surfaced - Error code to i18n key mapping extracted to geoOnceErrorKey(), shared by both editors - Test for the race guard: a search result picked while the reverse geocode is in flight must survive --- .../Journey/JourneyDetailPageEntryEditor.tsx | 11 +- .../screens/journey/MJourneyEntrySheet.tsx | 101 ++++++++++++------ client/src/pages/JourneyDetailPage.test.tsx | 54 ++++++++++ .../JourneyDetailPage.helpers.ts | 11 ++ 4 files changed, 138 insertions(+), 39 deletions(-) diff --git a/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx b/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx index accb1d687..46e2f188b 100644 --- a/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx +++ b/client/src/components/Journey/JourneyDetailPageEntryEditor.tsx @@ -5,11 +5,11 @@ import { type ResilientResult, type UploadProgress } from '../../utils/uploadQue import { useTranslation } from '../../i18n' import { journeyApi, mapsApi, addonsApi } from '../../api/client' import { useToast } from '../shared/Toast' -import { getCurrentPositionOnce, GeoOnceError } from '../../hooks/useGeolocation' +import { getCurrentPositionOnce } from '../../hooks/useGeolocation' import { getApiErrorMessage } from '../../types' import type { JourneyEntry, JourneyPhoto, GalleryPhoto, JourneyTrip } from '../../store/journeyStore' import { MOOD_CONFIG, WEATHER_CONFIG } from '../../pages/journeyDetail/JourneyDetailPage.constants' -import { photoUrl, isValidGeoPoint } from '../../pages/journeyDetail/JourneyDetailPage.helpers' +import { photoUrl, isValidGeoPoint, geoOnceErrorKey } from '../../pages/journeyDetail/JourneyDetailPage.helpers' import MarkdownToolbar from './MarkdownToolbar' import { DatePicker } from './JourneyDetailPageDatePicker' import { ProviderPicker, type ProviderPhotoGroup } from './JourneyDetailPageProviderPicker' @@ -222,12 +222,7 @@ export function EntryEditor({ entry, journeyId, tripDates, galleryPhotos, trips, if (name) setLocationName(prev => (prev === fallbackName ? name : prev)) } catch { /* best effort — keep the coordinate fallback */ } } catch (err) { - const code = err instanceof GeoOnceError ? err.code : 'unavailable' - toast.error( - code === 'permission-denied' ? t('journey.editor.locationPermissionDenied') - : code === 'timeout' ? t('journey.editor.locationTimeout') - : code === 'insecure-context' ? t('journey.editor.locationInsecureContext') - : t('journey.editor.locationUnavailable')) + toast.error(t(geoOnceErrorKey(err))) } finally { setLocating(false) } diff --git a/client/src/mobile/screens/journey/MJourneyEntrySheet.tsx b/client/src/mobile/screens/journey/MJourneyEntrySheet.tsx index 64a78fe14..c8ebbf3b7 100644 --- a/client/src/mobile/screens/journey/MJourneyEntrySheet.tsx +++ b/client/src/mobile/screens/journey/MJourneyEntrySheet.tsx @@ -1,5 +1,5 @@ import { useEffect, useRef, useState } from 'react' -import { Camera, Plus, Image, X, MapPin, Trash2, CheckCircle2, MinusCircle } from 'lucide-react' +import { Camera, Plus, Image, X, MapPin, Locate, Trash2, CheckCircle2, MinusCircle } from 'lucide-react' import MSheet from '../../components/MSheet' import MIconBtn from '../../components/MIconBtn' import { useTranslation } from '../../../i18n' @@ -7,9 +7,10 @@ import { useToast } from '../../../components/shared/Toast' import { journeyApi, mapsApi, weatherApi } from '../../../api/client' import { getApiErrorMessage } from '../../../types' import { normalizeImageFiles } from '../../../utils/convertHeic' +import { getCurrentPositionOnce } from '../../../hooks/useGeolocation' import type { ResilientResult, UploadProgress } from '../../../utils/uploadQueue' import type { JourneyEntry, JourneyPhoto, GalleryPhoto } from '../../../store/journeyStore' -import { photoUrl } from '../../../pages/journeyDetail/JourneyDetailPage.helpers' +import { photoUrl, geoOnceErrorKey } from '../../../pages/journeyDetail/JourneyDetailPage.helpers' import JournalBody from '../../../components/Journey/JournalBody' import { journeyWeatherCategory, MOBILE_MOODS, MOBILE_WEATHERS } from './mobileJourneyMeta' @@ -43,7 +44,7 @@ interface MJourneyEntrySheetProps { export default function MJourneyEntrySheet({ entry, galleryPhotos, quickCapture = false, readOnly = false, onClose, onSave, onUploadPhotos, onDelete, onDone, }: MJourneyEntrySheetProps) { - const { t } = useTranslation() + const { t, language } = useTranslation() const toast = useToast() const [title, setTitle] = useState(entry.title || '') @@ -77,40 +78,35 @@ export default function MJourneyEntrySheet({ useEffect(() => { if (!quickCapture || readOnly || entry.location_lat != null || entry.location_lng != null) return - if (!navigator.geolocation) { - setLocationError(t('common.error')) - return - } let active = true setLocating(true) - navigator.geolocation.getCurrentPosition(async position => { - const lat = position.coords.latitude - const lng = position.coords.longitude - if (!active) return - setLocationLat(lat) - setLocationLng(lng) + getCurrentPositionOnce({ enableHighAccuracy: true, maximumAge: 60_000, timeout: 10_000 }) + .then(async pos => { + if (!active) return + setLocationLat(pos.lat) + setLocationLng(pos.lng) - const [placeResult, weatherResult] = await Promise.allSettled([ - mapsApi.reverse(lat, lng), - weatherApi.getCurrent(lat, lng, 'en'), - ]) - if (!active) return - if (placeResult.status === 'fulfilled') { - setLocationName(placeResult.value.name || placeResult.value.address || '') - } - if (weatherResult.status === 'fulfilled' && !weatherResult.value.error) { - setWeather(current => current || journeyWeatherCategory(weatherResult.value.main, weatherResult.value.description)) - } - setLocating(false) - }, error => { - if (!active) return - setLocationError(error.message || t('common.error')) - setLocating(false) - }, { enableHighAccuracy: true, maximumAge: 60_000, timeout: 10_000 }) + const [placeResult, weatherResult] = await Promise.allSettled([ + mapsApi.reverse(pos.lat, pos.lng, language), + weatherApi.getCurrent(pos.lat, pos.lng, 'en'), + ]) + if (!active) return + if (placeResult.status === 'fulfilled') { + setLocationName(placeResult.value.name || placeResult.value.address || '') + } + if (weatherResult.status === 'fulfilled' && !weatherResult.value.error) { + setWeather(current => current || journeyWeatherCategory(weatherResult.value.main, weatherResult.value.description)) + } + setLocating(false) + }, err => { + if (!active) return + setLocationError(t(geoOnceErrorKey(err))) + setLocating(false) + }) return () => { active = false } - }, [quickCapture, readOnly, entry.location_lat, entry.location_lng, entry.entry_date, t]) + }, [quickCapture, readOnly, entry.location_lat, entry.location_lng, entry.entry_date, t, language]) const isDirty = title !== (entry.title || '') || @@ -205,6 +201,36 @@ export default function MJourneyEntrySheet({ }, 400) } + const handleUseCurrentLocation = async () => { + if (locating) return + setLocating(true) + setLocationError('') + try { + const pos = await getCurrentPositionOnce() + // Fill coordinates right away; the name is refined below once the + // reverse geocode comes back. + const fallbackName = `${pos.lat.toFixed(5)}, ${pos.lng.toFixed(5)}` + if (locationTimerRef.current) clearTimeout(locationTimerRef.current) + setLocationLat(pos.lat) + setLocationLng(pos.lng) + setLocationName(fallbackName) + setLocationQuery('') + setLocationResults([]) + setShowLocationResults(false) + try { + const data = await mapsApi.reverse(pos.lat, pos.lng, language) + const name = data.name || data.address + // Only replace the coordinate fallback — don't clobber a search + // result the user may have picked while the reverse call was in flight. + if (name) setLocationName(prev => (prev === fallbackName ? name : prev)) + } catch { /* best effort — keep the coordinate fallback */ } + } catch (err) { + setLocationError(t(geoOnceErrorKey(err))) + } finally { + setLocating(false) + } + } + const addTag = () => { const value = tagInput.trim().replace(/,+$/, '') if (!value) return @@ -510,6 +536,19 @@ export default function MJourneyEntrySheet({ className="min-w-0 flex-1 bg-transparent font-geist text-[0.75rem] text-m-ink outline-none placeholder:text-m-faint" /> {locationLat != null && } + {!readOnly && ( + + )} {showLocationResults && locationResults.length > 0 && (
diff --git a/client/src/pages/JourneyDetailPage.test.tsx b/client/src/pages/JourneyDetailPage.test.tsx index 58a828b98..ffc8c39e9 100644 --- a/client/src/pages/JourneyDetailPage.test.tsx +++ b/client/src/pages/JourneyDetailPage.test.tsx @@ -3940,5 +3940,59 @@ describe('JourneyDetailPage', () => { ); }); }); + + it('keeps a search result picked while the reverse geocode is in flight', async () => { + stubGeoSuccess(); + let releaseReverse!: () => void; + const reverseGate = new Promise(resolve => { releaseReverse = resolve; }); + let reverseReturned = false; + server.use( + http.get('/api/maps/reverse', async () => { + await reverseGate; + reverseReturned = true; + return HttpResponse.json({ name: 'Colosseum', address: 'Rome, Italy' }); + }), + http.post('/api/maps/search', () => { + return HttpResponse.json({ + places: [{ name: 'Vatican City', address: 'Vatican, Rome', lat: 41.9, lng: 12.45 }], + source: 'osm', + }); + }), + ); + + const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }); + await renderAndWait(); + await openEntryEditor(user); + + await user.click(screen.getByRole('button', { name: 'Use my current location' })); + + const locationInput = screen.getByPlaceholderText('Search location...'); + await waitFor(() => { + expect(locationInput).toHaveValue('41.90000, 12.50000'); + }); + + // With the reverse geocode still pending, search for and pick a place + await user.type(locationInput, 'Vatican'); + vi.advanceTimersByTime(500); + await waitFor(() => { + expect(screen.getByText('Vatican City')).toBeInTheDocument(); + }); + await user.click(screen.getByText('Vatican City')); + await waitFor(() => { + expect(locationInput).toHaveValue('Vatican City'); + }); + + // Now let the reverse geocode land; it must not overwrite the pick. + // If the guard regresses, the input flips to 'Colosseum' and the + // inner waitFor resolves, failing the rejects assertion. + releaseReverse(); + await waitFor(() => { + expect(reverseReturned).toBe(true); + }); + await expect( + waitFor(() => expect(locationInput).toHaveValue('Colosseum'), { timeout: 300 }), + ).rejects.toThrow(); + expect(locationInput).toHaveValue('Vatican City'); + }); }); }); diff --git a/client/src/pages/journeyDetail/JourneyDetailPage.helpers.ts b/client/src/pages/journeyDetail/JourneyDetailPage.helpers.ts index daf268d77..d65021488 100644 --- a/client/src/pages/journeyDetail/JourneyDetailPage.helpers.ts +++ b/client/src/pages/journeyDetail/JourneyDetailPage.helpers.ts @@ -1,5 +1,16 @@ import type { JourneyEntry } from '../../store/journeyStore' import { GRADIENTS } from './JourneyDetailPage.constants' +import { GeoOnceError } from '../../hooks/useGeolocation' + +// Shared by the desktop entry editor and the mobile entry sheet so a failed +// one-shot position fix surfaces the same translated message everywhere. +export function geoOnceErrorKey(err: unknown): string { + const code = err instanceof GeoOnceError ? err.code : 'unavailable' + return code === 'permission-denied' ? 'journey.editor.locationPermissionDenied' + : code === 'timeout' ? 'journey.editor.locationTimeout' + : code === 'insecure-context' ? 'journey.editor.locationInsecureContext' + : 'journey.editor.locationUnavailable' +} export function pickGradient(id: number): string { return GRADIENTS[id % GRADIENTS.length]