Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: ion-content is still scrollable while ion-menu is opening. #21193

Closed
BerkeAras opened this issue May 2, 2020 · 27 comments · Fixed by #26976
Closed

bug: ion-content is still scrollable while ion-menu is opening. #21193

BerkeAras opened this issue May 2, 2020 · 27 comments · Fixed by #26976
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@BerkeAras
Copy link

Bug Report

Ionic version:

[x] 4.x
[x] 5.x

Current behavior:

ion-content is still scrollable while ion-menu is opening. This is happens only with cordova and capacitor, not in the browser.

Expected behavior:

ion-content should not be scrollable.

Steps to reproduce:

While opening a ion-menu, swipe finger up and down.

Other information:

Here you can see it: https://www.youtube.com/watch?v=K-ImPdKFoiA

@ionitron-bot ionitron-bot bot added the triage label May 2, 2020
@lincolnthree
Copy link

lincolnthree commented May 4, 2020

Confirmed here as well, on all platforms/modes (iOS/Android/Web/etc). This happens easily and consistently and results in a VERY slow/jittery interaction for the user.

@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report labels May 6, 2020
@ionitron-bot ionitron-bot bot removed the triage label May 6, 2020
@BerkeAras
Copy link
Author

Are there any updates?

@lincolnthree
Copy link

Still an issue. Seems to have gotten worse specifically in the latest chrome releases. Almost impossible to swipe open any menu without extreme lag / jank. It's almost to the point where I'd consider disabling swipe-to-open because it's just a really bad impression.

Users have been complaining our apps are slow when in reality it's just the menu opening :/

Not sure how to proceed with this as I believe it's a result of the event being captured by multiple elements.

@lincolnthree
Copy link

I stumbled on this, haven't had a chance to dig in too deep yet, but could be useful for finding a solution/workaround?

pawelgrzybek/siema#37

@lincolnthree
Copy link

Possibly related: w3c/pointerevents#303

@BerkeAras
Copy link
Author

Eventually it is possible to disable the scroll feature of IonContent as soon as the menu gets opened.

@BerkeAras
Copy link
Author

I will create create a demo of my solution ASAP.

@lincolnthree
Copy link

lincolnthree commented Aug 29, 2020

Awesome, thank you! I actually think I just came up with a solution as well. Does yours look something like this?

@liamdebeasi This seems to cancel the touchmove event on IonContent scrollable element.

// mainElmt is the <main> element from IonContent: <main class="inner-scroll scroll-x scroll-y" part="scroll"><slot></slot></main>

  constructor(
    private _rnd: Renderer2,
    private _dom: DomController)
{ ... }

// Called from menu (ionWillOpen) --->

  lockContent() {
      this._dom.write(() => {
        this._rnd.setStyle(this.mainElmt, 'pointer-events', 'none');
        Timeout.fn(() => {
          this._dom.write(() => {
            this._rnd.removeStyle(this.mainElmt, 'pointer-events');
          });
        }, 0);
      });
  }

@BerkeAras
Copy link
Author

@lincolnthree That looks very good, I've just started to create something. Are you able to create a PR?

@lincolnthree
Copy link

lincolnthree commented Aug 29, 2020

@BerkeAras I probably can tomorrow. Right now I've done this in a directive that attaches to every [ion-content] element automatically.

What was your solution?

@BerkeAras
Copy link
Author

I dont have a solution. I just created a new project, but I dont have something yet.

@lincolnthree
Copy link

lincolnthree commented Aug 29, 2020

Ah :) Here's something that might help:

@Directive({
  selector: 'ion-content'
})
export class IonContentDirective implements OnInit, OnDestroy {

  pricate static readonly locks: IonContentStyleDirective[] = [];

  private mainElmt: HTMLElement;
  private styled = false;

  constructor(
    private _el: ElementRef,
    private _rnd: Renderer2,
    private _dom: DomController
  ) {
    // Very hacky singleton. Don't look at me like that.
    IonContentStyleDirective.locks.push(this);
  }

  static cancelContentScrollEvents() {
    for (const c of IonContentStyleDirective.locks) {
      c.cancelPointerEvents();
    }
  }

  static activateContentScrollEvents() {
    for (const c of IonContentStyleDirective.locks) {
      c.activatePointerEvents();
    }
  }

  cancelPointerEvents() {
    console.log('cancel pointer events', this._el.nativeElement)
    this._dom.write(() => {
      this._rnd.setStyle(this.mainElmt, 'pointer-events', 'none');
    });
  }

  activatePointerEvents() {
    console.log('activate pointer events', this._el.nativeElement)
    this._dom.write(() => {
      this._rnd.removeStyle(this.mainElmt, 'pointer-events');
    });
  }

  ngOnInit() {
  }

  ngOnDestroy() {
    IonContentStyleDirective.locks.splice(IonContentStyleDirective.locks.indexOf(this), 1);
  }
}

@lincolnthree
Copy link

There may be a simpler way, but that's what I did in ~10 minutes.

@lincolnthree
Copy link

I also noticed it helps to do this when the menu is being closed, as well.

@BerkeAras
Copy link
Author

I've just created a very basic version of @lincolnthree's version. This is useful for especially No-Framework Users.

https://github.com/BerkeAras/Ionic-5-Menu-Swipe-Issue

@lincolnthree
Copy link

lincolnthree commented Aug 31, 2020

@liamdebeasi Not sure my tag came through since I edited it in. Just making sure you saw it, since I believe we have a viable workaround/fix for this if you want to incorporate it.

FYI. This seems to work best when it is ALSO called on menu ionWillOpen and ionWillClose, as menus may also have content inside them that can scroll when closing.

So something like this:

  _menuWillClose(side: string) {
    IonContentStyleDirective.cancelContentScrollEvents();
  }
  _menuDidClose(side: string) {
    IonContentStyleDirective.activateContentScrollEvents();
  }
  _menuWillOpen(side: string) {
    IonContentStyleDirective.cancelContentScrollEvents();
  }
  _menuDidOpen(side: string) {
    IonContentStyleDirective.activateContentScrollEvents();
  }

@lincolnthree
Copy link

lincolnthree commented Sep 3, 2020

As an additional aside. It seems to improve/reduce menu jitter if the ion-content::part(scroll) has touch-action: pan-y set in CSS. I believe this prevents the browser from attempting to attach horizontal (x-axis) touch events to this element in the first place.

@lincolnthree
Copy link

@liamdebeasi Facepalm... 🤦 I think the simplest fix is actually just to apply touch-action: pan-y to the main/scroll element in ion-content. I haven't tested in "all browsers" yet, but just this CSS change seems to resolve all the background-scrolling and jitter issues on chrome/android/and ios safari.

@lincolnthree
Copy link

That said, I'm going to get back to my "day job" and let the dust settle on this one for a bit.

@liamdebeasi
Copy link
Member

Hi everyone,

Does this issue still reproduce in the latest version of Ionic 6?

@liamdebeasi liamdebeasi added the needs: reply the issue needs a response from the user label Mar 13, 2023
@lincolnthree
Copy link

@liamdebeasi I will try with version 7 and let you know. Unfortunately I do not have an active build with Ionic 6 anymore :)

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Mar 14, 2023
@lincolnthree
Copy link

Yes, this is still an issue in version 7:

ScreenFlow.mp4

@lincolnthree
Copy link

Uhhh.. here's a link. Not sure what happened to that video: https://photos.app.goo.gl/xubuaC4PkMKf9syX6

@liamdebeasi liamdebeasi added type: bug a confirmed bug report and removed triage type: bug a confirmed bug report labels Mar 16, 2023
@liamdebeasi
Copy link
Member

Here's a dev build with a proposed fix if you are interested in testing:

v6: 6.6.3-dev.11678980356.1ab22fc7

v7: 7.0.0-dev.11678980795.1e4cbe0d

@lincolnthree
Copy link

Looks good @liamdebeasi -- And nice fix! Thanks for doing this :)

@liamdebeasi
Copy link
Member

Thanks for the issue. This has been resolved via #26976, and a fix will be available in an upcoming release of Ionic Framework.

@ionitron-bot
Copy link

ionitron-bot bot commented Apr 16, 2023

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.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Apr 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants