Wait for the nav collapse animation to end before changing the language on mobile (#367)

closes rdar://94461604

Author: dobromir-hristov

src/components/DocumentationTopic/DocumentationNav.vue

@@ -45,7 +45,7 @@
         <span v-else class="nav-title-link inactive">Documentation</span>
       </slot>
     </template>
-    <template slot="tray">
+    <template #tray="{ closeNav }">
       <Hierarchy
         :currentTopicTitle="title"
         :isSymbolDeprecated="isSymbolDeprecated"
@@ -63,6 +63,7 @@
           :interfaceLanguage="interfaceLanguage"
           :objcPath="objcPath"
           :swiftPath="swiftPath"
+          :closeNav="closeNav"
         />
         <slot name="menu-items" />
       </NavMenuItems>

src/components/DocumentationTopic/DocumentationNav/LanguageToggle.vue

@@ -66,13 +66,14 @@
           >
             {{ language.name }}
           </span>
-          <router-link
+          <a
             v-else
+            href="#"
             class="nav-menu-link"
-            :to="getRoute(language.route)"
+            @click.prevent="pushRoute(language.route)"
           >
             {{ language.name }}
-          </router-link>
+          </a>
         </li>
       </ul>
     </div>
@@ -111,6 +112,10 @@ export default {
       type: String,
       required: false,
     },
+    closeNav: {
+      type: Function,
+      default: () => {},
+    },
   },
   data() {
     return {
@@ -160,7 +165,8 @@ export default {
         path: this.isCurrentPath(route.path) ? null : this.normalizePath(route.path),
       };
     },
-    pushRoute(route) {
+    async pushRoute(route) {
+      await this.closeNav();
       // Persist the selected language as a preference in the store (backed by
       // the browser's local storage so that it can be retrieved later for
       // subsequent navigation without the query parameter present)

src/components/NavBase.vue

@@ -45,7 +45,7 @@
             @transitionend.self="onTransitionEnd"
             @click="handleTrayClick"
           >
-            <slot name="tray">
+            <slot name="tray" :closeNav="closeNav">
               <NavMenuItems>
                 <slot name="menu-items" />
               </NavMenuItems>
@@ -91,7 +91,7 @@ const NavStateClasses = {
   isDark: 'theme-dark',
   isOpen: 'nav--is-open',
   inBreakpoint: 'nav--in-breakpoint-range',
-  isTransitioning: 'nav--is-opening',
+  isTransitioning: 'nav--is-transitioning',
   isSticking: 'nav--is-sticking',
   hasSolidBackground: 'nav--solid-background',
   hasNoBorder: 'nav--noborder',
@@ -215,6 +215,20 @@ export default {
     },
     closeNav() {
       this.isOpen = false;
+      return this.resolveOnceTransitionsEnd();
+    },
+    resolveOnceTransitionsEnd() {
+      // if outside the breakpoint, resolve as there is no tray animation
+      if (!this.inBreakpoint) return Promise.resolve();
+      // enable the transitioning up tracking
+      this.isTransitioning = true;
+      // resolve a promise, when we stop transitioning
+      return new Promise((resolve) => {
+        const unwatch = this.$watch('isTransitioning', () => {
+          resolve();
+          unwatch();
+        });
+      });
     },
     /**
      * When the closing animation ends,
@@ -620,7 +634,7 @@ $content-max-width: map-deep-get($breakpoint-attributes, (nav, large, content-wi
       visibility: visible;
       transition-delay: 0.2s, 0s;
 
-      @include unify-selector('.nav--is-opening', $nested: true) {
+      @include unify-selector('.nav--is-transitioning', $nested: true) {
         overflow-y: hidden;
       }
 

tests/unit/components/DocumentationTopic/DocumentationNav.spec.js

@@ -210,6 +210,7 @@ describe('DocumentationNav', () => {
       interfaceLanguage: propsData.interfaceLanguage,
       swiftPath: propsData.swiftPath,
       objcPath: propsData.objcPath,
+      closeNav: expect.any(Function),
     });
   });
 

tests/unit/components/DocumentationTopic/DocumentationNav/LanguageToggle.spec.js

@@ -14,10 +14,12 @@ import Language from 'docc-render/constants/Language';
 import LanguageToggle
   from 'docc-render/components/DocumentationTopic/DocumentationNav/LanguageToggle.vue';
 import InlineChevronDownIcon from 'theme/components/Icons/InlineChevronDownIcon.vue';
-import { createEvent } from '../../../../../test-utils';
+import { createEvent, flushPromises } from '../../../../../test-utils';
 
 const { NavMenuItemBase } = LanguageToggle.components;
 
+const closeNav = jest.fn().mockResolvedValue('');
+
 describe('LanguageToggle', () => {
   let wrapper;
 
@@ -26,6 +28,7 @@ describe('LanguageToggle', () => {
     objcPath: 'documentation/foo',
     swiftPath: 'documentation/foo',
     breadcrumbCount: 1,
+    closeNav,
   };
 
   const mocks = {
@@ -58,6 +61,7 @@ describe('LanguageToggle', () => {
 
   beforeEach(() => {
     wrapper = createWrapper();
+    jest.clearAllMocks();
   });
 
   it('renders `NavMenuItemBase` at the root', () => {
@@ -161,13 +165,15 @@ describe('LanguageToggle', () => {
     expect(options.at(1).text()).toBe(Language.objectiveC.name);
   });
 
-  it('calls router and changes v-model when different option is selected', () => {
+  it('calls router and changes v-model when different option is selected', async () => {
     expect(wrapper.find('.current-language').text()).toBe(Language.swift.name);
 
     wrapper.findAll('#language-toggle option').at(1).element.selected = true;
     wrapper.find('#language-toggle').trigger('change');
 
     expect(wrapper.find('.current-language').text()).toBe(Language.objectiveC.name);
+    expect(closeNav).toHaveBeenCalledTimes(1);
+    await flushPromises();
     expect(mocks.$router.push).toHaveBeenCalledWith({ path: null, query: { language: 'objc' } });
     expect(provide.store.setPreferredLanguage).toHaveBeenCalledWith('objc');
   });
@@ -205,21 +211,37 @@ describe('LanguageToggle', () => {
     expect(currentLanguage.text()).toBe(Language.swift.name);
   });
 
-  it('renders a link for the alternative language inside `language-list-container`', () => {
+  it('renders a link for the alternative language inside `language-list-container`', async () => {
     const link = wrapper.find('.language-list-container').find('a.nav-menu-link');
     expect(link.exists()).toBe(true);
     expect(link.text()).toBe(Language.objectiveC.name);
-    expect(link.props('to').query).toEqual({ language: Language.objectiveC.key.url });
+    link.trigger('click');
+    expect(closeNav).toHaveBeenCalledTimes(1);
+    await flushPromises();
+    expect(mocks.$router.push)
+      .toHaveBeenCalledWith({
+        path: null,
+        query: { language: Language.objectiveC.key.url },
+      });
   });
 
-  it('clears out the language query if language is Swift', () => {
+  it('clears out the language query if language is Swift', async () => {
     wrapper.setData({ languageModel: Language.objectiveC.key.api });
 
     const link = wrapper.find('.language-list-container').find('a.nav-menu-link');
-    expect(link.props('to').query).toEqual({ language: undefined });
+    link.trigger('click');
+    expect(closeNav).toHaveBeenCalledTimes(1);
+    await flushPromises();
+    expect(mocks.$router.push)
+      .toHaveBeenCalledWith({
+        path: null,
+        query: {
+          language: undefined,
+        },
+      });
   });
 
-  it('keeps extra query parameters when changing language', () => {
+  it('keeps extra query parameters when changing language', async () => {
     const query = {
       foo: 'foo',
       bar: 'bar',
@@ -234,15 +256,29 @@ describe('LanguageToggle', () => {
     wrapper = createWrapper(undefined, mocksWithQuery);
 
     const link = wrapper.find('.language-list-container').find('a.nav-menu-link');
-    expect(link.props('to').query).toEqual(expect.objectContaining(query));
+    link.trigger('click');
+    expect(closeNav).toHaveBeenCalledTimes(1);
+    await flushPromises();
+    expect(mocks.$router.push)
+      .toHaveBeenCalledWith({
+        path: null,
+        query: { ...query, language: Language.objectiveC.key.url },
+      });
   });
 
-  it('renders different paths if languages have different paths', () => {
+  it('renders different paths if languages have different paths', async () => {
     // Re-mount component to be able to update data() through new props
     wrapper = createWrapper({ ...propsData, objcPath: 'documentation/bar' });
 
     const link = wrapper.find('.language-list-container').find('a.nav-menu-link');
-    expect(link.props('to').path).toEqual('/documentation/bar');
+    link.trigger('click');
+    expect(closeNav).toHaveBeenCalledTimes(1);
+    await flushPromises();
+    expect(mocks.$router.push)
+      .toHaveBeenCalledWith({
+        path: '/documentation/bar',
+        query: { language: Language.objectiveC.key.url },
+      });
   });
 
   it('changes the model, if the interfaceLanguage changes', () => {

tests/unit/components/NavBase.spec.js

@@ -204,14 +204,21 @@ describe('NavBase', () => {
   });
 
   it('renders the `tray` slot', async () => {
+    let slotProps = null;
     wrapper = await createWrapper({
-      slots: {
-        tray: '<div class="tray-slot">Tray slot content</div>',
+      scopedSlots: {
+        tray(props) {
+          slotProps = props;
+          return this.$createElement('div', { class: 'tray-slot' }, 'Tray slot content');
+        },
       },
     });
     const tray = wrapper.find({ ref: 'tray' });
     expect(tray.find('.tray-slot').text()).toBe('Tray slot content');
     expect(wrapper.find(NavMenuItems).exists()).toBe(false);
+    expect(slotProps).toEqual({
+      closeNav: expect.any(Function),
+    });
   });
 
   it('renders the `menu-items` slot', async () => {
@@ -551,5 +558,21 @@ describe('NavBase', () => {
       wrapper.find(BreakpointEmitter).vm.$emit('change', BreakpointName.large);
       expect(wrapper.classes()).not.toContain(NavStateClasses.isOpen);
     });
+
+    it('resolves closeNav on transitionEnd, only when inside Breakpoint', async () => {
+      wrapper = await createWrapper({
+        data: () => ({ inBreakpoint: true, isOpen: true }),
+      });
+      expect(wrapper.classes()).toContain(NavStateClasses.isOpen);
+      let resolved = false;
+      wrapper.vm.closeNav().then(() => {
+        resolved = true;
+      });
+      await wrapper.vm.$nextTick();
+      expect(resolved).toBe(false);
+      emitEndOfTrayTransition(wrapper);
+      await wrapper.vm.$nextTick();
+      expect(resolved).toBe(true);
+    });
   });
 });