From 36f3a19a200b47e2684d1bb4517b8a57e6c5b3d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marina=20A=C3=ADsa?= Date: Wed, 12 Jun 2024 17:19:53 +0200 Subject: [PATCH 1/3] Lock and unlock disabled targets --- src/components/Filter/FilterInput.vue | 5 ++- src/components/Filter/TagList.vue | 7 ++++ src/utils/scroll-lock.js | 31 ++++++++++------ .../components/Filter/FilterInput.spec.js | 12 +++---- tests/unit/utils/scroll-lock.spec.js | 35 ++++++++----------- 5 files changed, 52 insertions(+), 38 deletions(-) diff --git a/src/components/Filter/FilterInput.vue b/src/components/Filter/FilterInput.vue index dcdf72334..18e747aed 100644 --- a/src/components/Filter/FilterInput.vue +++ b/src/components/Filter/FilterInput.vue @@ -37,6 +37,7 @@
scrollHeight > scrollWidth; + /** * Handles the scrolling of the targetElement * @param {TouchEvent} event * @param {HTMLElement} targetElement * @return {boolean} */ -function handleScroll(event, targetElement) { - 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); - } +function handleScroll(event, target) { + if (isVerticalScroll(target)) { + const clientY = event.targetTouches[0].clientY - initialClientY; + 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); + if (isTargetElementTotallyScrolled(target) && clientY < 0) { + // element is at the bottom of its scroll. + return preventDefault(event); + } } // prevent the scroll event from going up to the parent/window @@ -138,7 +140,11 @@ export default { if (!isIosDevice()) { simpleLock(); } else { + // lock everything but target element advancedLock(targetElement); + // lock everything but disabled targets + const disabledTargets = document.querySelectorAll(`[${SCROLL_LOCK_DISABLE_ATTR}]`); + disabledTargets.forEach(target => advancedLock(target)); } isLocked = true; }, @@ -152,6 +158,9 @@ 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}]`); + disabledTargets.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..543879efe 100644 --- a/tests/unit/utils/scroll-lock.spec.js +++ b/tests/unit/utils/scroll-lock.spec.js @@ -31,10 +31,12 @@ 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 +72,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 +83,24 @@ 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'); + // 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)); + + scrollLock.unlockScroll(container); + expect(disabledTarget.ontouchmove).toBeFalsy(); + expect(disabledTarget.ontouchstart).toBeFalsy(); + }); + it('prevents body scrolling', () => { scrollLock.lockScroll(container); // assert body scroll is getting prevented when swiping up/down From f47301f93cc824c3ea61a507a363d2d5db40568a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marina=20A=C3=ADsa?= Date: Thu, 13 Jun 2024 15:53:40 +0200 Subject: [PATCH 2/3] It prevent event if user tries to perform vertical scroll in an horizontal scrolling element --- src/utils/scroll-lock.js | 9 ++++++++- tests/unit/utils/scroll-lock.spec.js | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/utils/scroll-lock.js b/src/utils/scroll-lock.js index 019607323..4507c225e 100644 --- a/src/utils/scroll-lock.js +++ b/src/utils/scroll-lock.js @@ -10,6 +10,7 @@ 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 export const SCROLL_LOCK_DISABLE_ATTR = 'data-scroll-lock-disable'; @@ -82,8 +83,10 @@ const isVerticalScroll = ({ scrollHeight, scrollWidth }) => scrollHeight > scrol * @return {boolean} */ function handleScroll(event, target) { + const clientY = event.targetTouches[0].clientY - initialClientY; + const clientX = event.targetTouches[0].clientX - initialClientX; + if (isVerticalScroll(target)) { - const clientY = event.targetTouches[0].clientY - initialClientY; if (target.scrollTop === 0 && clientY > 0) { // element is at the top of its scroll. return preventDefault(event); @@ -93,6 +96,9 @@ function handleScroll(event, target) { // 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); } // prevent the scroll event from going up to the parent/window @@ -114,6 +120,7 @@ function advancedLock(targetElement) { if (event.targetTouches.length === 1) { // detect single touch. initialClientY = event.targetTouches[0].clientY; + initialClientX = event.targetTouches[0].clientX; } }; targetElement.ontouchmove = (event) => { diff --git a/tests/unit/utils/scroll-lock.spec.js b/tests/unit/utils/scroll-lock.spec.js index 543879efe..1a2ec8aca 100644 --- a/tests/unit/utils/scroll-lock.spec.js +++ b/tests/unit/utils/scroll-lock.spec.js @@ -101,6 +101,30 @@ describe('scroll-lock', () => { expect(disabledTarget.ontouchstart).toBeFalsy(); }); + it('prevent event if user tries to perform vertical scroll in an horizontal scrolling element', () => { + Object.defineProperty(container, 'scrollWidth', { value: 100, writable: true }); + + const touchStartEvent = { + targetTouches: [{ clientY: 0, clientX: 0 }], + }; + 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 From 692444693adac907854195b03c9036cfbdb83f1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marina=20A=C3=ADsa?= Date: Mon, 17 Jun 2024 17:48:34 +0200 Subject: [PATCH 3/3] Create SCROLL_LOCK_DISABLE_HORIZONTAL_ATTR attribute --- src/components/Filter/FilterInput.vue | 6 +++--- src/components/Filter/TagList.vue | 6 +++--- src/utils/scroll-lock.js | 22 +++++++++++++--------- tests/unit/utils/scroll-lock.spec.js | 19 +++++++++++++++++-- 4 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/components/Filter/FilterInput.vue b/src/components/Filter/FilterInput.vue index 18e747aed..da4d9e6f9 100644 --- a/src/components/Filter/FilterInput.vue +++ b/src/components/Filter/FilterInput.vue @@ -37,7 +37,7 @@
window.navigator && window.navigator.platform @@ -74,19 +76,17 @@ function advancedUnlock(targetElement) { document.removeEventListener('touchmove', preventDefault); } -const isVerticalScroll = ({ scrollHeight, scrollWidth }) => scrollHeight > scrollWidth; - /** * Handles the scrolling of the targetElement * @param {TouchEvent} event * @param {HTMLElement} targetElement * @return {boolean} */ -function handleScroll(event, target) { +function handleScroll(event, target, isHorizontal) { const clientY = event.targetTouches[0].clientY - initialClientY; const clientX = event.targetTouches[0].clientX - initialClientX; - if (isVerticalScroll(target)) { + if (!isHorizontal) { if (target.scrollTop === 0 && clientY > 0) { // element is at the top of its scroll. return preventDefault(event); @@ -110,7 +110,7 @@ function handleScroll(event, target) { * 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; @@ -126,7 +126,7 @@ function advancedLock(targetElement) { targetElement.ontouchmove = (event) => { if (event.targetTouches.length === 1) { // detect single touch. - handleScroll(event, targetElement); + handleScroll(event, targetElement, isHorizontal); } }; } @@ -149,9 +149,12 @@ export default { } else { // lock everything but target element advancedLock(targetElement); - // lock everything but disabled targets + // 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; }, @@ -167,7 +170,8 @@ export default { advancedUnlock(targetElement); // revert the old scroll position for disabled targets const disabledTargets = document.querySelectorAll(`[${SCROLL_LOCK_DISABLE_ATTR}]`); - disabledTargets.forEach(target => advancedUnlock(target)); + 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/utils/scroll-lock.spec.js b/tests/unit/utils/scroll-lock.spec.js index 1a2ec8aca..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; @@ -32,6 +32,7 @@ describe('scroll-lock', () => {
long
+
`); document.body.appendChild(DOM); @@ -90,23 +91,37 @@ describe('scroll-lock', () => { 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', () => { - Object.defineProperty(container, 'scrollWidth', { value: 100, writable: true }); + // 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,