Skip to content

Commit d0d2b33

Browse files
author
Dobromir Hristov
authored
Fix QuickNav Preview scrolling issue on iOS mobile device (#638)
resolves rdar://108269711
1 parent dc7d0bb commit d0d2b33

File tree

4 files changed

+37
-4
lines changed

4 files changed

+37
-4
lines changed

src/components/Navigator/QuickNavigationModal.vue

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,10 @@
6060
</p>
6161
</div>
6262
<template v-else>
63-
<div class="quick-navigation__refs">
63+
<div
64+
v-bind="{[SCROLL_LOCK_DISABLE_ATTR]: true}"
65+
class="quick-navigation__refs"
66+
>
6467
<Reference
6568
v-for="(symbol, index) in filteredSymbols"
6669
class="quick-navigation__reference"
@@ -120,6 +123,7 @@
120123
class="quick-navigation__preview"
121124
:json="previewJSON"
122125
:state="previewState"
126+
v-bind="{[SCROLL_LOCK_DISABLE_ATTR]: true}"
123127
/>
124128
</template>
125129
</div>
@@ -141,6 +145,7 @@ import keyboardNavigation from 'docc-render/mixins/keyboardNavigation';
141145
import LRUMap from 'docc-render/utils/lru-map';
142146
import { convertChildrenArrayToObject, getParents } from 'docc-render/utils/navigatorData';
143147
import { fetchDataForPreview } from 'docc-render/utils/data';
148+
import { SCROLL_LOCK_DISABLE_ATTR } from '@/utils/scroll-lock';
144149
145150
const { PreviewState } = QuickNavigationPreview.constants;
146151
@@ -174,6 +179,7 @@ export default {
174179
focusedInput: false,
175180
cachedSymbolResults: {},
176181
previewIsLoadingSlowly: false,
182+
SCROLL_LOCK_DISABLE_ATTR,
177183
};
178184
},
179185
props: {

src/utils/scroll-lock.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
let isLocked = false;
1212
let initialClientY = -1;
1313
let scrolledClientY = 0;
14+
// Adds this attribute to an inner scrollable element to allow it to scroll
15+
export const SCROLL_LOCK_DISABLE_ATTR = 'data-scroll-lock-disable';
1416

1517
const isIosDevice = () => window.navigator
1618
&& window.navigator.platform
@@ -79,12 +81,14 @@ function advancedUnlock(targetElement) {
7981
*/
8082
function handleScroll(event, targetElement) {
8183
const clientY = event.targetTouches[0].clientY - initialClientY;
82-
if (targetElement.scrollTop === 0 && clientY > 0) {
84+
// check if any parent has a scroll-lock disable, if not use the targetElement
85+
const target = event.target.closest(`[${SCROLL_LOCK_DISABLE_ATTR}]`) || targetElement;
86+
if (target.scrollTop === 0 && clientY > 0) {
8387
// element is at the top of its scroll.
8488
return preventDefault(event);
8589
}
8690

87-
if (isTargetElementTotallyScrolled(targetElement) && clientY < 0) {
91+
if (isTargetElementTotallyScrolled(target) && clientY < 0) {
8892
// element is at the bottom of its scroll.
8993
return preventDefault(event);
9094
}

tests/unit/components/Navigator/QuickNavigationModal.spec.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import QuickNavigationHighlighter from '@/components/Navigator/QuickNavigationHi
1616
import QuickNavigationModal from '@/components/Navigator/QuickNavigationModal.vue';
1717
import QuickNavigationPreview from '@/components/Navigator/QuickNavigationPreview.vue';
1818
import Reference from '@/components/ContentNode/Reference.vue';
19+
import { SCROLL_LOCK_DISABLE_ATTR } from '@/utils/scroll-lock';
1920
import { flushPromises } from '../../../../test-utils';
2021

2122
jest.mock('@/utils/data');
@@ -165,6 +166,7 @@ describe('QuickNavigationModal', () => {
165166
expect(wrapper.vm.debouncedInput).toBe(inputValue);
166167
expect(wrapper.findAll('.quick-navigation__symbol-match').length).toBe(filteredSymbols.length);
167168
expect(wrapper.find('.no-results').exists()).toBe(false);
169+
expect(wrapper.find('.quick-navigation__refs').attributes(SCROLL_LOCK_DISABLE_ATTR)).toBeTruthy();
168170
});
169171

170172
it('it renders the `no results found` string when no symbols are found given an input', () => {
@@ -375,6 +377,7 @@ describe('QuickNavigationModal', () => {
375377
const preview = wrapper.find(QuickNavigationPreview);
376378
expect(preview.exists()).toBe(true);
377379
expect(preview.props('state')).toBe(PreviewState.loading);
380+
expect(preview.attributes(SCROLL_LOCK_DISABLE_ATTR)).toBeTruthy();
378381
});
379382

380383
it('renders with a successful state and data when data is loaded', async () => {

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* See https://swift.org/CONTRIBUTORS.txt for Swift project authors
99
*/
1010

11-
import scrollLock from 'docc-render/utils/scroll-lock';
11+
import scrollLock, { SCROLL_LOCK_DISABLE_ATTR } from 'docc-render/utils/scroll-lock';
1212
import { createEvent, parseHTMLString } from '../../../test-utils';
1313

1414
const { platform } = window.navigator;
@@ -56,6 +56,9 @@ describe('scroll-lock', () => {
5656
preventDefault,
5757
stopPropagation,
5858
touches: [1],
59+
target: {
60+
closest: jest.fn(),
61+
},
5962
};
6063
// init the scroll lock
6164
scrollLock.lockScroll(container);
@@ -67,6 +70,8 @@ describe('scroll-lock', () => {
6770
container.ontouchmove(touchMoveEvent);
6871
expect(preventDefault).toHaveBeenCalledTimes(1);
6972
expect(stopPropagation).toHaveBeenCalledTimes(0);
73+
expect(touchMoveEvent.target.closest).toHaveBeenCalledTimes(1);
74+
expect(touchMoveEvent.target.closest).toHaveBeenCalledWith(`[${SCROLL_LOCK_DISABLE_ATTR}]`);
7075

7176
// simulate scroll middle
7277
// simulate we have enough to scroll
@@ -81,6 +86,21 @@ describe('scroll-lock', () => {
8186
expect(preventDefault).toHaveBeenCalledTimes(2);
8287
expect(stopPropagation).toHaveBeenCalledTimes(1);
8388

89+
// simulate there is a scroll-lock-disable target
90+
container.ontouchmove({
91+
...touchMoveEvent,
92+
targetTouches: [{ clientY: -10 }],
93+
target: {
94+
closest: jest.fn().mockReturnValue({
95+
...container,
96+
clientHeight: 150,
97+
}),
98+
},
99+
});
100+
// assert scrolling was allowed
101+
expect(preventDefault).toHaveBeenCalledTimes(2);
102+
expect(stopPropagation).toHaveBeenCalledTimes(2);
103+
84104
scrollLock.unlockScroll(container);
85105
expect(container.ontouchmove).toBeFalsy();
86106
expect(container.ontouchstart).toBeFalsy();

0 commit comments

Comments
 (0)