Closed jskrzypek closed 8 years ago
Fix submitted as a PR.
This bug seems to be present on android as well as iOS, which makes sense given that it's coming from the drag gesture code and not from platform-specific code or implementations.
See #4022 for documentation of the issue on Android 4.4 webview
Just tested this on ios and android. Seems to be working fine for me with 1.0.1. Is there any other information you could provide? I can't seem to replicate it.
@mhartington I'll be in the office tomorrow and push up a repo. I'm happy to help you reproduce on Monday.
I'm having trouble replicating the problem. Fixing the code the way I mentioned fixes the bug we see in our app, but I am having trouble recreating the issue in a demonstration app. I will update once I manage that.
Ok I got it. (The problem with replication was happening because of some very strange behaviour when the html template from my pen is in the state passed to $urlRouterProvider.otherwise()
. But I'll document to that later. Also I cannot replicate the error on my android device, but it looks like the same thing that is blocking the bug from the $urlRouterProvider
might be (incorrectly) preventing the error from happening on android.)
Here is a repo that demonstrates the bug we see: https://github.com/jskrzypek/4008-preventDefault.
Shameless plug side-note: It's built with
generator-mcfly
our yeoman generator that gives you a pre-wired gulp & browserify build system for angular apps and options to automatically integrate ionic.
Steps to reproduce:
Clone the repo & cd
into the directory
git clone https://github.com/jskrzypek/4008-preventDefault#master && cd 4008-preventDefault
Install npm & bower dependencies
npm install && bower install
Use git to apply a patch to stick some console.log
messages on the Drag
handler
git apply ionic.js.patch
Use our gulp task to build the distributed code (using browserify)
gulp dist
cd
into the directory where the ionic project sits
cd dist/app/dev
Add platforms
ionic platform add android ios
ionic run ...
. Note that I am currently only able to reproduce this bug in ios.ios
or chrome://inspect#devices for android
)ion-list
you should be able to scroll fine via dragging, and should see messages logged by the drag handler from our patch.ion-content
. On ios you will not be able to scroll via dragging, but messages will be logged to the console showing us that the ev.preventDefault()
method is being called on these legitimate drag events.drag-handler.js
script that swaps out the current version of the handler for the one with my suggested fixes.ion-content
tab and ev.preventDefault()
is not being called when it shouldn't be.@mhartington Just letting you know that the repo is live with the bug replication. The other issue that was blocking it, I'm not really sure how I should start documenting...
For me it is working on android devices, but still not scrolling on ios
http://forum.ionicframework.com/t/overflow-scroll-true-ios-scrolling-not-working/28007
@jskrzypek i checked your repo out and tested it with IOS and i love your red heart but I have no idea what it does. It would fix my Problem.
+1
@Gunbit All that button does is run a script that replaces ionic.Gestures.gestures.Drag.handler
with a nearly identical function that has this change in it:
// Prevent gestures that are not intended for this event handler from firing subsequent times
- if (inst.options.prevent_default_directions.length === 0
- || inst.options.prevent_default_directions.indexOf(ev.direction) != -1) {
+ if (inst.options.prevent_default_directions.length > 0
+ && inst.options.prevent_default_directions.indexOf(ev.direction) != -1) {
ev.srcEvent.preventDefault();
Those lines are found here: https://github.com/driftyco/ionic/blob/master/js/utils/gestures.js#L1174-L1175 Thanks should go to @ghaiat for locating the bug :smile:
A workaround to implement this fix would be to add the following run block. You might put also want to wrap it in a deviceready
or $ionicPlatform.ready()
listener to make sure this happens after ionic
is attached to the window
.
.run(function() {
ionic.Gestures.gestures.Drag.handler = function dragGesture(ev, inst) {
if(ev.srcEvent.type == 'touchstart' || ev.srcEvent.type == 'touchend') {
this.preventedFirstMove = false;
} else if(!this.preventedFirstMove && ev.srcEvent.type == 'touchmove') {
// Prevent gestures that are not intended for this event handler from firing subsequent times
if(inst.options.prevent_default_directions.length > 0 && inst.options.prevent_default_directions.indexOf(ev.direction) != -1) {
ev.srcEvent.preventDefault();
}
this.preventedFirstMove = true;
}
// current gesture isnt drag, but dragged is true
// this means an other gesture is busy. now call dragend
if(ionic.Gestures.detection.current.name != this.name && this.triggered) {
inst.trigger(this.name + 'end', ev);
this.triggered = false;
return;
}
// max touches
if(inst.options.drag_max_touches > 0 &&
ev.touches.length > inst.options.drag_max_touches) {
return;
}
switch(ev.eventType) {
case ionic.Gestures.EVENT_START:
this.triggered = false;
break;
case ionic.Gestures.EVENT_MOVE:
// when the distance we moved is too small we skip this gesture
// or we can be already in dragging
if(ev.distance < inst.options.drag_min_distance &&
ionic.Gestures.detection.current.name != this.name) {
return;
}
// we are dragging!
if(ionic.Gestures.detection.current.name != this.name) {
ionic.Gestures.detection.current.name = this.name;
if(inst.options.correct_for_drag_min_distance) {
// When a drag is triggered, set the event center to drag_min_distance pixels from the original event center.
// Without this correction, the dragged distance would jumpstart at drag_min_distance pixels instead of at 0.
// It might be useful to save the original start point somewhere
var factor = Math.abs(inst.options.drag_min_distance / ev.distance);
ionic.Gestures.detection.current.startEvent.center.pageX += ev.deltaX * factor;
ionic.Gestures.detection.current.startEvent.center.pageY += ev.deltaY * factor;
// recalculate event data using new start point
ev = ionic.Gestures.detection.extendEventData(ev);
}
}
// lock drag to axis?
if(ionic.Gestures.detection.current.lastEvent.drag_locked_to_axis || (inst.options.drag_lock_to_axis && inst.options.drag_lock_min_distance <= ev.distance)) {
ev.drag_locked_to_axis = true;
}
var last_direction = ionic.Gestures.detection.current.lastEvent.direction;
if(ev.drag_locked_to_axis && last_direction !== ev.direction) {
// keep direction on the axis that the drag gesture started on
if(ionic.Gestures.utils.isVertical(last_direction)) {
ev.direction = (ev.deltaY < 0) ? ionic.Gestures.DIRECTION_UP : ionic.Gestures.DIRECTION_DOWN;
} else {
ev.direction = (ev.deltaX < 0) ? ionic.Gestures.DIRECTION_LEFT : ionic.Gestures.DIRECTION_RIGHT;
}
}
// first time, trigger dragstart event
if(!this.triggered) {
inst.trigger(this.name + 'start', ev);
this.triggered = true;
}
// trigger normal event
inst.trigger(this.name, ev);
// direction event, like dragdown
inst.trigger(this.name + ev.direction, ev);
// block the browser events
if((inst.options.drag_block_vertical && ionic.Gestures.utils.isVertical(ev.direction)) ||
(inst.options.drag_block_horizontal && !ionic.Gestures.utils.isVertical(ev.direction))) {
ev.preventDefault();
}
break;
case ionic.Gestures.EVENT_END:
// trigger dragend
if(this.triggered) {
inst.trigger(this.name + 'end', ev);
}
this.triggered = false;
break;
}
};
})
@mhartington Any progress on checking this out?
Merged this into our 1.0.2 branch e10b5d23c9cb3e04c6374574303e55aef6aefee4
@mhartington Awesome, thanks!
@jskrzypek Your fixed have a side effects(ion-fresher can only work once)
@huip can you elaborate what you mean
@jskrzypek I mean if ion-content contains ion-fresher tag, when pull down to refresh item-list,the pull-down events can not work.
I just updated the ionic lib to 1.0.1 and got the same problem on Android. :/ How do I downgrade to ionic 1.0.0 ?
I tested on android v5.0.2 ...i dont know if the problem is because of the version or because I use the pull to refresh function.. any ideas?
@mhartington when is 1.0.2 scheduled for release? I don't understand why this is not fixed sooner, I already released a few apps with version 1.0.0 because 1.0.1 has this huge bug. I have the same problem in every new app I build or update...
@JoeyHoutenbos clone the repo --> switch to branch 1.0.2 and use the gulp "bundle" task to create an own release
or run "gulp build --release" to mini and uglify
I've built a custom release.. if someone needs: https://github.com/KillerCodeMonkey/ionic/releases/tag/1.0.2_custom
Looking into this.
@JoeyHoutenbos Are you seeing the same issue as @lucasalmeida92 or are you just asking for a 1.0.2 release?
@lucasalmeida92 and @huip I'm not seeing any issues with pull to refresh
@KillerCodeMonkey I know I can do this and will use this. But I think the problem is too big to "just" merge it in the next milestone when we're not sure when this will be released. I want to use native scrolling as this improves the performance a lot!
Implementing a workaround by using native scrolling only for Android or implementing a fix as created by jskrzypek doesn't sound like a good solution. Sure I can make my own build of the 1.0.2 branch, but there might be other problems with this build since it's not released yet.. For that reason I'm using version 1.0.0 in some apps instead of the newer 1.0.1. So @mhartington I am asking (or just interested when the release is scheduled) for a 1.0.2 release to fix the scrolling issues on iOS. I don't have the same problem as lucasalmeida92, for me it's just iOS (in every app so far).
Alright, I thought there was a big issue with the PR in question.
@mhartington i dont know whats is happeing yet but when i have more details ill let u guys know! Thank u for your support ! ;)
For now ill keep using the 1.0.0 for production.
Sounds good. @JoeyHoutenbos this should be in master now. Feel free to pull from that
This fix allowed me to scroll on iOS8, however there are issues with the scroll momentum. The next touch after a fling is registered as a click and not a 'stop scrolling' event, and the touch areas have not updated meaning that it clicks whatever was in that position when the momentum kicked in.
Do you have an example showing this @mrtomrichy
@mhartington I have made a test project which implemented the fix from above and it suffers from the same issue. You can clone the project from here:
https://github.com/mrtomrichy/Ionic-iOS8-Scroll-Issue
I have also recorded a video of the problem in the simulator, and I have also recreated it on device.
@mhartington Don't suppose you've had a chance to look into this? After some more playing around I've noticed that scroll callbacks are not fired on inertial scrolling, which I suspect is part of the issue.
Cheers
Will look into this today, could you open a new issue for this @mrtomrichy
This fix is in conflict with a behavior observed on Android 4.4 devices and caused a regression. The issue related is #3695. The ===
and ||
of the code section actually were intentional.
The scrolling issue only occured on iOS right? Maybe there is a need for a platform specific fix.
@MartinMa What's the logic behind that? If the ===
and ||
of the code section were intentional as you say then isn't this comment describing the opposite of that behavior?
// Prevent gestures that are not intended for this event handler from firing subsequent times
Prior to commit https://github.com/driftyco/ionic/commit/e10b5d23c9cb3e04c6374574303e55aef6aefee4 this would seem to indicate that either the comment was incorrect or the code was.
Also the bug was documented on android 4.4 as well in issue #4022
Did 1.2.1 resolve any of these issues? PTR with native scrolling received a number of related fixes
I cannot reproduce this. Can someone provide a simple example case that shows the issue in Ionic today? Thanks.
Closing as this should be fixed.
I have a situation with scrolling of list
in the controller i do ( also tried with or without Platform.isIOS Check... did not work
set the code for controller and the template... Please let me know where am I doing it wrong
`$scope.contentScrolling = function(){ if (ionic.Platform.isIOS()) { $ionicScrollDelegate.$getByHandle('listscroll').resize(); };
};
<ion-list class="item-previews">
<ion-item href="#/item-details-phones/{{item.ID}}" class="item-icon-right item-preview" collection-repeat="item in itemsArray | orderBy: 'Name'">
<div class="item-thumbnail" style="background-image: url( '{{item.ThumbnailScr}}' );"></div>
<ion-option-button class="button-positive" ng-click="showPoints(item)">points</ion-option-button>
<ion-option-button class="button-balanced" ng-click="showMap(item)">map</ion-option-button>
<div class="item-data">
<span class="item-name" ng-bind-html="item.Name"></span>
<div class="sub-title" class="item-location">
<span ng-bind-html="item.Location"></span>
<span class="item-designation">{{item.itemType}} item</span>
<span class="item-presidential" ng-class="{'show-label' : item.Presidential}">Presidential Suite</span>
</div>
<i class="icon ion-chevron-right icon-accessory"></i>
</div>
</ion-item>
</ion-list>
</ion-scroll>`
@mlynch I am on 1.2.4 This still exists. Not able to scroll unless I set jsScrolling to true
Type: bug
Platform: ios 8 webview
@perrygovier (and the rest of the team but the commit below is Perry's)
The current nightly build breaks native scrolling in
ion-content
s when they don't have anion-list
or anion-scroll
wrapping the contents. The view is not scrollable.To reproduce build a project with the code
in this codepen: http://codepen.io/jskrzypek/pen/oXpPmGin this repo: https://github.com/jskrzypek/4008-preventDefault and then follow the instructions on the readme (or below).I think this commit introduced the bug: https://github.com/driftyco/ionic/commit/56ab0f2b5d7f5bd35ed6252749cfa1867c3b0f6a, it's on line https://github.com/driftyco/ionic/blob/master/js/utils/gestures.js#L1174.
It seems this is more likely what was meant (and fixes the bug):