diff --git a/src/features/webhooks/human/Location.jsx b/src/features/webhooks/human/Location.jsx index 01058f2d1..b2abe35c4 100644 --- a/src/features/webhooks/human/Location.jsx +++ b/src/features/webhooks/human/Location.jsx @@ -42,9 +42,28 @@ const Location = () => { const [save] = useMutation(SET_HUMAN, { fetchPolicy: 'no-cache' }) + // The locate control watches the device position, so `locationfound` keeps + // firing for as long as it is active. Only consume the first event after the + // user explicitly asks for their location, otherwise a background GPS tick + // would silently overwrite a location they set on purpose. + const awaitingLocate = React.useRef(false) + + // Search, My Location, and the map picker call `handleLocationChange` + // directly, which both writes the store and POSTs. That store write also + // re-fires the `webhookLocation` sync effect below (which exists for the + // drag picker, whose only signal is the store). Remember the coordinates we + // are already persisting so the effect can skip a duplicate save. + /** @type {React.MutableRefObject<[number, number] | null>} */ + const savingLocation = React.useRef(null) + /** @param {[number, number]} location */ const handleLocationChange = (location) => { if (location.every((x) => x !== 0)) { + // Whatever we are about to save supersedes a locate request that has not + // come back yet, otherwise a slow GPS fix would land on top of a pick the + // user made while waiting for it. + awaitingLocate.current = false + savingLocation.current = location useWebhookStore.setState((prev) => ({ location: prev.location.some((x, i) => x !== location[i]) ? [location[0] ?? 0, location[1] ?? 0] @@ -56,32 +75,64 @@ const Location = () => { data: location, status: 'POST', }, - }).then(({ data: newData }) => { - if (newData?.webhook) { - useWebhookStore.setState({ human: newData.webhook.human }) - } }) + .then(({ data: newData }) => { + if (newData?.webhook) { + useWebhookStore.setState({ human: newData.webhook.human }) + } + }) + .finally(() => { + // Only clear if a newer save has not already claimed the ref, so an + // in-flight save can't wipe the marker for the location after it. + if ( + savingLocation.current && + savingLocation.current[0] === location[0] && + savingLocation.current[1] === location[1] + ) { + savingLocation.current = null + } + }) } } const map = useMapEvents({ locationfound: (newLoc) => { + if (!awaitingLocate.current) return + awaitingLocate.current = false const { location } = useWebhookStore.getState() const { lat, lng } = newLoc.latlng - if (lat !== location[0] && lng !== location[1]) { + if (lat !== location[0] || lng !== location[1]) { handleLocationChange([lat, lng]) } }, }) React.useEffect(() => { - if (webhookLocation[0] !== latitude && webhookLocation[1] !== longitude) { + const saving = savingLocation.current + if ( + saving && + saving[0] === webhookLocation[0] && + saving[1] === webhookLocation[1] + ) { + // `handleLocationChange` already saved these exact coordinates and only + // updated the store, which is what re-triggered this effect. The drag + // picker, by contrast, writes the store without saving and relies on the + // POST below. + return + } + if (webhookLocation[0] !== latitude || webhookLocation[1] !== longitude) { handleLocationChange(webhookLocation) } }, [webhookLocation]) React.useEffect(() => { - if (webhookLocation.every((x) => x === 0)) { + // `human` is empty until the query resolves, so wait for real coordinates + // rather than seeding the store with undefined. + if ( + typeof latitude === 'number' && + typeof longitude === 'number' && + webhookLocation.every((x) => x === 0) + ) { useWebhookStore.setState({ location: [latitude, longitude] }) } }, [latitude, longitude]) @@ -120,7 +171,12 @@ const Location = () => { size="small" variant="contained" color={color} - onClick={() => lc._onClick()} + onClick={() => { + lc._onClick() + // `_onClick` toggles: if that turned the control off, no + // `locationfound` is coming and the request must not stay armed. + awaitingLocate.current = !!lc._active + }} startIcon={} > {t('my_location')} diff --git a/src/hooks/useLocation.js b/src/hooks/useLocation.js index 58bdf1308..cec0b31be 100644 --- a/src/hooks/useLocation.js +++ b/src/hooks/useLocation.js @@ -13,7 +13,7 @@ import { RecoveringLocateControl } from '@utils/locateControl' /** * Use location hook - * @returns {{ lc: import('leaflet.locatecontrol').LocateControl & { _onClick: () => void }, requesting: boolean, color: import('@mui/material').ButtonProps['color'], locationError: { show: boolean, message: string }, hideLocationError: () => void }} + * @returns {{ lc: import('leaflet.locatecontrol').LocateControl & { _onClick: () => void, _active?: boolean }, requesting: boolean, color: import('@mui/material').ButtonProps['color'], locationError: { show: boolean, message: string }, hideLocationError: () => void }} */ export function useLocation(dependency = false) { const map = useMap()