diff --git a/modules/react-mapbox/src/components/marker.ts b/modules/react-mapbox/src/components/marker.ts index 45406b419..c1fbec740 100644 --- a/modules/react-mapbox/src/components/marker.ts +++ b/modules/react-mapbox/src/components/marker.ts @@ -9,6 +9,7 @@ import type {MarkerEvent, MarkerDragEvent} from '../types/events'; import {MapContext} from './map'; import {arePointsEqual} from '../utils/deep-equal'; +import {compareClassNames} from '../utils/compare-class-names'; export type MarkerProps = MarkerOptions & { /** Longitude of the anchor location */ @@ -32,7 +33,6 @@ export const Marker = memo( forwardRef((props: MarkerProps, ref: React.Ref) => { const {map, mapLib} = useContext(MapContext); const thisRef = useRef({props}); - thisRef.current.props = props; const marker: MarkerInstance = useMemo(() => { let hasChildren = false; @@ -102,6 +102,7 @@ export const Marker = memo( useImperativeHandle(ref, () => marker, []); + const oldProps = thisRef.current.props; if (marker.getLngLat().lng !== longitude || marker.getLngLat().lat !== latitude) { marker.setLngLat([longitude, latitude]); } @@ -123,7 +124,14 @@ export const Marker = memo( if (marker.getPopup() !== popup) { marker.setPopup(popup); } + const classNameDiff = compareClassNames(oldProps.className, props.className); + if (classNameDiff) { + for (const c of classNameDiff) { + marker.toggleClassName(c); + } + } + thisRef.current.props = props; return createPortal(props.children, marker.getElement()); }) ); diff --git a/modules/react-mapbox/src/components/popup.ts b/modules/react-mapbox/src/components/popup.ts index babee5ae5..6225c52ac 100644 --- a/modules/react-mapbox/src/components/popup.ts +++ b/modules/react-mapbox/src/components/popup.ts @@ -9,6 +9,7 @@ import type {PopupEvent} from '../types/events'; import {MapContext} from './map'; import {deepEqual} from '../utils/deep-equal'; +import {compareClassNames} from '../utils/compare-class-names'; export type PopupProps = PopupOptions & { /** Longitude of the anchor location */ @@ -24,11 +25,6 @@ export type PopupProps = PopupOptions & { children?: React.ReactNode; }; -// Adapted from https://github.com/mapbox/mapbox-gl-js/blob/v1.13.0/src/ui/popup.js -function getClassList(className: string) { - return new Set(className ? className.trim().split(/\s+/) : []); -} - /* eslint-disable complexity,max-statements */ export const Popup = memo( forwardRef((props: PopupProps, ref: React.Ref) => { @@ -37,7 +33,6 @@ export const Popup = memo( return document.createElement('div'); }, []); const thisRef = useRef({props}); - thisRef.current.props = props; const popup: PopupInstance = useMemo(() => { const options = {...props}; @@ -75,32 +70,24 @@ export const Popup = memo( useImperativeHandle(ref, () => popup, []); if (popup.isOpen()) { + const oldProps = thisRef.current.props; if (popup.getLngLat().lng !== props.longitude || popup.getLngLat().lat !== props.latitude) { popup.setLngLat([props.longitude, props.latitude]); } - if (props.offset && !deepEqual(popup.options.offset, props.offset)) { + if (props.offset && !deepEqual(oldProps.offset, props.offset)) { + popup.options.anchor = props.anchor; popup.setOffset(props.offset); } - if (popup.options.anchor !== props.anchor || popup.options.maxWidth !== props.maxWidth) { - popup.options.anchor = props.anchor; + if (oldProps.anchor !== props.anchor || oldProps.maxWidth !== props.maxWidth) { popup.setMaxWidth(props.maxWidth); } - if (popup.options.className !== props.className) { - const prevClassList = getClassList(popup.options.className); - const nextClassList = getClassList(props.className); - - for (const c of prevClassList) { - if (!nextClassList.has(c)) { - popup.removeClassName(c); - } - } - for (const c of nextClassList) { - if (!prevClassList.has(c)) { - popup.addClassName(c); - } + const classNameDiff = compareClassNames(oldProps.className, props.className); + if (classNameDiff) { + for (const c of classNameDiff) { + popup.toggleClassName(c); } - popup.options.className = props.className; } + thisRef.current.props = props; } return createPortal(props.children, container); diff --git a/modules/react-mapbox/src/utils/compare-class-names.ts b/modules/react-mapbox/src/utils/compare-class-names.ts new file mode 100644 index 000000000..420345b9c --- /dev/null +++ b/modules/react-mapbox/src/utils/compare-class-names.ts @@ -0,0 +1,29 @@ +/** Compare two classNames string and return the difference */ +export function compareClassNames( + prevClassName: string | undefined, + nextClassName: string | undefined +): string[] | null { + if (prevClassName === nextClassName) { + return null; + } + + const prevClassList = getClassList(prevClassName); + const nextClassList = getClassList(nextClassName); + const diff: string[] = []; + + for (const c of nextClassList) { + if (!prevClassList.has(c)) { + diff.push(c); + } + } + for (const c of prevClassList) { + if (!nextClassList.has(c)) { + diff.push(c); + } + } + return diff.length === 0 ? null : diff; +} + +function getClassList(className: string | undefined) { + return new Set(className ? className.trim().split(/\s+/) : []); +} diff --git a/modules/react-mapbox/test/components/marker.spec.jsx b/modules/react-mapbox/test/components/marker.spec.jsx index 15f19b58c..69bfe5068 100644 --- a/modules/react-mapbox/test/components/marker.spec.jsx +++ b/modules/react-mapbox/test/components/marker.spec.jsx @@ -49,6 +49,7 @@ test('Marker', async t => { offset={[0, 1]} rotation={30} draggable + className="classA" pitchAlignment="map" rotationAlignment="map" onDragStart={() => (callbackType = 'dragstart')} @@ -64,6 +65,7 @@ test('Marker', async t => { t.not(rotation, marker.getRotation(), 'rotation is updated'); t.not(pitchAlignment, marker.getPitchAlignment(), 'pitchAlignment is updated'); t.not(rotationAlignment, marker.getRotationAlignment(), 'rotationAlignment is updated'); + t.ok(marker._element.classList.contains('classA'), 'className is updated'); marker.fire('dragstart'); t.is(callbackType, 'dragstart', 'onDragStart called'); diff --git a/modules/react-mapbox/test/components/popup.spec.jsx b/modules/react-mapbox/test/components/popup.spec.jsx index 9028167e9..f1850cfb9 100644 --- a/modules/react-mapbox/test/components/popup.spec.jsx +++ b/modules/react-mapbox/test/components/popup.spec.jsx @@ -67,8 +67,7 @@ test('Popup', async t => { ); await sleep(1); - - t.is(popup.options.className, 'classA', 'className is updated'); + t.ok(popup._container.classList.contains('classA'), 'className is updated'); root.unmount(); t.end(); diff --git a/modules/react-mapbox/test/utils/compare-class-names.spec.js b/modules/react-mapbox/test/utils/compare-class-names.spec.js new file mode 100644 index 000000000..af7c8d50e --- /dev/null +++ b/modules/react-mapbox/test/utils/compare-class-names.spec.js @@ -0,0 +1,52 @@ +import test from 'tape-promise/tape'; +import {compareClassNames} from '@vis.gl/react-mapbox/utils/compare-class-names'; + +test('compareClassNames', t => { + const TEST_CASES = [ + { + title: 'Empty class names', + prevClassName: '', + nextClassName: '', + output: null + }, + { + title: 'Identical class names', + prevClassName: 'marker active', + nextClassName: 'active marker ', + output: null + }, + { + title: 'Addition', + prevClassName: undefined, + nextClassName: 'marker', + output: ['marker'] + }, + { + title: 'Addition', + prevClassName: 'marker', + nextClassName: 'marker active', + output: ['active'] + }, + { + title: 'Removal', + prevClassName: 'marker active', + nextClassName: 'marker', + output: ['active'] + }, + { + title: 'Multiple addition & removal', + prevClassName: 'marker active', + nextClassName: 'marker hovered hidden', + output: ['hovered', 'hidden', 'active'] + } + ]; + + for (const testCase of TEST_CASES) { + t.deepEqual( + compareClassNames(testCase.prevClassName, testCase.nextClassName), + testCase.output, + testCase.title + ); + } + t.end(); +}); diff --git a/modules/react-mapbox/test/utils/index.js b/modules/react-mapbox/test/utils/index.js index 65ae66cec..d455dddc7 100644 --- a/modules/react-mapbox/test/utils/index.js +++ b/modules/react-mapbox/test/utils/index.js @@ -2,3 +2,4 @@ import './deep-equal.spec'; import './transform.spec'; import './style-utils.spec'; import './apply-react-style.spec'; +import './compare-class-names.spec'; diff --git a/modules/react-maplibre/src/components/marker.ts b/modules/react-maplibre/src/components/marker.ts index 45406b419..c1fbec740 100644 --- a/modules/react-maplibre/src/components/marker.ts +++ b/modules/react-maplibre/src/components/marker.ts @@ -9,6 +9,7 @@ import type {MarkerEvent, MarkerDragEvent} from '../types/events'; import {MapContext} from './map'; import {arePointsEqual} from '../utils/deep-equal'; +import {compareClassNames} from '../utils/compare-class-names'; export type MarkerProps = MarkerOptions & { /** Longitude of the anchor location */ @@ -32,7 +33,6 @@ export const Marker = memo( forwardRef((props: MarkerProps, ref: React.Ref) => { const {map, mapLib} = useContext(MapContext); const thisRef = useRef({props}); - thisRef.current.props = props; const marker: MarkerInstance = useMemo(() => { let hasChildren = false; @@ -102,6 +102,7 @@ export const Marker = memo( useImperativeHandle(ref, () => marker, []); + const oldProps = thisRef.current.props; if (marker.getLngLat().lng !== longitude || marker.getLngLat().lat !== latitude) { marker.setLngLat([longitude, latitude]); } @@ -123,7 +124,14 @@ export const Marker = memo( if (marker.getPopup() !== popup) { marker.setPopup(popup); } + const classNameDiff = compareClassNames(oldProps.className, props.className); + if (classNameDiff) { + for (const c of classNameDiff) { + marker.toggleClassName(c); + } + } + thisRef.current.props = props; return createPortal(props.children, marker.getElement()); }) ); diff --git a/modules/react-maplibre/src/components/popup.ts b/modules/react-maplibre/src/components/popup.ts index babee5ae5..173fa39a3 100644 --- a/modules/react-maplibre/src/components/popup.ts +++ b/modules/react-maplibre/src/components/popup.ts @@ -9,6 +9,7 @@ import type {PopupEvent} from '../types/events'; import {MapContext} from './map'; import {deepEqual} from '../utils/deep-equal'; +import {compareClassNames} from '../utils/compare-class-names'; export type PopupProps = PopupOptions & { /** Longitude of the anchor location */ @@ -24,11 +25,6 @@ export type PopupProps = PopupOptions & { children?: React.ReactNode; }; -// Adapted from https://github.com/mapbox/mapbox-gl-js/blob/v1.13.0/src/ui/popup.js -function getClassList(className: string) { - return new Set(className ? className.trim().split(/\s+/) : []); -} - /* eslint-disable complexity,max-statements */ export const Popup = memo( forwardRef((props: PopupProps, ref: React.Ref) => { @@ -37,7 +33,6 @@ export const Popup = memo( return document.createElement('div'); }, []); const thisRef = useRef({props}); - thisRef.current.props = props; const popup: PopupInstance = useMemo(() => { const options = {...props}; @@ -75,32 +70,24 @@ export const Popup = memo( useImperativeHandle(ref, () => popup, []); if (popup.isOpen()) { + const oldProps = thisRef.current.props; if (popup.getLngLat().lng !== props.longitude || popup.getLngLat().lat !== props.latitude) { popup.setLngLat([props.longitude, props.latitude]); } - if (props.offset && !deepEqual(popup.options.offset, props.offset)) { + if (props.offset && !deepEqual(oldProps.offset, props.offset)) { popup.setOffset(props.offset); } - if (popup.options.anchor !== props.anchor || popup.options.maxWidth !== props.maxWidth) { + if (oldProps.anchor !== props.anchor || oldProps.maxWidth !== props.maxWidth) { popup.options.anchor = props.anchor; popup.setMaxWidth(props.maxWidth); } - if (popup.options.className !== props.className) { - const prevClassList = getClassList(popup.options.className); - const nextClassList = getClassList(props.className); - - for (const c of prevClassList) { - if (!nextClassList.has(c)) { - popup.removeClassName(c); - } - } - for (const c of nextClassList) { - if (!prevClassList.has(c)) { - popup.addClassName(c); - } + const classNameDiff = compareClassNames(oldProps.className, props.className); + if (classNameDiff) { + for (const c of classNameDiff) { + popup.toggleClassName(c); } - popup.options.className = props.className; } + thisRef.current.props = props; } return createPortal(props.children, container); diff --git a/modules/react-maplibre/src/utils/compare-class-names.ts b/modules/react-maplibre/src/utils/compare-class-names.ts new file mode 100644 index 000000000..420345b9c --- /dev/null +++ b/modules/react-maplibre/src/utils/compare-class-names.ts @@ -0,0 +1,29 @@ +/** Compare two classNames string and return the difference */ +export function compareClassNames( + prevClassName: string | undefined, + nextClassName: string | undefined +): string[] | null { + if (prevClassName === nextClassName) { + return null; + } + + const prevClassList = getClassList(prevClassName); + const nextClassList = getClassList(nextClassName); + const diff: string[] = []; + + for (const c of nextClassList) { + if (!prevClassList.has(c)) { + diff.push(c); + } + } + for (const c of prevClassList) { + if (!nextClassList.has(c)) { + diff.push(c); + } + } + return diff.length === 0 ? null : diff; +} + +function getClassList(className: string | undefined) { + return new Set(className ? className.trim().split(/\s+/) : []); +} diff --git a/modules/react-maplibre/test/components/marker.spec.jsx b/modules/react-maplibre/test/components/marker.spec.jsx index 57ea6751c..89ce9b8c8 100644 --- a/modules/react-maplibre/test/components/marker.spec.jsx +++ b/modules/react-maplibre/test/components/marker.spec.jsx @@ -48,6 +48,7 @@ test('Marker', async t => { offset={[0, 1]} rotation={30} draggable + className="classA" pitchAlignment="viewport" rotationAlignment="viewport" onDragStart={() => (callbackType = 'dragstart')} @@ -63,6 +64,7 @@ test('Marker', async t => { t.not(rotation, marker.getRotation(), 'rotation is updated'); t.not(pitchAlignment, marker.getPitchAlignment(), 'pitchAlignment is updated'); t.not(rotationAlignment, marker.getRotationAlignment(), 'rotationAlignment is updated'); + t.ok(marker._element.classList.contains('classA'), 'className is updated'); marker.fire('dragstart'); t.is(callbackType, 'dragstart', 'onDragStart called'); diff --git a/modules/react-maplibre/test/components/popup.spec.jsx b/modules/react-maplibre/test/components/popup.spec.jsx index bd711cc59..bf3f4ac27 100644 --- a/modules/react-maplibre/test/components/popup.spec.jsx +++ b/modules/react-maplibre/test/components/popup.spec.jsx @@ -67,8 +67,7 @@ test('Popup', async t => { ); await sleep(1); - - t.is(popup.options.className, 'classA', 'className is updated'); + t.ok(popup._container.classList.contains('classA'), 'className is updated'); root.unmount(); t.end(); diff --git a/modules/react-maplibre/test/utils/compare-class-names.spec.js b/modules/react-maplibre/test/utils/compare-class-names.spec.js new file mode 100644 index 000000000..8bc914ddf --- /dev/null +++ b/modules/react-maplibre/test/utils/compare-class-names.spec.js @@ -0,0 +1,52 @@ +import test from 'tape-promise/tape'; +import {compareClassNames} from '@vis.gl/react-maplibre/utils/compare-class-names'; + +test('compareClassNames', t => { + const TEST_CASES = [ + { + title: 'Empty class names', + prevClassName: '', + nextClassName: '', + output: null + }, + { + title: 'Identical class names', + prevClassName: 'marker active', + nextClassName: 'active marker ', + output: null + }, + { + title: 'Addition', + prevClassName: undefined, + nextClassName: 'marker', + output: ['marker'] + }, + { + title: 'Addition', + prevClassName: 'marker', + nextClassName: 'marker active', + output: ['active'] + }, + { + title: 'Removal', + prevClassName: 'marker active', + nextClassName: 'marker', + output: ['active'] + }, + { + title: 'Multiple addition & removal', + prevClassName: 'marker active', + nextClassName: 'marker hovered hidden', + output: ['hovered', 'hidden', 'active'] + } + ]; + + for (const testCase of TEST_CASES) { + t.deepEqual( + compareClassNames(testCase.prevClassName, testCase.nextClassName), + testCase.output, + testCase.title + ); + } + t.end(); +}); diff --git a/modules/react-maplibre/test/utils/index.js b/modules/react-maplibre/test/utils/index.js index 65ae66cec..d455dddc7 100644 --- a/modules/react-maplibre/test/utils/index.js +++ b/modules/react-maplibre/test/utils/index.js @@ -2,3 +2,4 @@ import './deep-equal.spec'; import './transform.spec'; import './style-utils.spec'; import './apply-react-style.spec'; +import './compare-class-names.spec';