bug: scrolling issues with bottom sheet modal

davideas commented 2 years ago

Prerequisites

[X] I have read the Contributing Guidelines.
[X] I agree to follow the Code of Conduct.
[X] I have searched for existing issues that already include this feature request, without success.

Describe the Feature Request

Possibility to scroll content if this exceeds screen size only when bottom sheet is fully expanded and touched the max breakpoint. Then, when finishing to scroll the content down and there's nothng to scroll anymore, the bottom sheet should be dragged again at lower breakpoint.

Describe the Use Case

Having longer content in bottom sheet, content that exceeds the screen height is not visible because container not scrollable. It's even a real problem for smaller screens.

Describe Preferred Solution

No response

Describe Alternatives

No response

Related Code

No response

Additional Information

No response

averyjohnston commented 2 years ago

Thanks for the issue. Could you replicate your use case in a starter app and post the link here? This sounds like it may just be a bug, so I want to make sure I'm following.

ionitron-bot[bot] commented 2 years ago

Thanks for the issue! This issue has been labeled as needs reproduction. This label is added to issues that need a code reproduction.

Please reproduce this issue in an Ionic starter application and provide a way for us to access it (GitHub repo, StackBlitz, etc). Without a reliable code reproduction, it is unlikely we will be able to resolve the issue, leading to it being closed.

If you have already provided a code snippet and are seeing this message, it is likely that the code snippet was not enough for our team to reproduce the issue.

For a guide on how to create a good reproduction, see our Contributing Guide.

davideas commented 2 years ago

I created a quick sample project here: https://ionic-angular-action-sheet-g1wwzx.stackblitz.io But it should be used in mobile view with any browser not desktop view! In desktop view, scrolling works, it is usable nicely with mouse wheel too, but the problem of course is on mobile view. There, the touch creates issues when content is longer than the screen.

I saw a css property on <bottom-sheet-modal> element in ion-page class: overflow: hidden; Well even modifying it or adding overflow to an internal div of the bottom sheet when touching the component it drag and scroll at same moment. That's why I suggest to activate overflow-y: auto only when max breakpont is reached. When at lower breakpoints the touch should only drag the bottom sheet up and down without interferring with the scrolling of the content.

Could you please verify how we can scroll this content?

averyjohnston commented 2 years ago

Thanks! I see what you mean now. After investigation, it looks like the behavior you want exists on ion-content -- once the sheet modal is at its max breakpoint, any ion-content components inside get the scroll-y class, allowing scrolling. Could you try replacing the div.modal-content with ion-content and let me know if that looks how you would expect?

Either way, our documentation around this should be improved, since there isn't any example HTML for ModalPage in the usage examples (https://ionicframework.com/docs/api/modal#usage).

matthewgoodman13 commented 2 years ago

I'd like to chime in here! Your fix definitely does help, that is wrapping the Modal's content inside a IonContent. This allows for cards to stick to the top of the modal until its fully opened and allows for proper scrolling.

Did notice a couple things though. 1) The time delay from once the modal is fully opened and when the content is able to be scrolled is noticeable (can be seen when the scrollbars become visible). Its about half a second. This does not allow for a smooth scrolling experience. 2) Once this scrolling enables - it becomes much harder to actually swipe to close the modal. If the modal's content is already at the top, the next "down" gesture should close the modal.

@davideas let me know if you can confirm the same experience.

Note that my experience have been with using Ionic with ReactJS. No CSS properties applied.

Code Snippet:

const MyBottomSheetModal = ({ showModal, setShowModal }) => {
  return (
    <IonModal
      isOpen={showModal}
      initialBreakpoint={0.5}
      breakpoints={[0, 0.5, 1]}
      onDidDismiss={() => setShowModal(false)}
      >
      <IonContent>
      {[...Array(10)].map((x, i) => (
        <IonCard key={i}>
          <IonCardHeader><IonCardTitle>Card Title</IonCardTitle></IonCardHeader>
          <IonCardContent><p>This is a card.</p></IonCardContent>
        </IonCard>
      )  )}
      </IonContent>
  </IonModal>
  );
}

