fix(pickup): fix map not mounting again after having been unmounted (#…

…239) Fixes an issue where the map would not work again after removal. Updates the `usePickupLocationsMap`-composable to use a global state where possible and not use memoization to achieve the same. Additionally fixes an issue where the map could be panned before being fully loaded. Fixes: INT-641 --------- Co-authored-by: Edie Lemoine <[email protected]>
myparcelnl · Sep 19, 2024 · e810b8e · e810b8e
1 parent ae39b79
commit e810b8e
Show file tree

Hide file tree

Showing 6 changed files with 213 additions and 58 deletions.
diff --git a/.github/actions/build-v5/action.yml b/.github/actions/build-v5/action.yml
@@ -0,0 +1,24 @@
+name: 'Build (legacy v5)'
+description: 'Build the project'
+
+runs:
+  using: composite
+  steps:
+    - uses: myparcelnl/actions/yarn-install@v4
+      with:
+        node-version: 18
+
+    - name: 'Cache build'
+      uses: actions/cache@v4
+      id: cache-build
+      with:
+        path: |
+          ./dist
+        key: ${{ runner.os }}-build-${{ hashFiles('**/yarn.lock') }}-${{ hashFiles('**/src') }}
+
+    - name: 'Create build'
+      if: steps.cache-build.outputs.cache-hit != 'true'
+      run: yarn build
+      shell: bash
+      env:
+        NODE_OPTIONS: --openssl-legacy-provider
diff --git a/.github/actions/test-v5/action.yml b/.github/actions/test-v5/action.yml
@@ -0,0 +1,36 @@
+name: 'Run tests'
+description: 'Run tests and upload coverage'
+
+inputs:
+  codecov-token:
+    description: 'Codecov token'
+    required: true
+
+runs:
+  using: composite
+  steps:
+    - name: 'Cache coverage'
+      uses: actions/cache@v4
+      id: cache-coverage
+      with:
+        path: ./coverage
+        key: ${{ runner.os }}-coverage-${{ hashFiles('**/yarn.lock') }}-${{ hashFiles('**/src') }}
+        restore-keys: |
+          ${{ runner.os }}-coverage-${{ hashFiles('**/yarn.lock') }}
+          ${{ runner.os }}-coverage-
+
+    - uses: myparcelnl/actions/yarn-install@v4
+      if: steps.cache-coverage.outputs.cache-hit != 'true'
+      with:
+        node-version: 18
+
+    - name: 'Run tests'
+      if: steps.cache-coverage.outputs.cache-hit != 'true'
+      run: ./node_modules/.bin/cross-env yarn test:unit --coverage
+      shell: bash
+      env:
+        NODE_ICU_DATA: node_modules/full-icu
+
+    - uses: codecov/codecov-action@v4
+      with:
+        token: ${{ inputs.codecov-token }}
diff --git a/.github/workflows/push-v5.yml b/.github/workflows/push-v5.yml
@@ -0,0 +1,63 @@
+name: 'On push (legacy v5) ⚙️'
+
+on:
+  push:
+    branches:
+      - v5.x
+
+    paths-ignore:
+      - ./**/*.md
+      - ./.idea/**
+
+concurrency:
+  group: '${{ github.workflow }}-${{ github.ref }}'
+  cancel-in-progress: true
+
+jobs:
+  test:
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: ./.github/actions/test-v5
+        with:
+          codecov-token: ${{ secrets.CODECOV_TOKEN }}
+
+  build:
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: ./.github/actions/build-v5
+
+  release:
+    runs-on: ubuntu-22.04
+    needs:
+      - test
+      - build
+    steps:
+      - uses: myparcelnl/actions/setup-git-credentials@v4
+        id: credentials
+        with:
+          app-id: ${{ secrets.MYPARCEL_APP_ID }}
+          private-key: ${{ secrets.MYPARCEL_APP_PRIVATE_KEY }}
+
+      - uses: actions/checkout@v4
+        with:
+          token: ${{ steps.credentials.outputs.token }}
+          fetch-depth: 0
+
+      - uses: ./.github/actions/build-v5
+
+      - uses: myparcelnl/actions/semantic-release@v4
+        env:
+          NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
+        with:
+          token: ${{ steps.credentials.outputs.token }}
+
+  rebase-prs:
+    needs:
+      - test
+      - build
+    uses: ./.github/workflows/rebase-prs.yml
+    secrets: inherit
diff --git a/.github/workflows/run-tests-v5.yml b/.github/workflows/run-tests-v5.yml
@@ -0,0 +1,27 @@
+name: 'Run tests (legacy v5) 🧪'
+
+on:
+  pull_request:
+    branches:
+      - v5.x
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  test:
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: ./.github/actions/test-v5
+        with:
+          codecov-token: ${{ secrets.CODECOV_TOKEN }}
+
+  build:
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: ./.github/actions/build-v5
diff --git a/apps/delivery-options/src/components/map/LeafletMapInner/LeafletMapInner.vue b/apps/delivery-options/src/components/map/LeafletMapInner/LeafletMapInner.vue
@@ -25,7 +25,7 @@ const css = asyncComputed(async () => {
 const styleTag = useStyleTag(css);
 const scriptTag = useScriptTag('https://cdn.jsdelivr.net/npm/leaflet@1/dist/leaflet.js');
 
-const {initializeMap, activeMarker, center, map} = usePickupLocationsMap();
+const {initializeMap, activeMarker, center, map, loaded} = usePickupLocationsMap();
 
 onMounted(async () => {
   styleTag.load();
@@ -43,6 +43,8 @@ onUnmounted(() => {
 });
 
 onActivated(() => {
-  map.value?.panTo(toValue(activeMarker)?.getLatLng() ?? toValue(center));
+  if (loaded.value) {
+    map.value?.panTo(toValue(activeMarker)?.getLatLng() ?? toValue(center));
+  }
 });
 </script>
diff --git a/apps/delivery-options/src/composables/usePickupLocationsMap.ts b/apps/delivery-options/src/composables/usePickupLocationsMap.ts
@@ -1,7 +1,7 @@
 import {type Ref, ref, computed, watch, toRef, toValue, type ComputedRef} from 'vue';
 import {isString} from 'radash';
 import {type Map, type TileLayer, type Control} from 'leaflet';
-import {isDef, useDebounceFn, useMemoize} from '@vueuse/core';
+import {isDef, useDebounceFn} from '@vueuse/core';
 import {type MapTileLayerData, ConfigSetting} from '@myparcel-do/shared';
 import {type LeafletMapProps, type MapMarker} from '../types';
 import {useConfigStore} from '../stores';
@@ -26,47 +26,64 @@ interface UsePickupLocationsMap {
 
 const LEAFLET_DEFAULT_ZOOM = 14;
 
-// eslint-disable-next-line max-lines-per-function
-export const usePickupLocationsMap = useMemoize((): UsePickupLocationsMap => {
-  const {carriersWithPickup} = useResolvedPickupLocations();
-  const config = useConfigStore();
-
-  /**
-   * The element that the map is rendered in.
-   */
-  const container = ref<HTMLElement | undefined>();
-
-  /**
-   * The Leaflet map instance.
-   */
-  const map = ref<Map>();
-
-  /**
-   * The Leaflet tile layer instance.
-   */
-  const tileLayer = ref<TileLayer>();
-
-  /**
-   * The markers that are displayed on the map.
-   */
-  const markers = ref<MapMarker[]>([]);
+/**
+ * The Leaflet map instance.
+ */
+const map = ref<Map>();
+
+/**
+ * The Leaflet tile layer instance.
+ */
+const tileLayer = ref<TileLayer>();
+
+/**
+ * The markers that are displayed on the map.
+ */
+const markers = ref<MapMarker[]>([]);
+
+/**
+ * The Leaflet scale control instance.
+ */
+const scaleControl = ref<Control.Scale>();
+
+/**
+ * The center of the map.
+ */
+const center = ref<NonNullable<LeafletMapProps['center']>>([0, 0]);
+
+/**
+ * The zoom level of the map.
+ */
+const zoom = ref<NonNullable<LeafletMapProps['zoom']>>(LEAFLET_DEFAULT_ZOOM);
+
+/**
+ * The element that the map is rendered in.
+ */
+const container = ref<HTMLElement | undefined>();
+
+const loaded = computed(() => {
+  return (
+    Boolean(toValue(container)) &&
+    Boolean(toValue(map)) &&
+    Boolean(toValue(tileLayer)) &&
+    Boolean(toValue(scaleControl))
+  );
+});
 
-  /**
-   * The Leaflet scale control instance.
-   */
-  const scaleControl = ref<Control.Scale>();
+const activeMarker = computed<MapMarker | undefined>(() => {
+  return toValue(markers).find((marker) => {
+    const element = marker.getElement();
 
-  /**
-   * The center of the map.
-   */
-  const center = ref<NonNullable<LeafletMapProps['center']>>([0, 0]);
+    return element?.classList.contains(MAP_MARKER_CLASS_ACTIVE);
+  }) as MapMarker | undefined;
+});
 
-  /**
-   * The zoom level of the map.
-   */
-  const zoom = ref<NonNullable<LeafletMapProps['zoom']>>(LEAFLET_DEFAULT_ZOOM);
+const mutableShowLoadMoreButton = ref<boolean>(false);
 
-  const mutableShowLoadMoreButton = ref<boolean>(false);
+// eslint-disable-next-line max-lines-per-function
+export const usePickupLocationsMap = (): UsePickupLocationsMap => {
+  const {carriersWithPickup} = useResolvedPickupLocations();
+  const config = useConfigStore();
 
   const showLoadMoreButton = computed({
     get() {
@@ -125,6 +142,9 @@ export const usePickupLocationsMap = useMemoize((): UsePickupLocationsMap => {
 
     return () => {
       map.value?.remove();
+      // Reset any leaflet-bound values to their initial state.
+      tileLayer.value = undefined;
+      scaleControl.value = undefined;
       unwatchZoom();
     };
   };
@@ -144,23 +164,6 @@ export const usePickupLocationsMap = useMemoize((): UsePickupLocationsMap => {
     }
   }, 100);
 
-  const loaded = computed(() => {
-    return (
-      Boolean(toValue(container)) &&
-      Boolean(toValue(map)) &&
-      Boolean(toValue(tileLayer)) &&
-      Boolean(toValue(scaleControl))
-    );
-  });
-
-  const activeMarker = computed<MapMarker | undefined>(() => {
-    return toValue(markers).find((marker) => {
-      const element = marker.getElement();
-
-      return element?.classList.contains(MAP_MARKER_CLASS_ACTIVE);
-    }) as MapMarker | undefined;
-  });
-
   return {
     fitBounds,
     initializeMap,
@@ -174,4 +177,4 @@ export const usePickupLocationsMap = useMemoize((): UsePickupLocationsMap => {
     showLoadMoreButton,
     tileLayer,
   };
-});
+};