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

Panel, allowScrollOnElement on mobile issue. #10982

Closed
laiprorus opened this issue Oct 29, 2019 · 10 comments · Fixed by #11339
Closed

Panel, allowScrollOnElement on mobile issue. #10982

laiprorus opened this issue Oct 29, 2019 · 10 comments · Fixed by #11339

Comments

@laiprorus
Copy link
Contributor

laiprorus commented Oct 29, 2019

Environment Information

  • Package version(s): "office-ui-fabric-react": "^6.210.2"
  • Browser and OS versions: Safari on IPhone

Please provide a reproduction of the bug in a codepen

https://codepen.io/laiprorus/full/pooWjZw

Actual behavior:

The Panel and Overlay components use allowScrollOnElement function from https://github.com/OfficeDev/office-ui-fabric-react/blob/09d5abf9a89b1f2ae31de5427e24279537eb9a4c/packages/utilities/src/scroll.ts.
This function creates touchmove events that prevent the user from paning or zooming document body.
In my codepen, if you use a page on mobile, and use touch pinch to zoom in for example on the button, and then click, panel will open. But since you are zoomed in, you will only see part of it. And since "touchmove" events are prevented, you cant scroll to panels content.
There is no way for my code, that uses Panel component to disable those blocking events and it is also impossible to ensure the user is fully zoomed out before panel opens. And even if user is zoomed out, default behaviour on mobile for elements such as text input field, is to zoom in on that input field, and then the user has hard time to zoom out back.
Im not sure if its a bug or missing feature, but it is pretty breaking behavior for my current project. My workaround was to render this component inside Panel content to enable zoom and paning. const EnableScrollFix: React.FunctionComponent<{}> = () => { React.useEffect(() => { Fabric.enableBodyScroll(); $("div[data-is-scrollable]").each((index, element) => { const contents = $(element).contents(); $(element).replaceWith($("<div></div>").append(contents)); }); return () => { }; }, []); return (null); };
this components is empty, and is just used to get to that "componentDidMount" callback of Panel content. It replaces div that blocks events, with empty div, since i couldnt remove event otherwise. But its very cheesy solution, and especially risky since i am modifying dom elements created by react, with out its knowledge.

Expected behavior:

After Panel opens, user should be able to pan and zoom on mobile devices.

Priorities and help requested:

Are you willing to submit a PR to fix? No

Requested priority:
Blocking

Products/sites affected: corporate internal web application.

@maxwellred
Copy link
Contributor

Thank you @laiprorus! I'll look into this shortly and get back to you.

@laiprorus
Copy link
Contributor Author

laiprorus commented Oct 29, 2019

here screenshots of reproduced behavior
https://imgur.com/gallery/emLLZgi

  1. zoomed out
  2. zoomed in
    3-4 red is area that has scrolled blocked, cant scroll vertically or horizontally with finger in this are. green area sometimes work, since textfield component (or input element that is used by it) allows touch events, but fabric code throws errors in console that it cant use e.preventDefault() on this one.

and another one.
https://imgur.com/gallery/8CkDwsN
in this one user is almost stuck and cannot scroll anywhere.

@laiprorus
Copy link
Contributor Author

i looked more into the issue. Panel applies allowScrollOnElement to its content. And Overlay, that is created by Panel component, calls Fabric.disableBodyScroll after being mounted. Both of these then create events that then block touch scrolling/paning/zooming on mobile devices.
My workaround from original post caused other issues, so i am now looking for something else. I will try to modify fabric javascript directly, and if i find a solution i will post it and maybe create PR.

@maxwellred
Copy link
Contributor

Thank you @laiprorus! I'm still investigating this and other filed issues so PRs are always welcome!

@laiprorus
Copy link
Contributor Author

Here is my pull request that allows a workaround for this issue.
#11339

@maxwellred
Copy link
Contributor

@laiprorus Great job and thank you very much for the PR! It looks like there's some merge conflicts preventing the ci/cd checks from running so it'd be great if you could take a look at those when possible, thanks!

@laiprorus
Copy link
Contributor Author

@maxwellred I resolved the conflict using webtool of github. So i guess now we should just wait for approval or feedback? Or is there anything i could do now?

@maxwellred
Copy link
Contributor

@laiprorus Thanks! I'll reach out to the code owners and see if I can get eyes on.

@msft-github-bot
Copy link
Contributor

🎉This issue was addressed in #11339, which has now been successfully released as @uifabric/utilities@v7.8.0.:tada:

Handy links:

@msft-github-bot
Copy link
Contributor

🎉This issue was addressed in #11339, which has now been successfully released as office-ui-fabric-react@v7.76.0.:tada:

Handy links:

@microsoft microsoft locked as resolved and limited conversation to collaborators Jan 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants