ftlabs / fastclick

Polyfill to remove click delays on browsers with touch UIs
MIT License
18.66k stars 3.23k forks source link

Add .needsclick support to all child elements #119

Closed peterostrander closed 11 years ago

peterostrander commented 11 years ago

Hi there,

I'm using the Google Maps API to embed a map on a site, but fastclick disables the ability to tap any of the buttons inside the embed that control Street View, satellite view, etc (I assume Google has their own implementation that's interfering). Of course this only effects touch devices.

It would be nice to have an added class, maybe .needclick-children, that allows for a non-synthetic click on all child elements of a div.

Thanks.

Prinzhorn commented 11 years ago

^this

It makes sense to apply this to all children in general.

Given this example

<div class="needsclick">foo<span>bar</span></div>

It will currently only work when clicking on foo, but not on bar, because the span is the target. I personally think fastclick should by default do something equal to this jQuery code if($(e.target).closest('.needsclick').length > 0), which checks if any of the parents (or the element itself) has the needsclick class.

Prinzhorn commented 11 years ago

Like this (end of FastClick.prototype.needsClick)

var reNeedsClick = /\bneedsclick\b/;

do {
    if(reNeedsClick.test(target.className)) {
        return true;
    }
} while(target = target.parentNode);

return false;
matthew-andrews commented 11 years ago

@wilsonpage - Would it be feasible to check for the needsclick during the capturing rather than the bubbling stage?

wilsonpage commented 11 years ago

@matthew-andrews I don't know if there is any advantage to checking at the Capture phase. Essentially we need to check each ancestor's className of the event target for the existence of the 'needclick'. I'm guessing currently we are only checking the event.target. I'm not sure how expesive this is to do.

Optionally we could check for a flag that developers could set on the event object at a lower level before reaching the callback at the fastclick callback layer.

myNoFastClickElement.addEventListener('touchstart', function(event) {
  event.noFastClick = true;
});

// The touchstart listener in the fastclick lib
document.addEventListener('click', function(event) {
  if (event.noFastClick) retun;
  // Existing FastClick logic...
});

Does this make sense with regard to this problem?

W.

boryaku commented 11 years ago

Hi just wondering if there is a work around for this problem? if I click on my markers I don't get the click event.

matthew-andrews commented 11 years ago

Hi @boryaku one work around would be to add an event listener to the container who's children you would like to excluded from Fastclick.

// var myContainerEl = document.getElementsByClassName('needsclick')[0];
myContainerEl.addEventListener('touchstart', function(event) {
  event.target.classList.add('needsclick');
});

The way this work is it manually adds the needsclick class to the child elements as touchstart events happen (but before they reach FastClick).

A bit of a hack but should work with the current implementation of FastClick.

Eg. jsFiddle - http://jsfiddle.net/RCYCQ/

boryaku commented 11 years ago

Thanks matthew! I took the approach of using Leaflet :P and it works great.

matthew-andrews commented 11 years ago

You're welcome. What's leaflet?

wilsonpage commented 11 years ago

@matthew-andrews it's a mapping lib

matthew-andrews commented 11 years ago

Oh of course. Thanks @wilsonpage. Closing this issue...

Prinzhorn commented 11 years ago

Closing this issue...

Why? (given my example in https://github.com/ftlabs/fastclick/issues/119#issuecomment-20866803)

wilsonpage commented 11 years ago

@Prinzhorn @matthew-andrews's solution is a short term work around that should address your issue too. If you listen for the touchstart event on <div> and add the needsclick class dynamically in the callback to event.target, it means even clicks on <span> will still register as 'needsclick' with FastClick.

boryaku commented 11 years ago

It's being closed because all you need is leaflet :P

matthew-andrews commented 11 years ago

Also this feels like a similar issue to one where bootstrap dropdowns (in version 2.2.2, I think) and FastClick didn't play nicely together without some hacks (see use case 2 on the readme page). Ultimately that issue was fixed in bootstrap's codebase, rather than FastClick.

We could do something similar and report a bug to the Google Maps team?

https://code.google.com/p/gmaps-api-issues/issues/list

matthew-andrews commented 11 years ago

@boryaku, @peterostrander and @Prinzhorn,

I looked into this some more this evening. I can't reproduce the reported behaviour at all and in fact using FastClick with the Google Maps sample "Simple Map" example makes it feel a whole lot nicer to use. Update - I can, see below.

Compare: http://mattandre.ws/fastclick With: http://mattandre.ws/nofastclick

Am I missing something?

(I did spot one slight bug where it seems to stop the options beneath [Map] and [Satellite] from appearing - to toggle Terrain/Labels - I've raised an issue for Google here: https://code.google.com/p/gmaps-api-issues/issues/detail?id=5677)

Update: It's much worse if you use a dropdown menu, see here: http://mattandre.ws/fastclick/advanced.html)

jcesarmobile commented 10 years ago

I had the same problem with fastclick and jquery mobile

I was using < a class="needsclick" data-role="button" href="javascript:functionThatSendsClickToInput();" >Open input

The problem is jquery mobile, as my link has data-role="button" it creates 2 spans inside the < a > Example: < a class="needsclick" data-role="button" href="javascript:functionThatSendsClickToInput();" >< span class="ui-btn-inner" >< span class="ui-btn-text" >Open input</ span ></ span >

So the tap was in the spans instead the < a > and it doesn't work as it should.

I finally figured it out and added the needsclick class to the children manually, but it might be useful to add the needsclick class to all the children or make them work as the parent

imjoeybrennan commented 10 years ago

You guys should check out https://github.com/alexgibson/tap.js. Its like Fast Click Lite but does NOT have map compatibility issues. It allows you to target any selector to attribute "fast" tap to it and its link/submit children. Cheers.

rstacruz commented 10 years ago

For jQuery-able pages, this is a preferrable way to do it (imho):

$(document).on('touchstart', '.needsclick.recursive', function(event) {
  event.target.classList.add('needsclick');
  $(document).one('touchend', function() { event.target.classList.remove('needsclick'); });
});

This makes it any element with the class recursive needsclick keep their click events, working down to its descendants.

EDIT: selective-fastclick is a superior solution, as stated by Matthew Andrews himself below.

matthew-andrews commented 10 years ago

Another alternative would be to choose which elements you want fastclicked with Selective FastClick.

MiguelMadero commented 9 years ago

@rstacruz @matthew-andrews thanks the on('touchstart') did the trick.