diff --git a/client/src/components/Planner/PlaceFormModal.test.tsx b/client/src/components/Planner/PlaceFormModal.test.tsx index 40fbc45e1..1a0856b64 100644 --- a/client/src/components/Planner/PlaceFormModal.test.tsx +++ b/client/src/components/Planner/PlaceFormModal.test.tsx @@ -1,5 +1,5 @@ -// FE-COMP-PLACEFORM-001 to FE-COMP-PLACEFORM-036, FE-PLANNER-PLACEFORM-016 to FE-PLANNER-PLACEFORM-067, plus FE-PLANNER-PLACEFORM-068 to -080 -import { render, screen, waitFor, fireEvent, within } from '../../../tests/helpers/render'; +// FE-COMP-PLACEFORM-001 to FE-COMP-PLACEFORM-036, FE-PLANNER-PLACEFORM-016 to FE-PLANNER-PLACEFORM-067, plus FE-PLANNER-PLACEFORM-068 to -091 +import { render, screen, waitFor, fireEvent, within, act } from '../../../tests/helpers/render'; import userEvent from '@testing-library/user-event'; import { http, HttpResponse } from 'msw'; import { server } from '../../../tests/helpers/msw/server'; @@ -162,7 +162,7 @@ describe('PlaceFormModal', () => { expect(screen.queryByDisplayValue('Old Place')).not.toBeInTheDocument(); }); - it('clears the maps search input after the modal closes and reopens', async () => { + it('FE-PLANNER-PLACEFORM-088: closing the dialog clears the search field', async () => { const user = userEvent.setup(); const { rerender } = render(); await user.type(screen.getByPlaceholderText('Search places...'), 'Eiffel Tower'); @@ -173,6 +173,77 @@ describe('PlaceFormModal', () => { expect(screen.getByPlaceholderText('Search places...')).toHaveValue(''); }); + it('FE-PLANNER-PLACEFORM-089: closing the dialog drops the result list and its Google line', async () => { + const user = userEvent.setup(); + seedStore(useAuthStore, { user: buildUser(), isAuthenticated: true, hasMapsKey: true }); + server.use( + http.post('/api/maps/search', () => + HttpResponse.json({ places: [{ name: 'Weigh station', address: 'Ritzville', lat: '47.1', lng: '-118.4' }], source: 'trek-places+openstreetmap' }), + ), + ); + const { rerender } = render(); + const searchInput = screen.getByPlaceholderText('Search places...'); + await user.type(searchInput, 'Tokyo Station'); + await user.click(within(searchInput.closest('.flex') as HTMLElement).getByRole('button')); + expect(await screen.findByText('Weigh station')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /Search Google instead/ })).toBeInTheDocument(); + + rerender(); + rerender(); + + expect(screen.getByPlaceholderText('Search places...')).toHaveValue(''); + expect(screen.queryByText('Weigh station')).toBeNull(); + expect(screen.queryByRole('button', { name: /Search Google instead/ })).toBeNull(); + }); + + it('FE-PLANNER-PLACEFORM-090: suggestions still on their way when the dialog closes never open over the next one', async () => { + const user = userEvent.setup(); + const gate: { started: boolean; release?: () => void } = { started: false }; + server.use( + http.post('/api/maps/autocomplete', async () => { + gate.started = true; + await new Promise(res => { gate.release = res; }); + return HttpResponse.json({ suggestions: [{ placeId: 'late-1', mainText: 'Late Suggestion', secondaryText: 'Nowhere' }], source: 'trek-places' }); + }), + ); + const { rerender } = render(); + await user.type(screen.getByPlaceholderText('Search places...'), 'Late'); + await waitFor(() => expect(gate.started).toBe(true)); + + rerender(); + gate.release?.(); + await act(async () => { await new Promise(res => setTimeout(res, 100)); }); + rerender(); + + expect(screen.queryByText('Late Suggestion')).toBeNull(); + }); + + it('FE-PLANNER-PLACEFORM-091: a search that answers after the dialog closed leaves the next opening empty', async () => { + const user = userEvent.setup(); + const gate: { started: boolean; release?: () => void } = { started: false }; + server.use( + http.post('/api/maps/search', async () => { + gate.started = true; + await new Promise(res => { gate.release = res; }); + return HttpResponse.json({ places: [{ name: 'Late Result', address: 'Nowhere', lat: '1', lng: '2' }] }); + }), + ); + const { rerender } = render(); + const searchInput = screen.getByPlaceholderText('Search places...'); + await user.type(searchInput, 'Late'); + await user.click(within(searchInput.closest('.flex') as HTMLElement).getByRole('button')); + await waitFor(() => expect(gate.started).toBe(true)); + + rerender(); + gate.release?.(); + await act(async () => { await new Promise(res => setTimeout(res, 100)); }); + rerender(); + + expect(screen.queryByText('Late Result')).toBeNull(); + // The button is back to its icon rather than the "..." of a search in flight. + expect(within(screen.getByPlaceholderText('Search places...').closest('.flex') as HTMLElement).getByRole('button')).not.toHaveTextContent('...'); + }); + // ── Maps search ────────────────────────────────────────────────────────────── it('FE-PLANNER-PLACEFORM-018: maps search populates results via button click', async () => { diff --git a/client/src/components/Planner/PlaceFormModal.tsx b/client/src/components/Planner/PlaceFormModal.tsx index 97843bb2d..c2eb7086e 100644 --- a/client/src/components/Planner/PlaceFormModal.tsx +++ b/client/src/components/Planner/PlaceFormModal.tsx @@ -191,6 +191,10 @@ function usePlaceFormModal(props: PlaceFormModalProps) { const [acHighlight, setAcHighlight] = useState(-1) const acDebounceRef = useRef | null>(null) const acAbortRef = useRef(null) + // Counts the closings of this dialog, so an answer still on its way when it + // closed can tell that the opening it was asked for is over. A counter rather + // than an abort, because the full search takes no signal. + const searchEpochRef = useRef(0) // Ties one search's keystrokes and its details lookup into a single Google // billing session (see utils/placesSession). const placesSessionRef = useRef(new PlacesSession()) @@ -318,10 +322,29 @@ function usePlaceFormModal(props: PlaceFormModalProps) { // eslint-disable-next-line react-hooks/exhaustive-deps }, [place, prefillCoords, isOpen, assignmentId]) + // The planner keeps this dialog mounted while it is closed (Modal only stops + // drawing it), so everything the search block holds would greet the next + // opening: the last query, its list, the Google line offering that list's + // query again, suggestions still on their way. A pick from that list writes + // over the place being edited, whichever place that is by then. Reset when + // it closes, in an effect of its own: the one above also reruns while the + // dialog is open, when a right-click's reverse lookup fills prefillCoords, + // and must not wipe a search the user has started since. useEffect(() => { - if (!isOpen) { - setMapsSearch('') - } + if (isOpen) return + searchEpochRef.current += 1 + if (acDebounceRef.current) clearTimeout(acDebounceRef.current) + acAbortRef.current?.abort() + placesSessionRef.current.end() + searchMetaRef.current = null + acMetaRef.current = null + setMapsSearch('') + setMapsResults([]) + setSearchSource('') + setAcSuggestions([]) + setAcSource('') + setAcHighlight(-1) + setIsSearchingMaps(false) }, [isOpen]) useEffect(() => { @@ -409,11 +432,13 @@ function usePlaceFormModal(props: PlaceFormModalProps) { // it promises the same query. const trimmed = provider ? (searchMetaRef.current?.query ?? '') : mapsSearch.trim() if (!trimmed) return + const epoch = searchEpochRef.current setIsSearchingMaps(true) try { // A pasted Google Maps or Amap link resolves server-side into a place if (!provider && isMapUrl(trimmed)) { const resolved = await mapsApi.resolveUrl(trimmed) + if (epoch !== searchEpochRef.current) return if (resolved.lat && resolved.lng) { setForm(prev => ({ ...prev, @@ -430,13 +455,15 @@ function usePlaceFormModal(props: PlaceFormModalProps) { } } const result = await mapsApi.search(trimmed, language, locationBiasPoint, provider) + if (epoch !== searchEpochRef.current) return searchMetaRef.current = { query: trimmed, source: result.source || 'unknown' } setMapsResults(result.places || []) setSearchSource(result.source || '') } catch (err: unknown) { + if (epoch !== searchEpochRef.current) return toast.error(getApiErrorMessage(err, t('places.mapsSearchError'))) } finally { - setIsSearchingMaps(false) + if (epoch === searchEpochRef.current) setIsSearchingMaps(false) } } @@ -496,6 +523,7 @@ function usePlaceFormModal(props: PlaceFormModalProps) { setAcSuggestions([]) setAcHighlight(-1) const previousSearch = mapsSearch + const epoch = searchEpochRef.current setMapsSearch('') setForm(prev => ({ ...prev, name: suggestion.mainText })) setIsSearchingMaps(true) @@ -517,6 +545,10 @@ function usePlaceFormModal(props: PlaceFormModalProps) { } catch (err) { console.error('Failed to fetch place details:', err) } + // Closed while the details were on their way: the pick belongs to an + // opening that is over, and the fallback search below is not worth a + // request nobody will see. + if (epoch !== searchEpochRef.current) return if (!place && suggestion.source === 'openstreetmap' && suggestion.lat != null && suggestion.lng != null) { // The layer's rows carry no address; their second line is the name // written on the building. Searching for "Tokio Hauptbahnhof, 東京駅" @@ -535,6 +567,7 @@ function usePlaceFormModal(props: PlaceFormModalProps) { if (!place) { const query = [suggestion.mainText, suggestion.secondaryText].filter(Boolean).join(', ') const search = await mapsApi.search(query, language, locationBiasPoint) + if (epoch !== searchEpochRef.current) return place = search.places?.[0] ?? null } if (place) { @@ -544,12 +577,17 @@ function usePlaceFormModal(props: PlaceFormModalProps) { toast.error(t('places.mapsSearchError')) } } catch (err) { + if (epoch !== searchEpochRef.current) return console.error('Place suggestion lookup failed:', err) setMapsSearch(previousSearch) toast.error(getApiErrorMessage(err, t('places.mapsSearchError'))) } finally { - setIsSearchingMaps(false) - placesSessionRef.current.end() + // Closing already ended this session; ending it again here would cut off + // the one a new opening may have started meanwhile. + if (epoch === searchEpochRef.current) { + setIsSearchingMaps(false) + placesSessionRef.current.end() + } } }