From 76becce05b032eaa9c2d782acb75c28baf19c585 Mon Sep 17 00:00:00 2001 From: Xiaoji Chen Date: Tue, 4 Feb 2025 11:21:40 -0800 Subject: [PATCH 1/2] Handle class name change --- modules/react-mapbox/src/components/marker.ts | 13 ++++- modules/react-mapbox/src/components/popup.ts | 33 ++++-------- .../src/utils/compare-class-names.ts | 36 +++++++++++++ .../test/utils/compare-class-names.spec.js | 52 +++++++++++++++++++ modules/react-mapbox/test/utils/index.js | 1 + .../react-maplibre/src/components/marker.ts | 13 ++++- .../react-maplibre/src/components/popup.ts | 33 ++++-------- .../src/utils/compare-class-names.ts | 36 +++++++++++++ .../test/utils/compare-class-names.spec.js | 52 +++++++++++++++++++ modules/react-maplibre/test/utils/index.js | 1 + 10 files changed, 224 insertions(+), 46 deletions(-) create mode 100644 modules/react-mapbox/src/utils/compare-class-names.ts create mode 100644 modules/react-mapbox/test/utils/compare-class-names.spec.js create mode 100644 modules/react-maplibre/src/utils/compare-class-names.ts create mode 100644 modules/react-maplibre/test/utils/compare-class-names.spec.js diff --git a/modules/react-mapbox/src/components/marker.ts b/modules/react-mapbox/src/components/marker.ts index 45406b419..bdec59b70 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,17 @@ 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[0]) { + marker.addClassName(c); + } + for (const c of classNameDiff[1]) { + marker.removeClassName(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..ab86e3369 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,26 @@ 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) { - 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); - } + const classNameDiff = compareClassNames(oldProps.className, props.className); + if (classNameDiff) { + for (const c of classNameDiff[0]) { + popup.addClassName(c); } - for (const c of nextClassList) { - if (!prevClassList.has(c)) { - popup.addClassName(c); - } + for (const c of classNameDiff[1]) { + popup.removeClassName(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..6a77c9104 --- /dev/null +++ b/modules/react-mapbox/src/utils/compare-class-names.ts @@ -0,0 +1,36 @@ +/** Compare two classNames string and return the difference */ +export function compareClassNames( + prevClassName: string | undefined, + nextClassName: string | undefined +): [added: string[], removed: string[]] | null { + if (prevClassName === nextClassName) { + return null; + } + + const prevClassList = getClassList(prevClassName); + const nextClassList = getClassList(nextClassName); + const added: string[] = []; + const removed: string[] = []; + + for (const c of prevClassList) { + if (!nextClassList.has(c)) { + removed.push(c); + } + } + + if (removed.length === 0 && prevClassList.size === nextClassList.size) { + return null; + } + + for (const c of nextClassList) { + if (!prevClassList.has(c)) { + added.push(c); + } + } + + return [added, removed]; +} + +function getClassList(className: string | undefined) { + return new Set(className ? className.trim().split(/\s+/) : []); +} 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..8b6f0e1d0 --- /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..bdec59b70 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,17 @@ 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[0]) { + marker.addClassName(c); + } + for (const c of classNameDiff[1]) { + marker.removeClassName(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..ab86e3369 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,26 @@ 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) { - 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); - } + const classNameDiff = compareClassNames(oldProps.className, props.className); + if (classNameDiff) { + for (const c of classNameDiff[0]) { + popup.addClassName(c); } - for (const c of nextClassList) { - if (!prevClassList.has(c)) { - popup.addClassName(c); - } + for (const c of classNameDiff[1]) { + popup.removeClassName(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..6a77c9104 --- /dev/null +++ b/modules/react-maplibre/src/utils/compare-class-names.ts @@ -0,0 +1,36 @@ +/** Compare two classNames string and return the difference */ +export function compareClassNames( + prevClassName: string | undefined, + nextClassName: string | undefined +): [added: string[], removed: string[]] | null { + if (prevClassName === nextClassName) { + return null; + } + + const prevClassList = getClassList(prevClassName); + const nextClassList = getClassList(nextClassName); + const added: string[] = []; + const removed: string[] = []; + + for (const c of prevClassList) { + if (!nextClassList.has(c)) { + removed.push(c); + } + } + + if (removed.length === 0 && prevClassList.size === nextClassList.size) { + return null; + } + + for (const c of nextClassList) { + if (!prevClassList.has(c)) { + added.push(c); + } + } + + return [added, removed]; +} + +function getClassList(className: string | undefined) { + return new Set(className ? className.trim().split(/\s+/) : []); +} 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..057cf12a9 --- /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'; From 706dac4845b8a2de9939d9cbfb22d8dfdc0d9079 Mon Sep 17 00:00:00 2001 From: Xiaoji Chen Date: Tue, 4 Feb 2025 11:44:53 -0800 Subject: [PATCH 2/2] simplify --- modules/react-mapbox/src/components/marker.ts | 7 ++---- modules/react-mapbox/src/components/popup.ts | 8 +++--- .../src/utils/compare-class-names.ts | 25 +++++++------------ .../test/components/marker.spec.jsx | 2 ++ .../test/components/popup.spec.jsx | 3 +-- .../test/utils/compare-class-names.spec.js | 8 +++--- .../react-maplibre/src/components/marker.ts | 7 ++---- .../react-maplibre/src/components/popup.ts | 8 +++--- .../src/utils/compare-class-names.ts | 25 +++++++------------ .../test/components/marker.spec.jsx | 2 ++ .../test/components/popup.spec.jsx | 3 +-- .../test/utils/compare-class-names.spec.js | 8 +++--- 12 files changed, 42 insertions(+), 64 deletions(-) diff --git a/modules/react-mapbox/src/components/marker.ts b/modules/react-mapbox/src/components/marker.ts index bdec59b70..c1fbec740 100644 --- a/modules/react-mapbox/src/components/marker.ts +++ b/modules/react-mapbox/src/components/marker.ts @@ -126,11 +126,8 @@ export const Marker = memo( } const classNameDiff = compareClassNames(oldProps.className, props.className); if (classNameDiff) { - for (const c of classNameDiff[0]) { - marker.addClassName(c); - } - for (const c of classNameDiff[1]) { - marker.removeClassName(c); + for (const c of classNameDiff) { + marker.toggleClassName(c); } } diff --git a/modules/react-mapbox/src/components/popup.ts b/modules/react-mapbox/src/components/popup.ts index ab86e3369..6225c52ac 100644 --- a/modules/react-mapbox/src/components/popup.ts +++ b/modules/react-mapbox/src/components/popup.ts @@ -75,6 +75,7 @@ export const Popup = memo( popup.setLngLat([props.longitude, props.latitude]); } if (props.offset && !deepEqual(oldProps.offset, props.offset)) { + popup.options.anchor = props.anchor; popup.setOffset(props.offset); } if (oldProps.anchor !== props.anchor || oldProps.maxWidth !== props.maxWidth) { @@ -82,11 +83,8 @@ export const Popup = memo( } const classNameDiff = compareClassNames(oldProps.className, props.className); if (classNameDiff) { - for (const c of classNameDiff[0]) { - popup.addClassName(c); - } - for (const c of classNameDiff[1]) { - popup.removeClassName(c); + for (const c of classNameDiff) { + popup.toggleClassName(c); } } thisRef.current.props = props; diff --git a/modules/react-mapbox/src/utils/compare-class-names.ts b/modules/react-mapbox/src/utils/compare-class-names.ts index 6a77c9104..420345b9c 100644 --- a/modules/react-mapbox/src/utils/compare-class-names.ts +++ b/modules/react-mapbox/src/utils/compare-class-names.ts @@ -2,33 +2,26 @@ export function compareClassNames( prevClassName: string | undefined, nextClassName: string | undefined -): [added: string[], removed: string[]] | null { +): string[] | null { if (prevClassName === nextClassName) { return null; } const prevClassList = getClassList(prevClassName); const nextClassList = getClassList(nextClassName); - const added: string[] = []; - const removed: string[] = []; - - for (const c of prevClassList) { - if (!nextClassList.has(c)) { - removed.push(c); - } - } - - if (removed.length === 0 && prevClassList.size === nextClassList.size) { - return null; - } + const diff: string[] = []; for (const c of nextClassList) { if (!prevClassList.has(c)) { - added.push(c); + diff.push(c); } } - - return [added, removed]; + for (const c of prevClassList) { + if (!nextClassList.has(c)) { + diff.push(c); + } + } + return diff.length === 0 ? null : diff; } function getClassList(className: string | undefined) { 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 index 8b6f0e1d0..af7c8d50e 100644 --- a/modules/react-mapbox/test/utils/compare-class-names.spec.js +++ b/modules/react-mapbox/test/utils/compare-class-names.spec.js @@ -19,25 +19,25 @@ test('compareClassNames', t => { title: 'Addition', prevClassName: undefined, nextClassName: 'marker', - output: [['marker'], []] + output: ['marker'] }, { title: 'Addition', prevClassName: 'marker', nextClassName: 'marker active', - output: [['active'], []] + output: ['active'] }, { title: 'Removal', prevClassName: 'marker active', nextClassName: 'marker', - output: [[], ['active']] + output: ['active'] }, { title: 'Multiple addition & removal', prevClassName: 'marker active', nextClassName: 'marker hovered hidden', - output: [['hovered', 'hidden'], ['active']] + output: ['hovered', 'hidden', 'active'] } ]; diff --git a/modules/react-maplibre/src/components/marker.ts b/modules/react-maplibre/src/components/marker.ts index bdec59b70..c1fbec740 100644 --- a/modules/react-maplibre/src/components/marker.ts +++ b/modules/react-maplibre/src/components/marker.ts @@ -126,11 +126,8 @@ export const Marker = memo( } const classNameDiff = compareClassNames(oldProps.className, props.className); if (classNameDiff) { - for (const c of classNameDiff[0]) { - marker.addClassName(c); - } - for (const c of classNameDiff[1]) { - marker.removeClassName(c); + for (const c of classNameDiff) { + marker.toggleClassName(c); } } diff --git a/modules/react-maplibre/src/components/popup.ts b/modules/react-maplibre/src/components/popup.ts index ab86e3369..173fa39a3 100644 --- a/modules/react-maplibre/src/components/popup.ts +++ b/modules/react-maplibre/src/components/popup.ts @@ -78,15 +78,13 @@ export const Popup = memo( popup.setOffset(props.offset); } if (oldProps.anchor !== props.anchor || oldProps.maxWidth !== props.maxWidth) { + popup.options.anchor = props.anchor; popup.setMaxWidth(props.maxWidth); } const classNameDiff = compareClassNames(oldProps.className, props.className); if (classNameDiff) { - for (const c of classNameDiff[0]) { - popup.addClassName(c); - } - for (const c of classNameDiff[1]) { - popup.removeClassName(c); + for (const c of classNameDiff) { + popup.toggleClassName(c); } } thisRef.current.props = props; diff --git a/modules/react-maplibre/src/utils/compare-class-names.ts b/modules/react-maplibre/src/utils/compare-class-names.ts index 6a77c9104..420345b9c 100644 --- a/modules/react-maplibre/src/utils/compare-class-names.ts +++ b/modules/react-maplibre/src/utils/compare-class-names.ts @@ -2,33 +2,26 @@ export function compareClassNames( prevClassName: string | undefined, nextClassName: string | undefined -): [added: string[], removed: string[]] | null { +): string[] | null { if (prevClassName === nextClassName) { return null; } const prevClassList = getClassList(prevClassName); const nextClassList = getClassList(nextClassName); - const added: string[] = []; - const removed: string[] = []; - - for (const c of prevClassList) { - if (!nextClassList.has(c)) { - removed.push(c); - } - } - - if (removed.length === 0 && prevClassList.size === nextClassList.size) { - return null; - } + const diff: string[] = []; for (const c of nextClassList) { if (!prevClassList.has(c)) { - added.push(c); + diff.push(c); } } - - return [added, removed]; + for (const c of prevClassList) { + if (!nextClassList.has(c)) { + diff.push(c); + } + } + return diff.length === 0 ? null : diff; } function getClassList(className: string | undefined) { 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 index 057cf12a9..8bc914ddf 100644 --- a/modules/react-maplibre/test/utils/compare-class-names.spec.js +++ b/modules/react-maplibre/test/utils/compare-class-names.spec.js @@ -19,25 +19,25 @@ test('compareClassNames', t => { title: 'Addition', prevClassName: undefined, nextClassName: 'marker', - output: [['marker'], []] + output: ['marker'] }, { title: 'Addition', prevClassName: 'marker', nextClassName: 'marker active', - output: [['active'], []] + output: ['active'] }, { title: 'Removal', prevClassName: 'marker active', nextClassName: 'marker', - output: [[], ['active']] + output: ['active'] }, { title: 'Multiple addition & removal', prevClassName: 'marker active', nextClassName: 'marker hovered hidden', - output: [['hovered', 'hidden'], ['active']] + output: ['hovered', 'hidden', 'active'] } ];