swipe events are interfering with native horizontal scrolling

[ballenjr]

There needs to be a way temporarily disable the ng-swipe-* directives. They are breaking horizontal scrolling. I presume there is no other way around it. But I am always open to suggestions.

Use case: I could not find where the transition bindings were happening. But I cleaned them up anyway because the whole point here is to be able to reBind later. Plus if you don't they continue to consume events(the touch events that is).

    var s = angular.element('.app').scope();
    s.unBindSwipe();
    $(".app").unbind("touchcancel").unbind("touchstart").unbind("mousedown").unbind("mousemove").unbind("mouseup").unbind("oTransitionEnd").unbind("otransitionend").unbind("touchend").unbind("touchmove").unbind("transitionend").unbind("webkitTransitionEnd");
    var off = s.$on('$routeChangeStart', function() {
        s.reBindSwipe();
        off();
    });

Patches: https://github.com/mcasimir/mobile-angular-ui/compare/master...ballenjr:patch-1 https://github.com/mcasimir/mobile-angular-ui/compare/master...ballenjr:patch-2 // Probably need some work to work without jQuery

Reference: http://wilsonpage.co.uk/touch-events-in-chrome-android/

[ballenjr]

BTW, all of the code that I submitted is tested and works. Except for patch-2. That one is meant to negate the need for line 3 of my use case. Or at least most of it. Like I said I didn't find where the transition events were being added. I suspect that they are an implementation within the webkit engine due to the css transitions, just a theory.

[mcasimir]

@ballenjr Thank you for reporting this and pointing out something is wrong, in any case it should not happen and i would prefer to don't pollute scope at all here.

Can you please explain the issue in details cause it not conflicting (or at least so it seems) on demo with my device, so we may find another solution.

Also:

in patch-2:

for(var e in events.split(" ")) $element.unbind(e); 

Here you are iterating through [1, 2, 3, 4, ...] so attempting to unbind() the index of the array, that does not throw errors, but does also not unbind events.

Anyway the whole stuff is not necessary, events bound to $element variable are only startEvents, other events are bound to $movementTarget.

Note that all the events are unbound from $element in case it is by a common coincidence === $movementTarget.

         // ---> Those are the only events directly bound to $element: 
         $element.on(startEvents, onTouchStart);

          return function unbind() {
            if ($element) { // <- wont throw if accidentally called twice
              $element.off(startEvents, onTouchStart);
              if (cancelEvents) { $movementTarget.off(cancelEvents, onTouchCancel); }
              $movementTarget.off(moveEvents, onTouchMove);
              $movementTarget.off(endEvents, onTouchEnd);
            }
           // ...

[ballenjr]

Hello @mcasimir,

I'll go in order.

First, I am pretty sure that, in angular, all scopes possess their own context unless specifically stated otherwise by the coder. And all scopes also "inherit" from the parent scope, unless explicitly prevented. So it should not be possible to pollute the scope in this directive unless you place the irrelevant primitive @ scope.$root, which I didn't. And at the same time, "technically" from the root down(in some respect) all parent scopes are inherently "polluted" with child scopes when defined. Plus the method that I am adding is not irrelevant to the directive(s) or the scope or the context. Am I wrong?

Second, I am noticing that the current uncompiled codebase looks different than what I pulled from the package repo. I don't remember if it was npm or bower. Though my posted version number was the same last I looked. I am only assuming that the issue is still present @ the current state. If not then great. Though I am not sure how it could not be since the important bits that actually consume the events still appear to be relatively the same. Perhaps there is an angular version discrepancy here. Or some other conflicting package that is changing the behavior of the on/off methods on my end.

Third, are you saying that you can do:

<div class="app" ng-swipe-right="Ui.turnOff("uiSidebarRight")" ng-swipe-left="Ui.turnOn("uiSidebarRight")">
    <div class="app-body">
      <div class="app-content">
          <ng-view>
            <div class="photolist set" style="width: 100%;height: 100px;white-space:nowrap;-webkit-transform:none;overflow-y:hidden;overflow-x:scroll;">
              <img ng-if="m.media=='image'" ng-src="{{m.path}}" style="margin: 10px 10px 10px 0;-webkit-transform:none;width:70px;height:70px;" ng-repeat="m in [{path:"images/image.png", ... }]" />
            </div>
          </ng-view>
      </div>
    </div>
</div>

and still be able to use the native horizontal scroll(without the issue described in my reference) in the latest Android Chrome, without applying my use case fix? I have not tried this with an isolated project, so again I am unsure if there is some other conflict going on. Would appreciate your input on that scenario. Including what angular and/or jQuery version you used. I did notice that my jQuery version is doing some weird logic in .off() and .on() but it seems to be related to the event tracking object and deciding whether or not to track them. Which is what made this issue so difficult to trace to begin with.

Fourth, I didn't use or test patch-2. So ya I see what you mean, but I assumed this would need to be redone anyway since I am not sure if unbind is even defined on $element(as you can see I am using jQuery to accomplish this for real). This was just to point out were it needed to happen. But I did also miss that some of the events were on $movementTarget. So thanks for clarifying that for me. It helps me better understand what is happening here.

Fifth, if you do find that the events are still being consumed, as I did, then you will see that the handlers were not unbound as they should have been, by inspecting the element in chrome under the Event Listeners tab next to the Styles tab. Just as I did. Thats how I found the extra events that I presume were added by the engine as a result of the transitions being applied and/or processed.

Fifth part 2, it doesn't really matter that we think that they are being unbound here. Because we don't have a way to call this method anyway. Because "whoever" never "polluted the scope"(not an admission or agreement) to store the unbind method. So there is no way for unbind to ever be called, that I can see. Not withstanding any destroy logic you may or may not have. Which I am guessing would not help here because destroy would imply that it would be impossible or very difficult to reestablish the swipe functionality afterward.

"dependencies": { "angular": "1.4.2", "angular-route": "1.4.1", "mobile-angular-ui": "1.2.0-beta.11", "font-awesome": "4.2.0", "angular-resource": "1.4.1", "angular-cookies": "1.4.1", "angular-mocks": "1.4.1", "angular-bootstrap": "0.13.0", "angular-jwt": "0.0.9", "angular-ui-router": "4b978ac340", "bootstrap": "3.3.5", "minimongo": "4a0b505424", "angular-swiper": "d201111865", "angular-touch": "1.4.2", "html2canvas": "0.4.1", "lodash": "3.10.0", "angular-reverse-geocode": "latest", "bootstrap-switch": "~3.3.2", "moment": "~2.10.6", "jquery": "2.1.4" }

"dependencies": { // Could not be packaged with require for some reason so I manually "polluted the global scope" "cropper": "0.10.1", "ng-cropper": "0.3.0", "ngCordova": "0.1.17-alpha" }

[ballenjr]

@mcasimir Thanks for your help with this.

[mcasimir]

Hi many thanks to u for your long analysis. I have to review that, anyway: yes it's possible to pollute the scope unless your directive explicitly define a new isolated one, but its not a problem if we do that in a concise and predictable way.

If you have to avoid conflicts it is way simpler to roll your directive that uses $swipe service for now. Applying your workaround is still feasible but I prefer to explore a different solution first, something that is more generic.

Will let you know something soon. Thank u again!

Il giorno lun 24 ago 2015 17:56 ballenjr [email protected] ha scritto:

Hello @mcasimir https://github.com/mcasimir,

I'll go in order.

First, I am pretty sure that, in angular, all scopes possess their own context unless specifically stated otherwise by the coder. And all scopes also "inherit" from the parent scope, unless explicitly prevented. So it should not be possible to pollute the scope in this directive unless you place the irrelevant primitive @ scope.$root, which I didn't. And at the same time, "technically" from the root down(in some respect) all parent scopes are inherently "polluted" with child scopes when defined. Am I wrong?

Second, I am noticing that the current uncompiled codebase looks different than what I pulled from the package repo. I don't remember if it was npm or bower. Though my posted version number was the same last I looked. I am only assuming that the issue is still present @ the current state. If not then great. Though I am not sure how it could not be since the important bits that actually consume the events still appear to be relatively the same. Perhaps there is an angular version discrepancy here. Or some other conflicting package that is changing the behavior of the on/off methods on my end.

Third, are you saying that you can do:

and still be able to use the native horizontal scroll in the latest Android Chrome, without applying my use case fix? I have not tried this with an isolated project, so again I am unsure if there is some other conflict going on. Would appreciate your input on that scenario. Including what angular and/or jQuery version you used. I did notice that my jQuery version is doing some weird logic in .off() and .on() but it seems to be related to the event tracking object and deciding whether or not to track them. Which is what made this issue so difficult to trace to begin with.

Fourth, I didn't use or test patch-2. So ya I see what you mean, but I assumed this would need to be redone anyway since I am not sure if unbind is even defined on $element(as you can see I am using jQuery to accomplish this for real). This was just to point out were it needed to happen. But I did also miss that some of the events were on $movementTarget. So thanks for clarifying that for me. It helps me better understand what is happening here.

Fifth, if you do find that the events are still being consumed, as I did, then you will see that the handlers were not unbound as they should have been, by inspecting the element in chrome under the Event Listeners tab next to the Styles tab. Just as I did. Thats how I found the extra events that I presume were added by the engine as a result of the transitions being applied and/or processed.

Fifth part 2, it doesn't really matter that we think that they are being unbound here. Because we don't have a way to call this method anyway. Because "whoever" never "polluted the scope"(not an admission or agreement) to store the unbind method. So there is no way for unbind to ever be called, that I can see. Not withstanding any destroy logic you may or may not have. Which I am guessing would not help here because destroy would imply that it would be impossible or very difficult to reestablish the swipe functionality afterward.

"dependencies": { "angular": "1.4.2", "angular-route": "1.4.1", "mobile-angular-ui": "1.2.0-beta.11", "font-awesome": "4.2.0", "angular-resource": "1.4.1", "angular-cookies": "1.4.1", "angular-mocks": "1.4.1", "angular-bootstrap": "0.13.0", "angular-jwt": "0.0.9", "angular-ui-router": "4b978ac340", "bootstrap": "3.3.5", "minimongo": "4a0b505424", "angular-swiper": "d201111865", "angular-touch": "1.4.2", "html2canvas": "0.4.1", "lodash": "3.10.0", "angular-reverse-geocode": "latest", "bootstrap-switch": "~3.3.2", "moment": "~2.10.6", "jquery": "2.1.4" }

"dependencies": { // Could not be packaged with require for some reason so I manually "polluted the global scope" "cropper": "0.10.1", "ng-cropper": "0.3.0", "ngCordova": "0.1.17-alpha" }

— Reply to this email directly or view it on GitHub https://github.com/mcasimir/mobile-angular-ui/issues/293#issuecomment-134260839 .

[ballenjr]

Absolutely, and that is what I did too.

[ballenjr]

I see that you are right about the scope not being isolated:

angular.element("body").scope().reBindSwipe
->  e.directive.u.reBindSwipe()
angular.element(".app").scope().reBindSwipe
->  e.directive.u.reBindSwipe()

I agree that it's location should be predictable when it is isolated. It does need to be easily accessible. I am not sure where it would be attached to if it is isolated. I would hope that it goes here angular.element(".app").scope() but now I am not sure. I hope you have an idea. For now I don't really care though because I named it uniquely enough that it won't hurt anything where it is.

[ballenjr]

What do you think about this? https://github.com/mcasimir/mobile-angular-ui/compare/master...ballenjr:patch-1

[ballenjr]

No that won't work either. Each new child scope would just clobber the last one, wouldn't it?

[ballenjr]

But if that is the case then why does my original workaround even work?

[ballenjr]

Maybe that is why all of the handlers were not removed by calling scope.unBindSwipe()...

[ballenjr]

Now I am a total loss. Not only does creating a new child scope not prevent the parent scope from being polluted(as I originally thought it wouldn't) but attempting to run unbind for both directives results in the element being nulled on the first pass and causes an error on the second pass. So clearly there was no distinction there to begin with. And I also found the section of the code that is supposed to be allowing native scroll to take over. But I don't understand how testing (totalY > totalX) would indicate anything other than vertical scroll in the most extreme case. So I guess that does not surprise me that it didn't work for horizontal.

[ballenjr]

Nevermind, that worked perfectly. https://github.com/mcasimir/mobile-angular-ui/compare/master...ballenjr:patch-1

[ballenjr]

I just forgot to pass true to $new

[ballenjr]

And a working version of patch-2 is no longer needed because it now unbinds for all relevant directives.

[ballenjr]

But, unfortunately because the directives were created the way that they were the rebind needs to happen in a similar fashion.

s.$swipe.rebind("ngSwipeLeft");
s.$swipe.rebind("ngSwipeRight");

or

s.$swipe.rebindBoth();

Or you will get an error.

[ballenjr]

I considered trying to key the element. But I think that even if the directive is on multiple elements then either you wouldn't want to unbind/rebind or you would want to unbind/rebind all of them at once. Not real sure how to accomplish the later unless I keep a map of directiveName -> element. Seems a little inefficient though.

[ballenjr]

I am out of use cases that I can test so this will be my last commit. https://github.com/mcasimir/mobile-angular-ui/compare/master...ballenjr:patch-1

[ballenjr]

I went ahead and tested the one element use case and it worked fine. I assume the rest of the element keys should work as expected. If its not too inefficient for ya.

[ballenjr]

So as a thought exercise(since you seem to be the sole contributor on this repo), do you think there would be a way to have your cake and eat it too in this case? Meaning keep the swipe functionality and still be able to, for instance, register an element for native side scrolling precedence.

One other thing that I was wondering is were there any plans to create another version of this swipe directive that does not wait for touchend before starting the slide animation?

Most apps today respond to the pull of the finger pretty much right away and follow the finger until it lets go then if you are past a certain threshold it completes the rest of the animation otherwise it reverts back to the original state.

[mcasimir]

@ballenjr Sure, that would be a big improvement! I gave a very quick thought, but what about this:

<my-elem
        ui-swipe-left='expr'
        ui-swipe-right='expr'
        ui-swipe-valid='myCustomValidityFn($touch)'></my-elem>

[ballenjr]

I dunno. Doesn't that leave it open for some one to take to long to pass back to native? I was thinking something more like:

<swipeelement ng-swipe-*>
<nestedlist side-scroller>
<listitem ng-repeat>
</nestedlist>
</swipeelement>

Then when the first touch event hits the side-scroller it sets a flag that tells ng-swipe-* not to bother doing anything cuz native scroll needs to take over.

But if you were talking about the swipe improvement. I don't think the improvement would be the check that it is valid. I think the improvement would be that it assumes its valid after the same criteria that you already have. But it doesn't wait for the touch end. It just starts moving the element as soon as 30px have gone by. And the "element" track your finger until you let go then it can handle the rest of the animation based your fingers position in relation to the set threshold.

It would require forced gpu acceleration. But I don't think that should be a problem. And there could be a warning on it saying not for devices/browsers without gpu capabilities please use the other one to support other devices more reliably.

Come to think of it I don't remember seeing if there is a fallback for that in the CSS?

And I was also going to mention that the slide-to position should be a percentage and not a pixel value for responsiveness. I already made that override in mine. On Aug 25, 2015 9:56 AM, "Maurizio Casimirri" [email protected] wrote:

@ballenjr https://github.com/ballenjr Sure, that would be a big improvement! I gave a very quick thought, but what about this:

— Reply to this email directly or view it on GitHub https://github.com/mcasimir/mobile-angular-ui/issues/293#issuecomment-134669511 .

[ballenjr]

Or(back to the native scroller), maybe its just not possible because the engine is going to wait 200ms no matter what as long as you have handlers attached and don't call preventDefault().

[ballenjr]

Maybe it would have to reimplement the scroll feature to override the native behavior. I wonder if that could work cleanly and reliably?

[mcasimir]

Yep but that would just sucks.

[ballenjr]

No kidding.

[mcasimir]

@ballenjr please join https://gitter.im/mcasimir/mobile-angular-ui?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge

[ballenjr]

Whats that?

[mcasimir]

the chat for mobile-angular-ui :)

[ballenjr]

oh