Skip to content

Commit fbfd57a

Browse files
authored
fix(modal): sheet modal can be swiped down on content (#29260)
Issue number: Part of #24583 --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> When a sheet modal is fully opened swiping down on the IonContent when the content is scrolled to top does not move the sheet modal down. This does not align with the card modal where doing the same actions causes the card modal to move down. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Swiping down on the content now moves the sheet modal down as long as the content is scrolled to the top. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> Dev build: `7.8.3-dev.11712114476.152fc43d` Reviewers: Please test this on physical iOS and Android devices. Please check the following: 1. When a modal is fully opened, ensure that users can still scroll down (i.e. swipe up on the content to scroll down). 2. When a modal is fully opened, ensure that users can swipe down on the modal to dismiss when the content is scrolled to the top. 3. Repeat step 2 but ensure that swiping down scrolls content to the top when content is NOT already scrolled to the top. **Card Behavior** | Demo | | - | | <video src="https://github.com/ionic-team/ionic-framework/assets/2721089/910755c8-808e-4e0b-85d2-9a56a3b127c4"></video> | **Sheet Behavior** | `main` | branch | | - | - | | <video src="https://github.com/ionic-team/ionic-framework/assets/2721089/eb0b740f-b644-4602-93f9-8c634458239b"></video> | <video src="https://github.com/ionic-team/ionic-framework/assets/2721089/3844983e-ebbc-43f3-b5c2-20aad738b201"></video> |
1 parent 9b3cf9f commit fbfd57a

File tree

1 file changed

+36
-13
lines changed
  • core/src/components/modal/gestures

1 file changed

+36
-13
lines changed

core/src/components/modal/gestures/sheet.ts

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1+
import { isIonContent, findClosestIonContent } from '@utils/content';
12
import { createGesture } from '@utils/gesture';
2-
import { clamp, raf } from '@utils/helpers';
3+
import { clamp, raf, getElementRoot } from '@utils/helpers';
34

45
import type { Animation } from '../../../interface';
56
import type { GestureDetail } from '../../../utils/gesture';
@@ -142,22 +143,35 @@ export const createSheetGesture = (
142143

143144
const canStart = (detail: GestureDetail) => {
144145
/**
145-
* If the sheet is fully expanded and
146-
* the user is swiping on the content,
147-
* the gesture should not start to
148-
* allow for scrolling on the content.
146+
* If we are swiping on the content, swiping should only be possible if the content
147+
* is scrolled all the way to the top so that we do not interfere with scrolling.
148+
*
149+
* We cannot assume that the `ion-content` target will remain consistent between swipes.
150+
* For example, when using ion-nav within a modal it is possible to swipe, push a view,
151+
* and then swipe again. The target content will not be the same between swipes.
149152
*/
150-
const content = (detail.event.target! as HTMLElement).closest('ion-content');
153+
const contentEl = findClosestIonContent(detail.event.target! as HTMLElement);
151154
currentBreakpoint = getCurrentBreakpoint();
152155

153-
if (currentBreakpoint === 1 && content) {
154-
return false;
156+
if (currentBreakpoint === 1 && contentEl) {
157+
/**
158+
* The modal should never swipe to close on the content with a refresher.
159+
* Note 1: We cannot solve this by making this gesture have a higher priority than
160+
* the refresher gesture as the iOS native refresh gesture uses a scroll listener in
161+
* addition to a gesture.
162+
*
163+
* Note 2: Do not use getScrollElement here because we need this to be a synchronous
164+
* operation, and getScrollElement is asynchronous.
165+
*/
166+
const scrollEl = isIonContent(contentEl) ? getElementRoot(contentEl).querySelector('.inner-scroll') : contentEl;
167+
const hasRefresherInContent = !!contentEl.querySelector('ion-refresher');
168+
return !hasRefresherInContent && scrollEl!.scrollTop === 0;
155169
}
156170

157171
return true;
158172
};
159173

160-
const onStart = () => {
174+
const onStart = (detail: GestureDetail) => {
161175
/**
162176
* If canDismiss is anything other than `true`
163177
* then users should be able to swipe down
@@ -173,11 +187,10 @@ export const createSheetGesture = (
173187
canDismissBlocksGesture = baseEl.canDismiss !== undefined && baseEl.canDismiss !== true && minBreakpoint === 0;
174188

175189
/**
176-
* If swiping on the content
177-
* we should disable scrolling otherwise
178-
* the sheet will expand and the content will scroll.
190+
* If we are pulling down, then it is possible we are pulling on the content.
191+
* We do not want scrolling to happen at the same time as the gesture.
179192
*/
180-
if (contentEl) {
193+
if (detail.deltaY > 0 && contentEl) {
181194
contentEl.scrollY = false;
182195
}
183196

@@ -193,6 +206,16 @@ export const createSheetGesture = (
193206
};
194207

195208
const onMove = (detail: GestureDetail) => {
209+
/**
210+
* If we are pulling down, then it is possible we are pulling on the content.
211+
* We do not want scrolling to happen at the same time as the gesture.
212+
* This accounts for when the user scrolls down, scrolls all the way up, and then
213+
* pulls down again such that the modal should start to move.
214+
*/
215+
if (detail.deltaY > 0 && contentEl) {
216+
contentEl.scrollY = false;
217+
}
218+
196219
/**
197220
* Given the change in gesture position on the Y axis,
198221
* compute where the offset of the animation should be

0 commit comments

Comments
 (0)