Update scroll lock logic to lock everything but disabled targets

src/components/Filter/FilterInput.vue

@@ -37,6 +37,7 @@
         </button>
         <div
           :class="['filter__input-box-wrapper', { 'scrolling': isScrolling }]"
+          v-bind="{[SCROLL_LOCK_DISABLE_HORIZONTAL_ATTR]: true}"
           @scroll="handleScroll"
         >
           <TagList
@@ -106,7 +107,7 @@
         </div>
       </div>
       <TagList
-        v-if="displaySuggestedTags"
+        v-show="displaySuggestedTags"
         :id="SuggestedTagsId"
         ref="suggestedTags"
         :ariaLabel="$tc('filter.suggested-tags', suggestedTags.length)"
@@ -128,6 +129,7 @@
 import ClearRoundedIcon from 'theme/components/Icons/ClearRoundedIcon.vue';
 import multipleSelection from 'docc-render/mixins/multipleSelection';
 import handleScrollbar from 'docc-render/mixins/handleScrollbar';
+import { SCROLL_LOCK_DISABLE_HORIZONTAL_ATTR } from 'docc-render/utils/scroll-lock';
 import FilterIcon from 'theme/components/Icons/FilterIcon.vue';
 import TagList from './TagList.vue';
 
@@ -225,6 +227,7 @@ export default {
       SuggestedTagsId,
       AXinputProperties,
       showSuggestedTags: false,
+      SCROLL_LOCK_DISABLE_HORIZONTAL_ATTR,
     };
   },
   computed: {

src/components/Filter/TagList.vue

@@ -24,6 +24,7 @@
         role="listbox"
         :aria-multiselectable="areTagsRemovable ? 'true' : 'false'"
         aria-orientation="horizontal"
+        v-bind="{[SCROLL_LOCK_DISABLE_HORIZONTAL_ATTR]: true}"
         @keydown.left.capture.prevent="focusPrev"
         @keydown.right.capture.prevent="focusNext"
         @keydown.up.capture.prevent="focusPrev"
@@ -61,6 +62,7 @@
 import { isSingleCharacter } from 'docc-render/utils/input-helper';
 import handleScrollbar from 'docc-render/mixins/handleScrollbar';
 import keyboardNavigation from 'docc-render/mixins/keyboardNavigation';
+import { SCROLL_LOCK_DISABLE_HORIZONTAL_ATTR } from 'docc-render/utils/scroll-lock';
 import Tag from './Tag.vue';
 
 export default {
@@ -69,6 +71,11 @@ export default {
     handleScrollbar,
     keyboardNavigation,
   ],
+  data() {
+    return {
+      SCROLL_LOCK_DISABLE_HORIZONTAL_ATTR,
+    };
+  },
   props: {
     tags: {
       type: Array,

src/utils/scroll-lock.js

@@ -10,9 +10,12 @@
 
 let isLocked = false;
 let initialClientY = -1;
+let initialClientX = -1;
 let scrolledClientY = 0;
-// Adds this attribute to an inner scrollable element to allow it to scroll
+// Adds this attribute to an vertical scrollable element to allow it to scroll
 export const SCROLL_LOCK_DISABLE_ATTR = 'data-scroll-lock-disable';
+// Adds this attribute to an horizontal scrollable element to allow it to scroll
+export const SCROLL_LOCK_DISABLE_HORIZONTAL_ATTR = 'data-scroll-lock-horizontal-disable';
 
 const isIosDevice = () => window.navigator
   && window.navigator.platform
@@ -79,17 +82,22 @@ function advancedUnlock(targetElement) {
  * @param {HTMLElement} targetElement
  * @return {boolean}
  */
-function handleScroll(event, targetElement) {
+function handleScroll(event, target, isHorizontal) {
   const clientY = event.targetTouches[0].clientY - initialClientY;
-  // check if any parent has a scroll-lock disable, if not use the targetElement
-  const target = event.target.closest(`[${SCROLL_LOCK_DISABLE_ATTR}]`) || targetElement;
-  if (target.scrollTop === 0 && clientY > 0) {
-    // element is at the top of its scroll.
-    return preventDefault(event);
-  }
+  const clientX = event.targetTouches[0].clientX - initialClientX;
 
-  if (isTargetElementTotallyScrolled(target) && clientY < 0) {
-    // element is at the bottom of its scroll.
+  if (!isHorizontal) {
+    if (target.scrollTop === 0 && clientY > 0) {
+      // element is at the top of its scroll.
+      return preventDefault(event);
+    }
+
+    if (isTargetElementTotallyScrolled(target) && clientY < 0) {
+      // element is at the bottom of its scroll.
+      return preventDefault(event);
+    }
+  } else if (Math.abs(clientY) > Math.abs(clientX)) {
+    // prevent event if user tries to perform vertical scroll in an horizontal scrolling element
     return preventDefault(event);
   }
 
@@ -102,7 +110,7 @@ function handleScroll(event, targetElement) {
  * Advanced scroll locking for iOS devices.
  * @param targetElement
  */
-function advancedLock(targetElement) {
+function advancedLock(targetElement, isHorizontal = false) {
   // add a scroll listener to the body
   document.addEventListener('touchmove', preventDefault, { passive: false });
   if (!targetElement) return;
@@ -112,12 +120,13 @@ function advancedLock(targetElement) {
     if (event.targetTouches.length === 1) {
       // detect single touch.
       initialClientY = event.targetTouches[0].clientY;
+      initialClientX = event.targetTouches[0].clientX;
     }
   };
   targetElement.ontouchmove = (event) => {
     if (event.targetTouches.length === 1) {
       // detect single touch.
-      handleScroll(event, targetElement);
+      handleScroll(event, targetElement, isHorizontal);
     }
   };
 }
@@ -138,7 +147,14 @@ export default {
     if (!isIosDevice()) {
       simpleLock();
     } else {
+      // lock everything but target element
       advancedLock(targetElement);
+      // lock everything but disabled targets with vertical scrolling
+      const disabledTargets = document.querySelectorAll(`[${SCROLL_LOCK_DISABLE_ATTR}]`);
+      disabledTargets.forEach(target => advancedLock(target));
+      // lock everything but disabled targets with horizontal scrolling
+      const disabledHorizontalTargets = document.querySelectorAll(`[${SCROLL_LOCK_DISABLE_HORIZONTAL_ATTR}]`);
+      disabledHorizontalTargets.forEach(target => advancedLock(target, true));
     }
     isLocked = true;
   },
@@ -152,6 +168,10 @@ export default {
     if (isIosDevice()) {
       // revert the old scroll position
       advancedUnlock(targetElement);
+      // revert the old scroll position for disabled targets
+      const disabledTargets = document.querySelectorAll(`[${SCROLL_LOCK_DISABLE_ATTR}]`);
+      const disabledHorizontalTargets = document.querySelectorAll(`[${SCROLL_LOCK_DISABLE_HORIZONTAL_ATTR}]`);
+      [...disabledTargets, ...disabledHorizontalTargets].forEach(target => advancedUnlock(target));
     } else {
       // remove all inline styles added by the `simpleLock` function
       document.body.style.removeProperty('overflow');

tests/unit/components/Filter/FilterInput.spec.js

@@ -378,7 +378,7 @@ describe('FilterInput', () => {
       });
       expect(selectedTags.props('activeTags')).toEqual(tags);
       // assert we tried to ficus first item
-      expect(spy).toHaveBeenCalledWith(0);
+      expect(spy).not.toHaveBeenCalled();
     });
 
     it('on paste, handles clipboard in HTML format, when copying and pasting from search directly', () => {
@@ -563,7 +563,7 @@ describe('FilterInput', () => {
       await wrapper.vm.$nextTick();
       // first time was `true`, from `focus`, then `blur` made it `false`
       expect(wrapper.emitted('show-suggested-tags')).toEqual([[true], [false]]);
-      expect(suggestedTags.exists()).toBe(false);
+      expect(suggestedTags.attributes('style')).toContain('display: none');
     });
 
     it('deletes `suggestedTags` component when `deleteButton` looses its focus on an external component', async () => {
@@ -576,7 +576,7 @@ describe('FilterInput', () => {
       });
       await wrapper.vm.$nextTick();
       expect(wrapper.emitted('show-suggested-tags')).toEqual([[true], [false]]);
-      expect(suggestedTags.exists()).toBe(false);
+      expect(suggestedTags.attributes('style')).toContain('display: none');
     });
 
     it('does not hide the tags, if the new focus target matches `input, button`, inside the main component', async () => {
@@ -598,7 +598,7 @@ describe('FilterInput', () => {
 
       await flushPromises();
 
-      expect(suggestedTags.exists()).toBe(false);
+      expect(suggestedTags.attributes('style')).toContain('display: none');
       expect(wrapper.emitted('show-suggested-tags')).toEqual([[true], [false]]);
     });
 
@@ -613,7 +613,7 @@ describe('FilterInput', () => {
       });
 
       await wrapper.vm.$nextTick();
-      expect(suggestedTags.exists()).toBe(false);
+      expect(suggestedTags.attributes('style')).toContain('display: none');
       expect(wrapper.emitted('show-suggested-tags')).toEqual([[true], [false]]);
       expect(wrapper.emitted('blur')).toBeTruthy();
     });
@@ -637,7 +637,7 @@ describe('FilterInput', () => {
       });
 
       it('deletes `suggestedTags` component when no tags available', () => {
-        expect(suggestedTags.exists()).toBe(false);
+        expect(suggestedTags.attributes('style')).toContain('display: none');
       });
 
       it('does not render `deleteButton` when there are no tags and `input` is empty', () => {

tests/unit/utils/scroll-lock.spec.js

@@ -8,7 +8,7 @@
  * See https://swift.org/CONTRIBUTORS.txt for Swift project authors
 */
 
-import scrollLock, { SCROLL_LOCK_DISABLE_ATTR } from 'docc-render/utils/scroll-lock';
+import scrollLock, { SCROLL_LOCK_DISABLE_ATTR, SCROLL_LOCK_DISABLE_HORIZONTAL_ATTR } from 'docc-render/utils/scroll-lock';
 import { createEvent, parseHTMLString } from '../../../test-utils';
 
 const { platform } = window.navigator;
@@ -31,10 +31,13 @@ describe('scroll-lock', () => {
     DOM = parseHTMLString(`
       <div class="container">
         <div class="scrollable">long</div>
+        <div ${SCROLL_LOCK_DISABLE_ATTR}="true" class="disabled-target"></div>
+        <div ${SCROLL_LOCK_DISABLE_HORIZONTAL_ATTR}="true" class="disabled-horizontal-target"></div>
       </div>
     `);
     document.body.appendChild(DOM);
     container = DOM.querySelector('.container');
+    Object.defineProperty(container, 'scrollHeight', { value: 10, writable: true });
   });
   afterEach(() => {
     window.navigator.platform = platform;
@@ -70,12 +73,7 @@ describe('scroll-lock', () => {
       container.ontouchmove(touchMoveEvent);
       expect(preventDefault).toHaveBeenCalledTimes(1);
       expect(stopPropagation).toHaveBeenCalledTimes(0);
-      expect(touchMoveEvent.target.closest).toHaveBeenCalledTimes(1);
-      expect(touchMoveEvent.target.closest).toHaveBeenCalledWith(`[${SCROLL_LOCK_DISABLE_ATTR}]`);
-
       // simulate scroll middle
-      // simulate we have enough to scroll
-      Object.defineProperty(container, 'scrollHeight', { value: 10, writable: true });
       container.ontouchmove({ ...touchMoveEvent, targetTouches: [{ clientY: -10 }] });
       expect(preventDefault).toHaveBeenCalledTimes(1);
       expect(stopPropagation).toHaveBeenCalledTimes(1);
@@ -86,26 +84,62 @@ describe('scroll-lock', () => {
       expect(preventDefault).toHaveBeenCalledTimes(2);
       expect(stopPropagation).toHaveBeenCalledTimes(1);
 
-      // simulate there is a scroll-lock-disable target
-      container.ontouchmove({
-        ...touchMoveEvent,
-        targetTouches: [{ clientY: -10 }],
-        target: {
-          closest: jest.fn().mockReturnValue({
-            ...container,
-            clientHeight: 150,
-          }),
-        },
-      });
-      // assert scrolling was allowed
-      expect(preventDefault).toHaveBeenCalledTimes(2);
-      expect(stopPropagation).toHaveBeenCalledTimes(2);
-
       scrollLock.unlockScroll(container);
       expect(container.ontouchmove).toBeFalsy();
       expect(container.ontouchstart).toBeFalsy();
     });
 
+    it('adds event listeners to the disabled targets too', () => {
+      const disabledTarget = DOM.querySelector('.disabled-target');
+      const disabledHorizontalTarget = DOM.querySelector('.disabled-horizontal-target');
+      // init the scroll lock
+      scrollLock.lockScroll(container);
+      // assert event listeners are attached
+      expect(disabledTarget.ontouchstart).toEqual(expect.any(Function));
+      expect(disabledTarget.ontouchmove).toEqual(expect.any(Function));
+      expect(disabledHorizontalTarget.ontouchstart).toEqual(expect.any(Function));
+      expect(disabledHorizontalTarget.ontouchmove).toEqual(expect.any(Function));
+
+      scrollLock.unlockScroll(container);
+      expect(disabledTarget.ontouchmove).toBeFalsy();
+      expect(disabledTarget.ontouchstart).toBeFalsy();
+      expect(disabledHorizontalTarget.ontouchstart).toBeFalsy();
+      expect(disabledHorizontalTarget.ontouchmove).toBeFalsy();
+    });
+
+    it('prevent event if user tries to perform vertical scroll in an horizontal scrolling element', () => {
+      // set horizontal scrolling element only
+      DOM = parseHTMLString(`
+        <div class="container">
+          <div class="scrollable">long</div>
+          <div ${SCROLL_LOCK_DISABLE_HORIZONTAL_ATTR}="true" class="disabled-horizontal-target"></div>
+        </div>
+      `);
+      document.body.appendChild(DOM);
+      container = DOM.querySelector('.container');
+
+      const touchStartEvent = {
+        targetTouches: [{ clientY: 0, clientX: 0 }],
+      };
+      // perform vertical scroll
+      const touchMoveEvent = {
+        targetTouches: [{ clientY: -10, clientX: 0 }],
+        preventDefault,
+        stopPropagation,
+        touches: [1],
+        target: {
+          closest: jest.fn(),
+        },
+      };
+
+      scrollLock.lockScroll(container);
+      container.ontouchstart(touchStartEvent);
+      container.ontouchmove(touchMoveEvent);
+
+      expect(preventDefault).toHaveBeenCalledTimes(1);
+      expect(stopPropagation).toHaveBeenCalledTimes(0);
+    });
+
     it('prevents body scrolling', () => {
       scrollLock.lockScroll(container);
       // assert body scroll is getting prevented when swiping up/down