Skip to content

fix: stop mapbox pins from flickering #628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mkouzel-yext merged 4 commits into from
Feb 11, 2026
Merged

fix: stop mapbox pins from flickering #628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c56a47f
fix mapbox pin updates
mkouzel-yext Feb 10, 2026
d2de1dc
Update snapshots
github-actions[bot] Feb 10, 2026
c1c9fa8
fix bug
mkouzel-yext Feb 10, 2026
4ce7434
Merge branch 'main' into prevent-pin-flickering
mkouzel-yext Feb 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
181 changes: 128 additions & 53 deletions src/components/MapboxMap.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ export function MapboxMap<T>({
markerOptionsOverride,
}: MapboxMapProps<T>): React.JSX.Element {
const mapboxInstance = (iframeWindow as Window & { mapboxgl?: typeof mapboxgl })?.mapboxgl ?? mapboxgl;
// keep the mapbox access token in sync with prop changes.
useEffect(() => {
mapboxInstance.accessToken = mapboxAccessToken;
}, [mapboxAccessToken, mapboxInstance]);
Expand All @@ -175,6 +176,7 @@ export function MapboxMap<T>({
setSelectedResult(prev => prev === result ? undefined : result);
}, []);

// notify consumers when the selected pin changes.
useEffect(() => {
onPinClick?.(selectedResult);
}, [onPinClick, selectedResult]);
Expand All @@ -192,6 +194,8 @@ export function MapboxMap<T>({
if (supportsCreateRoot) {
const root = markerRoots.current.get(element);
if (root) {
// unmount must be called after the current render finishes, so schedule it for the next
// microtask
scheduleRootUnmount(root);
markerRoots.current.delete(element);
}
Expand All @@ -215,12 +219,74 @@ export function MapboxMap<T>({
}
}, []);

// builds and attaches a single marker to the mapbox map
const createMarker = useCallback((
mapbox: mapboxgl.Map,
result: Result<T>,
index: number,
selected: boolean
) => {
const markerLocation = getCoordinate(result);
if (!markerLocation) {
return null;
}
const { latitude, longitude } = markerLocation;
if (!Number.isFinite(latitude) || !Number.isFinite(longitude)) {
return null;
}

const el = document.createElement('div');
let markerOptions: mapboxgl.MarkerOptions = {};
if (PinComponent) {
if (renderPin) {
console.warn(
'Found both PinComponent and renderPin props. Using PinComponent.'
);
}
attachPinComponent(el, (
<PinComponent
index={index}
mapbox={mapbox}
result={result}
selected={selected}
/>
));
markerOptions.element = el;
} else if (renderPin) {
renderPin({ index, mapbox, result, container: el });
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The renderPin function call is missing the selected parameter. According to the PinComponentProps type definition (line 38), the selected property should be passed to renderPin. This should be: renderPin({ index, mapbox, result, selected, container: el })

Suggested change
renderPin({ index, mapbox, result, container: el });
renderPin({ index, mapbox, result, selected, container: el });

Copilot uses AI. Check for mistakes.
markerOptions.element = el;
}

if (markerOptionsOverride) {
markerOptions = {
...markerOptions,
...markerOptionsOverride(selected)
};
}

const marker = new mapboxInstance.Marker(markerOptions)
.setLngLat({ lat: latitude, lng: longitude })
.addTo(mapbox);

marker?.getElement().addEventListener('click', () => handlePinClick(result));
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The optional chaining marker?.getElement() will return undefined if marker is falsy, and then .addEventListener() will throw an error. Either remove the optional chaining on marker (since marker should always be defined here), or use the pattern marker.getElement()?.addEventListener() or const element = marker?.getElement?.(); if (element) { element.addEventListener(...) }

Suggested change
marker?.getElement().addEventListener('click', () => handlePinClick(result));
marker.getElement()?.addEventListener('click', () => handlePinClick(result));

Copilot uses AI. Check for mistakes.

return { marker, location: markerLocation };
}, [
PinComponent,
attachPinComponent,
getCoordinate,
handlePinClick,
mapboxInstance,
markerOptionsOverride,
renderPin
]);

const removeMarkers = useCallback(() => {
markers.current.forEach(marker => {
if (!marker) {
return;
}
const element = typeof marker.getElement === 'function' ? marker.getElement() : null;
const element = marker?.getElement?.();
if (element) {
cleanupPinComponent(element);
}
Expand Down Expand Up @@ -280,6 +346,7 @@ export function MapboxMap<T>({
}
}, [locale]);

// initialize the map once and update mapbox options when allowUpdates is true.
useEffect(() => {
if (mapContainer.current) {
if (map.current && allowUpdates) {
Expand Down Expand Up @@ -332,70 +399,36 @@ export function MapboxMap<T>({
}
}, [allowUpdates, mapboxInstance, mapboxOptions, onDragDebounced, localizeMap]);

// resize the map when its iframe container changes size.
useEffect(() => {
if (iframeWindow && map.current) {
map.current.resize();
}
}, [iframeWindow]);

// create and place markers when results change, then cleanup on teardown
useEffect(() => {
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When locationResults changes, the selectedResult effects (lines 506-547 and 549-569) may run with stale markerData.current before the new markers are created. Consider resetting previousSelectedResult.current to undefined in the locationResults effect cleanup function, or at the start of the locationResults effect, to prevent attempting to update markers that no longer exist.

Suggested change
useEffect(() => {
useEffect(() => {
// Reset previousSelectedResult to avoid referencing markers that have been removed.
previousSelectedResult.current = undefined;

Copilot uses AI. Check for mistakes.
removeMarkers();
const mapbox = map.current;
if (mapbox && locationResults) {
if (locationResults.length > 0) {
const bounds = new mapboxInstance.LngLatBounds();
// create a marker for each result
locationResults.forEach((result, i) => {
const markerLocation = getCoordinate(result);
if (markerLocation) {
const { latitude, longitude } = markerLocation;
if (!Number.isFinite(latitude) || !Number.isFinite(longitude)) {
return;
}
const el = document.createElement('div');
let markerOptions: mapboxgl.MarkerOptions = {};
if (PinComponent) {
if (renderPin) {
console.warn(
'Found both PinComponent and renderPin props. Using PinComponent.'
);
}
attachPinComponent(el, (
<PinComponent
index={i}
mapbox={mapbox}
result={result}
selected={selectedResult === result}
/>
));
markerOptions.element = el;
} else if (renderPin) {
renderPin({ index: i, mapbox, result, container: el });
markerOptions.element = el;
}

if (markerOptionsOverride) {
markerOptions = {
...markerOptions,
...markerOptionsOverride(selectedResult === result)
};
}

const marker = new mapboxInstance.Marker(markerOptions)
.setLngLat({ lat: latitude, lng: longitude })
.addTo(mapbox);

marker?.getElement().addEventListener('click', () => handlePinClick(result));
markers.current.push(marker);
if (marker) {
markerData.current.push({ marker, result, index: i });
}
bounds.extend([longitude, latitude]);
const created = createMarker(mapbox, result, i, false);
if (!created) {
return;
}
markers.current.push(created.marker);
markerData.current.push({ marker: created.marker, result, index: i });
bounds.extend([created.location.longitude, created.location.latitude]);
});

// fit the map to the markers
mapbox.resize();
const canvas = mapbox.getCanvas();

// add padding to map
if (!bounds.isEmpty()
&& !!canvas
&& canvas.clientHeight > 0
Expand Down Expand Up @@ -442,6 +475,7 @@ export function MapboxMap<T>({
mapbox.fitBounds(bounds, resolvedOptions);
}

// return a cleanup function to remove markers when the map component unmounts
return () => {
markers.current.forEach((marker, i) => {
marker?.getElement().removeEventListener('click', () => handlePinClick(locationResults[i]));
Expand All @@ -458,27 +492,68 @@ export function MapboxMap<T>({
}
}
}, [
PinComponent,
attachPinComponent,
getCoordinate,
createMarker,
handlePinClick,
locationResults,
mapboxInstance,
mapboxOptions,
markerOptionsOverride,
removeMarkers,
renderPin,
selectedResult,
staticFilters
]);

const previousSelectedResult = useRef<Result<T> | undefined>(undefined);

// update marker options when markerOptionsOverride changes or selectedResult changes
useEffect(() => {
const mapbox = map.current;
if (!mapbox || !markerOptionsOverride) {
previousSelectedResult.current = selectedResult;
return;
}

const prevSelected = previousSelectedResult.current;
previousSelectedResult.current = selectedResult;

// markerOptionsOverride is applied at creation time, so we recreate only the affected
// markers to reflect selection changes without tearing down all pins.
const resultsToUpdate = new Set<Result<T>>();
if (prevSelected) {
resultsToUpdate.add(prevSelected);
}
if (selectedResult) {
resultsToUpdate.add(selectedResult);
}

resultsToUpdate.forEach((result) => {
const markerEntry = markerData.current.find(entry => entry.result === result);
if (!markerEntry) {
return;
}
// recreate the marker to apply new markerOptionsOverride (e.g. color/scale).
const oldMarker = markerEntry.marker;
const element = oldMarker?.getElement?.();
if (element) {
cleanupPinComponent(element);
}
oldMarker?.remove?.();

const created = createMarker(mapbox, result, markerEntry.index, selectedResult === result);
if (!created) {
return;
}
markerEntry.marker = created.marker;
markers.current[markerEntry.index] = created.marker;
});
}, [cleanupPinComponent, createMarker, markerOptionsOverride, selectedResult]);

// re-render custom PinComponent on selection changes to update the visual state
useEffect(() => {
const mapbox = map.current;
if (!mapbox || !PinComponent) {
return;
}
markerData.current.forEach(({ marker, result, index }) => {
const element = typeof marker.getElement === 'function' ? marker.getElement() : null;
const element = marker?.getElement?.();
if (!element) {
return;
}
Expand Down
19 changes: 16 additions & 3 deletions test-site/src/components/MapPin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useEffect, useRef, useState, useCallback } from 'react';
import React, { useEffect, useRef, useState, useCallback, useMemo } from 'react';
import { Popup, LngLatLike, Map } from 'mapbox-gl';
import { PinComponent, Coordinate } from '@yext/search-ui-react';
import { Location } from '../pages/LocationsPage';
Expand All @@ -9,7 +9,7 @@ const transformToMapboxCoord = (coordinate: Coordinate): LngLatLike => ({
});

export const MapPin: PinComponent<Location> = props => {
const { mapbox, result } = props;
const { mapbox, result, selected } = props;
const yextCoordinate = result.rawData.yextDisplayCoordinate;
const [active, setActive] = useState(false);
const popupRef = useRef(new Popup({ offset: 15 })
Expand All @@ -29,10 +29,23 @@ export const MapPin: PinComponent<Location> = props => {
setActive(true);
}, []);

const { width, height } = useMemo(() => {
return selected
? {
width: 49.5,
height: 63
}
: {
width: 33,
height: 42
};
}, [selected]);

return (
<button onClick={handleClick} aria-label='Show pin details'>
<svg
width="33" height="42"
width={width}
height={height}
viewBox="0 0 30 38"
fill="#1e293b"
stroke="#fff"
Expand Down
1 change: 1 addition & 0 deletions test-site/src/pages/LocationsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ export function LocationsPage() {
<MapboxMap
mapboxAccessToken={process.env.REACT_APP_MAPBOX_API_KEY || 'REPLACE_KEY'}
mapboxOptions={mapboxOptions}
// markerOptionsOverride={markerOptionsOverride}
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commented-out code should be removed before merging. If it's needed for future reference or testing, consider documenting why it's being kept or move it to a separate branch/documentation.

Copilot uses AI. Check for mistakes.
PinComponent={MapPin}
onDrag={onDrag}
/>
Expand Down
Loading