Handle class name change

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<MarkerInstance>) => {
     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());
   })
 );

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<PopupInstance>) => {
@@ -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);

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+/) : []);
+}

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');

modules/react-mapbox/test/components/popup.spec.jsx

@@ -67,8 +67,7 @@ test('Popup', async t => {
     </Map>
   );
   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();

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();
+});

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';

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<MarkerInstance>) => {
     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());
   })
 );

modules/react-maplibre/src/components/popup.ts

@@ -9,6 +9,7 @@
 
 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 @@
   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<PopupInstance>) => {
@@ -37,13 +33,12 @@
       return document.createElement('div');
     }, []);
     const thisRef = useRef({props});
-    thisRef.current.props = props;
 
     const popup: PopupInstance = useMemo(() => {
       const options = {...props};
       const pp = new mapLib.Popup(options);
       pp.setLngLat([props.longitude, props.latitude]);
       pp.once('open', e => {
         thisRef.current.props.onOpen?.(e as PopupEvent);
       });
       return pp;
@@ -75,32 +70,24 @@
     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);

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+/) : []);
+}

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');

modules/react-maplibre/test/components/popup.spec.jsx

@@ -67,8 +67,7 @@ test('Popup', async t => {
     </Map>
   );
   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();