A potential solution to issues with Google Places Autocomplete

[mesge]

This is based on a stack overflow solution to a known issue (#316) where FastClick conflicts with .pac-container in the google places autocomplete dropdown. I've checked out SelectiveFastClick but wasn't too interested in the extra dependencies.

http://stackoverflow.com/a/17176350/3413476

The option to exclude a certain node through tweaking the needsClick method introduces a way for users to use the power of needsclick in config without the need for any other library. In the future maybe an array of classes to ignore would be great, but for now the fix for Autocomplete works when you exclude pac-item only.

[epegzz]

Excellent solution @mesge, works like a charm! :)

[kanaabe]

+1

[mzaslavsky]

From some quick testing of this feature it appears excluding 'pac-item' or even '^pac-' is incapable of suppressing the disappearing dropdown bug that occurs when you select an option in the autocomplete list (tested on Android 2.3.4) i.e.

FastClick.attach(document.body, {
    excludeNode: '^pac-',
});

The potentially non-performant code suggested elsewhere (e.g. http://stackoverflow.com/questions/9972080/cant-tap-on-item-in-google-autocomplete-list-on-mobile#):

$(document).on({
    'DOMNodeInserted': function() {
        $('.pac-item, .pac-item span', this).addClass('needsclick');
    }
}, '.pac-container');

does work in my testing, suggesting that targeting the classless span under pac-item for exclusion may also be necessary on some platforms. Sure enough adding an additional check to a target's parentNode does fix the issue but I don't know how performant such a check would be:

return ((/\bneedsclick\b/).test(target.className) || (new RegExp(this.excludeNode).test(target.className)) || (new RegExp(this.excludeNode).test(target.parentNode.className)));

[epegzz]

@mzaslavsky I did not test on Android 2, but on Android 5, iOS 8 and desktop browsers @mesge's solution works well.

The DOMNodeInserted hack has a far higher performance impact than adding another RegExp check.

[mzaslavsky]

Thanks for the reply @epegzz. Rather than a single extra regEx on a parentNode I was actually thinking of implementing an upward iteration through the DOM (ala http://www.sitepoint.com/finding-an-ancestor-node/) for more generalised purposes. I haven't checked the performance of a native JS DOM traversal but the performance of jQuery's more complex parents() and closest() functions are notoriously slow on older smartphones (with Android 2.x). Using an extremely simple page at http://jsperf.com/parents-method I experience well over 300ms for either method (500ms to 1s). 3 simply chained iterations of JS's native parentNode function alone took ~11ms on my old Android. I'm sure using the DOMNodeInserted hack and modifying the DOM can end up even worse but I'm thinking a full DOM traversal on every click could slow down the older phone browsers that arguably need a fastclick hack the most. I don't know how many other third-party applications beyond Google Autocomplete could benefit from such a hack where classes on clickable elements are undefined.

On my website, for example, I wasn't really concerned as much with newer Androids since I understand listeners aren't attached when one uses Chrome 32+ on Android with width=device-width (or desktop browsers for that matter).

[epegzz]

Oh, I see your point @mzaslavsky. Your solution performs well, but is really ugly since not generic :(
I am wondering in what browsers the original pull request actually works and in which it doesn't.
If it's only Android 2 native browsers where it fails then in my opinion that would be acceptable. I personally don't want to support Android 2 browsers becoming the new IE6

[mzaslavsky]

Using the pull request as is (targeting '^pac-'), for Android 2.3.4 at least I think I've discovered a suitable CSS alternative to parentNode checks when dealing with Google Places Autocomplete (and possibly other similar scenarios).

You simply add:
.pac-item span {
pointer-events: none;
}

such that clicking the span element passes through to the underlying .pac-item.

This strategy has already been used to deal with unresponsive sub-elements within labels (#60).

[epegzz]

Yeah, I like that solution, good one! :-)

[ulion]

@mzaslavsky the css plan seems not work in my env chrome simulate mobile.

[mzaslavsky]

@ulion Sorry, could you elaborate?

I'll clarify my steps.
(1) I made @mesge's suggested changes to fastclick.js.
(2) When attaching Fastclick to the page I exclude all classes starting with "pac-":

    FastClick.attach(document.body, {
        excludeNode: '^pac-',
    });

(3) For the span sitting within (i.e. above) a .pac-item element I've added pointer-events: none to the css:

    .pac-item span {  
        pointer-events: none;
    }

As long as Google Places' click events aren't updated to react to this span I suppose there shouldn't be any unwanted side-effects (unless there are already). Note that I only made this extra CSS change to also support Android 2.3.4.

Also, sorry if I misunderstood, but are you emulating one of the mobiles in Chrome Dev Tools? Unless I'm mistaken, it's pretty much useless in replicating historical iOS and Android quirks (unless the bug is instantiated by a particular user agent string or another unique property e.g. pixel density).

[ulion]

@mzaslavsky

I thought your css code can replace the step 1 and step 2, but which did
not. indeed the step 1 and 2 just resolved the problem.

2015-07-04 6:56 GMT+08:00 mzaslavsky notifications@github.com:

@ulion https://github.com/ulion Sorry, could you elaborate?

I'll clarify my steps.
(1) I made @mesge https://github.com/mesge's suggested changes to
fastclick.js.
(2) When attaching Fastclick to the page I exclude all classes starting
with "pac-":
FastClick.attach(document.body, {
excludeNode: '^pac-',
});
(3) For the span sitting within (i.e. above) a .pac-item element I've
added pointer-events: none to the css:
.pac-item span {

pointer-events: none;
}

As long as Google Places' click events aren't updated to react to this
span I suppose there shouldn't be any unwanted side-effects (unless there
are already). Note that I only made this extra CSS change to also support
Android 2.3.4.

Also, sorry if I misunderstood, but are you emulating one of the mobiles
in Chrome Dev Tools? Unless I'm mistaken, it's pretty much useless in
replicating historical iOS and Android quirks (unless the bug is
instantiated by a particular user agent string or another unique property
e.g. pixel density).

—
Reply to this email directly or view it on GitHub
#347 (comment).

Ulion

[albertojg]

@mzaslavsky @epegzz @mesge

Any plan for this PR to be merged?

[jgodi]

+1, been using the patched version with this change to make this work

[pswoodworth]

+1 would be a lot cleaner IMHO

[jeffijoe]

This fixed the issue for me.

[COil]

Hi, what is the status of this PR ?

[bkempner]

Works for me too... been a year... suuuure would be nice to merge this guy.

[rowanbeentje]

All - thanks for the discussion on this. The next version will definitely allow needsclick to be a class on any element parent (which I think also addresses this - interested if people are using the regex-matching for non-pac classes?). Will also dig into this particular incompatibility and see if something can be done with the events to improve it.

[COil]

👍

[barryvdh]

Thanks, this fixed my problem also!

[collegeman]

I tried getting this to work by cloning @mesge's project into mine. This was my implementation:

$(function() {
  new FastClick(document.body, 'pac-item');
});

Best I can tell, it didn't work because the node receiving the click event was actually a nested <span>, so using the className wasn't working.

I decided to revert back to using the latest version of FastClick (1.0.6), and this is the fix I settled on:

$(function() {
  var needsClick = FastClick.prototype.needsClick;
  FastClick.prototype.needsClick = function(target) { 
    if ( (target.className || '').indexOf('pac-item') > -1 ) {
      return true;
    } else if ( (target.parentNode.className || '').indexOf('pac-item') > -1) {
      return true;
    } else {
      return needsClick.apply(this, arguments);
    }
  };

  FastClick.attach(document.body);
});

Thanks @mesge for your work on this though. Wouldn't have figured it out without you.

[mesge]

Nice one @collegeman. I'll admit my solution is a bit of a hack.

For that reason I'd personally recommend that future people do not merge my PR. Something along the lines of @collegeman's temporary solution is better, and the desired functionality is coming with the next version (cheers @rowanbeentje!).

Good discussion everyone.

[bohdan-vorona]

@collegeman Thank you!