diff --git a/src/components/Filter/FilterInput.vue b/src/components/Filter/FilterInput.vue index dcdf72334..da4d9e6f9 100644 --- a/src/components/Filter/FilterInput.vue +++ b/src/components/Filter/FilterInput.vue @@ -37,6 +37,7 @@
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'); diff --git a/tests/unit/components/Filter/FilterInput.spec.js b/tests/unit/components/Filter/FilterInput.spec.js index 0a4ba178b..3927a5376 100644 --- a/tests/unit/components/Filter/FilterInput.spec.js +++ b/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', () => { diff --git a/tests/unit/utils/scroll-lock.spec.js b/tests/unit/utils/scroll-lock.spec.js index 3a67eede6..5fbaad491 100644 --- a/tests/unit/utils/scroll-lock.spec.js +++ b/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(`
long
+
+
`); 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(` +
+
long
+
+
+ `); + 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