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

Scroll, user has to tap twice to fire tap event. #2065

Closed
hourliert opened this issue Jul 10, 2015 · 19 comments
Closed

Scroll, user has to tap twice to fire tap event. #2065

hourliert opened this issue Jul 10, 2015 · 19 comments
Assignees
Labels

Comments

@hourliert
Copy link

Hi,

I am developing a web app using polymer. Everything works great on desktop and almost everything works great and mobile expect one view. This view is accessible here.

On this specific view, the user could scroll the page and tap on a card to display a modal view. The problem is (on mobile):

  • if user doesn't scroll and directly taps on a card. It works perfectly.
  • if user scrolls and taps on a card, nothing happen but if the user taps on the same card again, it works.

I can reproduce this bug on my OnePlus One, Nexus 9 and ios simulator.

I have tried to debug it but before I explain to you what "I found", here is a similar view structure that I use:

<paper-drawer-panel> <!-- this element listen on **track** event to be able to swipe the drawer -->

  <div drawer>...</div>
  <paper-scroll-header-panel main>

   <card-list>
     <template is="dom-repeat" items="[[cards]]">
       <card model="[[item]]"></card> <!-- this element listen on **tap** event and fire a new event -->
     </template>
   </card-list>

  </paper-scroll-header-panel main>

</paper-drawer-panel>

So after debugging, I observed:

  • when the user directly taps on a card, info.prevent is false and everything works great, the tap event is fired.
  • when the user scrolls, first the <card/> tap event should occurred. But as dy is too high, the event is not fired. The gesture is also reset. Second the <paper-drawer-panel/> track event should occurred and it does. So this line is executed. This will prevent the next tap event. I guess all of this is normal. But then. the user will really tap on the card and now info.prevent is true because of the previous Gestures.prevent('tap');. The tap event is not fired but the tap gesture is reset. The user has to tap again on the card to finally fire the tap event.

If I remove this line. Everything works perfectly (obviously except the gesture to open the drawer).

I hope I was clear enough. Does someone have an idea ?
Thanks in advance,

Thomas

@rubenstolk
Copy link
Contributor

Thank you so much, I've got the same issue!

@kevinpschaaf
Copy link
Member

Thanks for the issue. This issue tracker is for the core Polymer library, not for issues with elements created with it. We'll move this to an appropriate issue tracker.

@hourliert
Copy link
Author

Thanks for your answer. Indeed I use 2 paper-elements but this issue seems also related to the gesture implementation. Where do you think this issue should be reported?

@rubenstolk
Copy link
Contributor

I also feel this is more of a core related issue than a specific element issue.

@valpackett
Copy link

+1

"Everything works perfectly" – well not everything, seems like the gesture to open the drawer doesn't work without the track event.

@tjsavage
Copy link
Contributor

@azakus Do you think this is a gestures issue or specific to an element?

@dfreedm
Copy link
Member

dfreedm commented Jul 17, 2015

This seems like a general gestures issue. Looks like a regression from #1590

@dfreedm dfreedm added p1 and removed needs move labels Jul 17, 2015
@valpackett
Copy link

Looks like the correct fix this time is this.info.prevent = false (instead of POINTERSTATE.tapPrevented)

@m4b
Copy link

m4b commented Jul 19, 2015

The issue is definitely in the track event code being handled incorrectly, similar issues coming up in <paper-drawer-panel>

@valpackett
Copy link

@m4b this isn't "similar" – this is the exact same issue! The example here features paper-drawer-panel too.

The fix is just like this one but POINTERSTATE.tapPrevented is now called this.info.prevent.

@rubenstolk
Copy link
Contributor

So anybody preparing a PR here?

@m4b
Copy link

m4b commented Jul 19, 2015

@myfreeweb I saw that you won't sign the CLA so I mirrored your commit. Not sure how legal that is, heh, so we'll have to see. Also, I assume you're ok with that?

Anyway testing on firefox seems to resolve the issue, similarly with android, nice find!

@valpackett
Copy link

Of course. This fix is SO small I didn't even make a PR before @rubenstolk asked.

@m4b
Copy link

m4b commented Jul 19, 2015

Cool. A small fix, but a huge impact 👍 :) Mobile was essentially unusable imho before this fix; as I mentioned in the issue I raised, when scrolling through lists of items, it would cause next touch loss, but from the users perspective it just seemed like the app was broken and crappy.

I might finally be able to unleash my app on the world now ;)

@hourliert
Copy link
Author

Thanks for the fix. Waiting on the next polymer release and ready to publish my app too.
Thanks again.

@rubenstolk
Copy link
Contributor

For the ones who are really keen on launching their app before this PR gets merged and don't want to change the polymer code themselves:

  // Patch for https://github.com/Polymer/polymer/issues/2065
  window.addEventListener('WebComponentsReadyx', function () {
    var recognizer = window.Polymer.Gestures.recognizers.filter(function (r) {
      return r.name === 'tap';
    })[0];
    recognizer.mousedown = function (e) {
      this.info.prevent = false;
      this.save(e);
    };
    recognizer.touchstart = function (e) {
      this.info.prevent = false;
      this.save(e.changedTouches[0]);
    };
  });

@m4b
Copy link

m4b commented Jul 20, 2015

@rubenstolk: nice! As soon as iron-list/issues/21 is done (don't want to reference), which I'm thinking will be this week, things are looking quite nice!

@rubenstolk
Copy link
Contributor

Definitely, thanks so much @m4b @myfreeweb, and just in time before I have to release my app...

@m4b
Copy link

m4b commented Jul 20, 2015

Thank @myfreeweb I didn't do anything 😛

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants