refactor: improve activeTab URL modifications

packages/portal/src/components/item/ItemMediaPresentation.vue

@@ -26,12 +26,12 @@
         <template v-else>
           <template v-if="sidebarHasContent">
             <ItemMediaSidebar
-              v-show="showSidebar"
               ref="sidebar"
               tabindex="0"
               :annotation-list="hasAnnotations"
               :annotation-search="hasAnnotations && hasSearchService"
               :manifest-uri="uri"
+              :show="showSidebar"
               @keydown.escape.native="showSidebar = false"
             />
             <ItemMediaSidebarToggle

packages/portal/src/components/item/ItemMediaSidebar.vue

@@ -3,6 +3,7 @@
     name="fade"
   >
     <div
+      v-show="show"
       class="media-viewer-sidebar"
       data-qa="item media sidebar"
     >
@@ -158,6 +159,10 @@
       query: {
         type: String,
         default: null
+      },
+      show: {
+        type: Boolean,
+        default: true
       }
     },
 
@@ -173,8 +178,8 @@
         tabHashes.push('#links');
       }
 
-      const { activeTabHash, activeTabHistory, activeTabIndex } = useActiveTab(tabHashes);
-      return { activeTabHash, activeTabHistory, activeTabIndex };
+      const { activeTabHash, activeTabHistory, activeTabIndex, watchTabIndex, unwatchTabIndex } = useActiveTab(tabHashes);
+      return { activeTabHash, activeTabHistory, activeTabIndex, watchTabIndex, unwatchTabIndex };
     },
 
     data() {
@@ -184,6 +189,22 @@
       };
     },
 
+    watch: {
+      show(val) {
+        if (val) {
+          this.watchTabIndex();
+        } else {
+          this.unwatchTabIndex();
+        }
+      }
+    },
+
+    mounted() {
+      if (this.show) {
+        this.watchTabIndex();
+      }
+    },
+
     methods: {
       handleAnnotationsFetched(annotationsLength) {
         this.annotationsCount = annotationsLength;

packages/portal/src/composables/activeTab.js

@@ -12,19 +12,27 @@ export default function useActiveTab(tabHashes) {
   const setActiveTabIndexFromRouteHash = () => {
     if (tabHashes.includes(route?.hash)) {
       activeTabIndex.value = tabHashes.indexOf(route.hash);
+      activeTabHistory.value.push(activeTabHash.value);
     }
   };
 
   const activeTabHash = computed(() => {
     return tabHashes[activeTabIndex.value];
   });
 
-  watch(activeTabIndex, () => {
-    if (activeTabIndex.value !== -1) {
-      activeTabHistory.value.push(activeTabHash.value);
-      router.push({ ...route, hash: activeTabHash.value });
-    }
-  });
+  let unwatchTabIndex = () => {};
+
+  const watchTabIndex = () => {
+    // unwatch 1st to prevent duplicate watchers
+    unwatchTabIndex();
+
+    unwatchTabIndex = watch(activeTabIndex, () => {
+      if (activeTabIndex.value !== -1) {
+        activeTabHistory.value.push(activeTabHash.value);
+        router.replace({ ...route, hash: activeTabHash.value });
+      }
+    });
+  };
 
   if (route) {
     watch(route, () => {
@@ -39,6 +47,8 @@ export default function useActiveTab(tabHashes) {
   return {
     activeTabHash,
     activeTabHistory,
-    activeTabIndex
+    activeTabIndex,
+    watchTabIndex,
+    unwatchTabIndex
   };
 }

packages/portal/tests/unit/composables/activeTab.spec.js

@@ -5,12 +5,12 @@ import { reactive } from 'vue';
 
 import useActiveTab from '@/composables/activeTab.js';
 
-const routerPushSpy = sinon.spy();
+const routerReplaceSpy = sinon.spy();
 
 const route = reactive({ hash: '#links' });
 sinon.stub(vueRouter, 'useRoute').returns(route);
 sinon.stub(vueRouter, 'useRouter').returns({
-  push: routerPushSpy
+  replace: routerReplaceSpy
 });
 
 const tabHashes = ['#annotations', '#search', '#links'];
@@ -23,9 +23,9 @@ const component = {
     </div>
   `,
   setup() {
-    const { activeTabHash, activeTabIndex } = useActiveTab(tabHashes);
+    const { activeTabHash, activeTabIndex, watchTabIndex } = useActiveTab(tabHashes);
 
-    return { activeTabHash, activeTabIndex };
+    return { activeTabHash, activeTabIndex, watchTabIndex };
   }
 };
 
@@ -65,23 +65,26 @@ describe('useActiveTab', () => {
       expect(input.element.value).toBe('2');
     });
 
-    it('is watched for changes to update route', async() => {
+    it('is updated on route changes', async() => {
       const wrapper = factory();
 
-      wrapper.find('#activeTabIndex').setValue(1);
+      route.hash = '#annotations';
       await wrapper.vm.$nextTick();
 
-      expect(routerPushSpy.calledWith({ hash: '#search' })).toBe(true);
+      const input = wrapper.find('#activeTabIndex');
+      expect(input.element.value).toBe('0');
     });
+  });
 
-    it('is updated on route changes', async() => {
+  describe('watchTabIndex', () => {
+    it('starts watching for changes to replace hash in route', async() => {
       const wrapper = factory();
+      wrapper.vm.watchTabIndex();
 
-      route.hash = '#annotations';
+      wrapper.find('#activeTabIndex').setValue(1);
       await wrapper.vm.$nextTick();
 
-      const input = wrapper.find('#activeTabIndex');
-      expect(input.element.value).toBe('0');
+      expect(routerReplaceSpy.calledWith({ hash: '#search' })).toBe(true);
     });
   });
 });