davideas commented 2 years ago

Could you try replacing the div.modal-content with ion-content and let me know if that looks how you would expect?

Yes thanks @amandaesmith3 ! now at least is scrollable, well this wasn't documented :-)

Did notice a couple things though. 1) The time delay from once the modal is fully opened and when the content is able to be scrolled is noticeable (can be seen when the scrollbars become visible). Its about half a second. This does not allow for a smooth scrolling experience. 2) Once this scrolling enables - it becomes much harder to actually swipe to close the modal. If the modal's content is already at the top, the next "down" gesture should close the modal.

Unfortunately, I confirm both points from @matthewgoodman13. @amandaesmith3 Can we improve there? When maxed, it can be dragged down again only by the handle. Dragging from the center of the sheet is doesn't work anymore. EDIT: Dragging is not working anymore even if content fits the screen.
I also add another point I mentioned in my initial message, the "max breakpoint"! If the max isn't 1 than the dragging should stop at the max breakpoint we set, for example if max is 0.75 then the sheet should not move up anymore so that the content can scroll too, currently it allows the move and then snap down again, it happened the content scrolls too having the ui a bit broken (to reproduce slowly drag up). I modified the project to show the issue with max at 0.75.

matthewgoodman13 commented 2 years ago

I also add another point I mentioned in my initial message, the "max breakpoint"! If the max isn't 1 than the dragging should stop at the max breakpoint we set, for example if max is 0.75 then the sheet should not move up anymore so that the content can scroll too, currently it allows the move and then snap down again, it happened the content scrolls too having the ui a bit broken (to reproduce slowly drag up).

Getting this as well. I also do find it is important, especially if you want absolute positioned buttons at the bottom of a 75% modal. If the max breakpoint is at 75%, scrolling up creates extra "whitespace" and shouldn't be allowed.

matthewgoodman13 commented 2 years ago

I also add another point I mentioned in my initial message, the "max breakpoint"! If the max isn't 1 than the dragging should stop at the max breakpoint we set, for example if max is 0.75 then the sheet should not move up anymore so that the content can scroll too, currently it allows the move and then snap down again, it happened the content scrolls too having the ui a bit broken (to reproduce slowly drag up).

Getting this as well. I also do find it is important, especially if you want absolute positioned buttons at the bottom of a 75% modal. If the max breakpoint is at 75%, scrolling up creates extra "whitespace" and shouldn't be allowed.

I found a fix for the height thing. You can set modal height to 75vh, then use a max breakpoint of 1. Modal will not expand above this threshold. Gesture to close as discussed still needs to be fixed along with the "delay"

averyjohnston commented 2 years ago

Thanks for the extra details, folks. I'm able to replicate all issues mentioned, and in core, meaning they likely happen in all frameworks. To summarize:

Once the modal is fully opened, there is a noticeable delay before content becomes scrollable. (screencast)
Once you can scroll, dragging down on anywhere but the handle when the content is scrolled to the top doesn't do anything, when it should move the modal. (screencast)
The sheet modal usage examples need updating with example HTML for the modal, and a note about using ion-content to ensure things are scrollable.
When the max breakpoint is not 1, and the modal is fully opened, it can still be dragged up higher before snapping back down. (Workaround mentioned above: set --height: 75vh on the ion-modal, or whatever height you need, and use a max breakpoint of 1.) (screencast)
For the above scenario, scrolling on mobile is very difficult, since most of your gestures drag the modal around instead. This is a slightly separate issue from 4 as it happens in both directions -- if you're scrolled down, and drag down, the modal will usually move when this should scroll the content up instead. (screencast)
I found this one while investigating: for modals with a max breakpoint of less than 1, the bottom of the content is cut off and can't be scrolled to. (screencast) The --height: 75vh workaround works here too.

matthewgoodman13 commented 2 years ago

@amandaesmith3 Great article! Has this upgrade resolved all the found modal bugs?

averyjohnston commented 2 years ago

@matthewgoodman13 We're still working on these issues; you can see the full 6.1.0 changelog here: https://github.com/ionic-team/ionic-framework/releases/tag/v6.1.0

jakubkoje commented 1 year ago

No news I guess? Maybe the steps above should be at least mentioned in the documentation because without them, the sheet modal is unusable.

Captainfl4me commented 1 year ago

Has anyone found a fix for the second report issue ? This is definitly causing accessibility issue on ios as when fully extand the bar is near the phone status bar so you often open the notification center. I was thinking handling the problem myself by using custom gesture to move the modal when user reach top of the scroll but I don't really know how I can move the modal directly from Angular ?

durgamalleswariyeruva commented 10 months ago

@amandaejohnston , @liamdebeasi , As per the comment on jan 24, When the max breakpoint is not 1, and the modal is fully opened, it can still be dragged up higher before snapping back down. (Workaround mentioned above: set --height: 75vh on the ion-modal, or whatever height you need, and use a max breakpoint of 1. ---- I am not able to smoothly drag down without touching the handler.(If i can drag by touching the handle(top notch) initially then only sheetmodal is moving to previous breakpoint, otherwise i am not able to drag down especially in ios devices. currently using below properties inside sheetmodal [backdropDismiss]="false" [backdropBreakpoint]="1" [isOpen]="true" [initialBreakpoint]="0.25" [breakpoints]="[0.25,1]" [swipeToClose]="false"

Any help is appreciated. currently using ionic version 6.18.0 and angular 12 version.

liamdebeasi commented 6 months ago

Hi everyone,

I have an update on the status of this thread. There are several issues noted here, so I'll give updates per-issue:

There is a delay when re-enabling scrolling. This is being addressed in a PR on this repo.
Dragging down on a fully open modal does not do anything unless users swipe on the header. This is being a addressed in a PR on this repo.
Sheet modal documentation should note that IonContent is necessary. This is being addressed in a PR on the Ionic Docs repo.
Sheet modal can be dragged above max breakpoint before snapping back. This is being tracked in a separate issue. The main here is that once you hit the max breakpoint, you should not be able to drag up anymore. There should be a rubber band effect instead.
Scrolling up on a sheet modal with a max breakpoint of < 1 is difficult. The root cause here is the same as 4 due to our lack of support for the "rubber band" effect when swiping up past the max breakpoint. We need additional logic to detect when a user swipes beyond the max breakpoint. This will be tracked in the same thread as point 4 above.
Max breakpoint of <1 causes part of the modal to be inaccessible offscreen. Ionic is working as intended here. The max breakpoint essentially says "This is the max percentage of the modal that can be shown in the viewport". For example, if you have a max breakpoint of 0.9 that is the same as saying "At most 90% of the modal can be shown in the viewport". As a result, 10% will always be inaccessible outside of the viewport. Developers probably should not make the max breakpoint anything less than 1. It seems like the most common use case for this is to have a sheet modal that does not take up the full screen (but still has all of its content visible). In that case, developers should change the height of the modal rather than make the max breakpoint less than 1. ~I can look into some better documentation for this.~ edit: This is being addressed in https://github.com/ionic-team/ionic-docs/pull/3589.

Here is a dev build that should fix issues 1 and 2 if anyone is interested in testing: 7.8.3-dev.11712114476.152fc43d

liamdebeasi commented 6 months ago

Thanks for the issue. This has been resolved via https://github.com/ionic-team/ionic-framework/pull/29312, and a fix will be available in an upcoming release of Ionic Framework. Please see https://github.com/ionic-team/ionic-framework/issues/24583#issuecomment-2033478601 for updates on related work.

ionitron-bot[bot] commented 5 months ago

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

ionic-team / ionic-framework