benmajor / jQuery-Touch-Events

A collection of mobile event plugins for jQuery.
MIT License
699 stars 225 forks source link

Not supporting jQuery 3.4.1 #159

Closed ppetree closed 4 years ago

ppetree commented 4 years ago

In referencing issue 150 where this plugin didn't work in 3.3.1, I can confirm that it does not work in 3.4.1 as well.

I posted comments on issue 150 and then realized @benmajor had closed the issue so, not being sure if he got new comments added to an already closed issue I'm opening this issue.

With jquery-3.4.1. I get no errors in the console but no touch/swipe events either. I can also confirm that adding jquery-migrate-3.1.0 does allow the touch/swipe events to work and removing migrate takes you back to square one with no touch/swipe events.

benmajor commented 4 years ago

Thanks @ppetree. So am I correct in assuming that adding jquery-migrate works, but there are no errors in the console ordinarily if not? That seems a little strange, but thanks for the heads-up. I'll investigate.

ppetree commented 4 years ago

Yes you are correct.

Scenario: I'm moving a project off jquery-mobile and on to Framework7. One of the big pieces to replace is the touch/swipe handler. After rooting around and playing with things like hammer.js I found yours via stackoverflow and it's virtually identical to JQM so it makes the port that much easier. Mostly it was plug and play except it didn't work. So, I rooted through the solved issues and saw the problem on 3.3.1 (issue 150) and how migrate had fixed that so I thought I'd give it a shot. Bang! Worked perfectly.

Just to verify, I modified your fiddle and removed 3.3.1 and added 3.4.1 and your plugin didn't work so I added the migrate plugin and it worked. I probably should have saved the modified fiddle under my name but really didn't think about it until now... and just to verify it again I ran your fiddle (note: this was your first fiddle, not the second one) again and the fiddle worked without migrate.

Update: I went to this fiddle which is the 2nd one you posted in issue 150. It works as posted. When I removed 3.3.1 and replaced it with this cdn version of 3.4.1 then it does not work.

ppetree commented 4 years ago

@benmajor The above post went through several revisions (trying to narrow down the problem for you). Whatever the code difference is between your plugin in the first fiddle and the second is probably the issue.

benmajor commented 4 years ago

Thanks @ppetree. It looks there's an issue with the plugin as is and 3.4.1. I'll do some digging to see what actually changed in 3.4.1 that might cause the plugin to stop working. It's odd that it works fine with jQuery Migrate, but doesn't throw an errors without it.

I will update here when I have looked into it.

benmajor commented 4 years ago

I've had a look into this, and it seems that it was just an issue with when jQuery was being loaded by jsFiddle (the error in the console was jQuery is not defined).

If you change the binding of the touch code to DOMReady, the library works as expected. Please see this fiddle, which uses the 2.0.1 version of the library, and jQuery 3.4.1.

ppetree commented 4 years ago

To revisit this... is there a way to tell the plugin to NOT do the swipe/touch/tap/whatever? Let's say I get the callback in my swipe handler and decide I don't want the swipe to occur, how can I tell your plugin to not do the swipe? (Returning false doesn't work.)

benmajor commented 4 years ago

You can just return anything in the handler, before handling the swipe in that case.

For example:

$('#element').on('tap', function() {

  // This could be any test you wish:
  if( ! $(this).children().length ) return false;

  // Now, handle it as usual.
});

Here's a Fiddle demonstrating how to prevent the callback in this fashion.

ppetree commented 4 years ago

Thanks Ben. That was kinda what I was doing. My code is now this:

  $("#settingsPage").on("swiperight", function (event) {
    el = document.elementFromPoint(event.handleObj.handler.arguments[1].startEvnt.offset.x, 
                                   event.handleObj.handler.arguments[1].startEvnt.offset.y);
    if( $(el).hasClass("no-swipe") || $(el).parents().hasClass("no-swipe") )
    {
      event.stopPropagation();
      return(false);
    }
    else
    {
      // execute swipe back
      app.views.main.router.back('/', {force: 'true', ignoreCache: true});
    }

I added the .parent().hasClass() and that seems to better but still not perfect.

benmajor commented 4 years ago

Okay, great. Just a heads-up, I think checking the length of the parents filtered by .no-swipe would be a faster test, i.e.:

if( $(el).hasClass("no-swipe") || $(el).parents('.no-swipe').length )