Fix Scroll Locking in ion-content/ion-scroll #2849
Conversation
| self.doTouchMove(getEventTouches(e), e.timeStamp, e.scale); | ||
| if(self.options.locking){ | ||
| if(!self.__scrollLockFix.direction){ | ||
| self.__scrollLockFix.direction = Math.abs(e.touches[0].pageX - self.__initialTouchLeft) > Math.abs(e.touches[0].pageY - self.__initialTouchTop); |
There was a problem hiding this comment.
Setting direction to false to indicate Y direction doesn't seem right. It will still fail the !self.__scrollLockFix.direction test in subsequent calls.
…erty; Added minimum length before fix occur
|
@gkalpak Thanks for spotting that issue. I rewrote the condition and added a small distance before the correction actually happens now. Let me know if there're other unnoticed issues. |
| @@ -772,7 +773,21 @@ ionic.views.Scroll = ionic.views.View.inherit({ | |||
| } | |||
| } | |||
|
|
|||
| self.doTouchMove(getEventTouches(e), e.timeStamp, e.scale); | |||
| if(self.options.locking && self.__scrollLockFix.direction === null){ | |||
There was a problem hiding this comment.
You could swap self.__scrollLockFix.direction === null for !self.__scrollLockFix.doFix (slightly less verbose and ("nit-pickingly") slightly more performant).
|
@gkalpak Updated code based on your suggestions. |
|
I was hoping this would fix a related problem, but does not. When putting scrollable ion-content inside of an ion-slid-box, the scrollable content DOES NOT lock on horizontal left/right drag, but DOES lock correctly on a vertical up/down drag. Ultimately, if you start a vertical or horizontal drag, the experience needs to lock in that single scroll direction. Currently you get an XY unprofessional user experience. Would this be a requested fix for slidebox or is there already an existing solution / workaround I'm overlooking? Example: |
|
@derek1906, can you add a unit test and codepen example? |
|
@perrygovier I have no idea how to create a unit test to test this but nevertheless, here's an example of a scrollable |
|
Derek http://lunabutor.hu/strategii_robot 25.03.2015, 04:08, "Derek Leung" [email protected]: I have no idea how to create a unit test to test this but nevertheless, here's of a scrollable ion-content with the fix on jsFiddle that can be tested by hand, and without the fix. You can see that the one without the fix can be scrolled in any direction even with locking set to true. —Reply to this email directly or . |
|
Greetings @derek1906! Thank you very much from all of us on the Ionic team for submitting this pull request. We appreciate your commitment to improving Ionic. Unfortunately, we are unable to merge this particular PR, as it may cause regressions or is already out of date with regard to recent fixes. We apologize, as we know how much time you may have put into the work. We hope you will continue to contribute to Ionic in the future. Please note that we are more likely to merge a smaller PR that has a passing test and changes only one meaningful feature in the project. For more information, please review our Pull Request Guidelines. Best regards, Ionitron |
This is a quick fix for the
lockingproperty forion-contentandion-scroll. In the current version of Ionic, settinglockingtotruewill not actually lock the scroll direction if dragged with an angle in the beginning.A more detailed description of the issue can be found here: http://forum.ionicframework.com/t/ion-content-scroll-locking-doesnt-work/14445
This fix is done by modifying the
doTouchMoveinvocation to only accept one change in direction at a time, iflockingis set totrue. This seems to fix the issue in my tests, however I'm not sure if there's anything else that is affected by this change.