Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

prevents unwanted opening of the soft keyboard #2989

Closed
wants to merge 1 commit into from

Conversation

JanLaussmann
Copy link
Contributor

Ghost clicks are busted but the corresponding form elements are still focused. This means that for example on smartphones the soft keyboard will be opened. This pull request prevents the unwanted opening of the soft keyboard.

Ghost clicks are busted but the corresponding form elements are still focused. This means that for example on smartphones the soft keyboard will be opened. This pull request prevents the unwanted opening of the soft keyboard.
@petebacondarwin
Copy link
Member

PR Checklist (Minor Feature)

  • Contributor signed CLA now or in the past (if you just signed, leave a comment here with your real name for cross reference)
  • Feature improves existing core functionality
  • API is compatible with existing Angular apis and relevant standards (if applicable)
  • PR doesn't contain a breaking change
  • PR contains unit tests
  • PR contains e2e tests (if suitable)
  • PR contains documentation update
  • PR passes all tests on Travis (sanity)
  • PR passes all tests on ci.angularjs.org (cross-browser compatibility)
  • PR is rebased against recent master
  • PR is squashed into one commit per logical change
  • PR's commit messages are descriptive and allows us to autogenerate release notes (required commit message format)
  • All changes requested in review have been implemented

@petebacondarwin
Copy link
Member

@JanLaussmann - thanks for this PR. It will need a bit more before we can merge it. Can you first sign the CLA, then take a look at our commit message guidelines. Also we will need a unit test to verify this change.

@JanLaussmann
Copy link
Contributor Author

@petebacondarwin thanks for you reply. I just signed the Contributor License Agreement (CLA). For cross reference my real name is "Jan Laußmann". I will provide a correct commit message and a unit test in the following days.

@petebacondarwin
Copy link
Member

@JanLaussmann - Hi, did you get time to fix up the commits and provide tests? Thanks.

@JanLaussmann
Copy link
Contributor Author

@petebacondarwin I don't know how a test for this pull request could be written because the wrong behavior is very specific to real touch devices and I don't know how I could emulate the problem in a test without a real hardware touch device.

But there are several other people with the same problem who would benefit from this pull request, see the discussion about "ghost clicks" here https://groups.google.com/d/msg/angular/NpgXfFh7nA0/ZpyYQIiQ_I0J

Also this pull request fixes issue #2810

Maby the ngMobile author @shepheb could provide some help?

@mhevery
Copy link
Contributor

mhevery commented Jul 31, 2013

MERGED

@mhevery mhevery closed this Jul 31, 2013
@JanLaussmann JanLaussmann deleted the patch-1 branch August 1, 2013 06:08
@damrbaby
Copy link
Contributor

damrbaby commented Aug 9, 2013

This PR makes ngMobile unusable for me. The problem is that if the text input or textarea is within an element that has ng-click attached to it, it will never get focused.

I'm currently testing on iPad with iOS 6.0. All of the forms in my app are no longer getting focus because they are within an ng-click area. I think we need to use a different approach.

For what it's worth I have found that using fastclick instead of ngMobile altogether solves the ghostclick problem I reported in #2810. I mention there that they addressed the issue in this PR: ftlabs/fastclick#27

@damrbaby
Copy link
Contributor

damrbaby commented Aug 9, 2013

I would like to argue that angular should remove the ng-click override from ngMobile altogether for the following reasons:

  1. ngMobile never attempted to remove the 300ms tap delay when tapping on form inputs. Fastclick handles this out of the box. It would be difficult for ngMobile to handle this since its code is tied to the ng-click directive. Meanwhile it doesn't make sense why angular would want to remove the delay on things like buttons and links that happen to have ng-click attached to them but not on form inputs.
  2. Any time you write a custom directive, you have to make sure to use ng-click in the directive template, otherwise the 300ms delay still applies (e.g. if you do element.click(function() {} ) in the directive, there will still be a delay there). That is pretty ridiculous.
  3. As soon as you include ngMobile in your app, you get the ng-click override. What if you just want to use the ng-swipe directive, for example, without the ng-click override?
  4. It just doesn't work smoothly. Fastclick is much better, it's framework agnostic, just focuses on removing the click delay (nothing else), and it's tried and tested by the mobile community.

So I am wondering why angular doesn't just adopt or just recommend the use of fastclick?

@matsko @IgorMinar @shepheb I'm cc-ing you on this because I think this is a serious problem. As a mobile developer who uses angular to write mobile apps, I am unable to use ngMobile at all.

damrbaby referenced this pull request Aug 9, 2013
Ghost clicks are busted but the corresponding form elements are still focused. This means that for example on smartphones the soft keyboard will be opened. This pull request prevents the unwanted opening of the soft keyboard.
@bshepherdson
Copy link
Contributor

@damrbaby My early experiments with combining fastclick with Angular were a failure. That was why I started out writing the custom ngClick ghost click buster and so on. Since others have since used it with success, I suppose whatever incompatibility there was has since been resolved, or it was my own pilot error.

I'm prepared to strip the ngClick override from ngMobile and recommend the use of fastclick instead, if it is working out of the box. It has several bugs and is not as robust as it needs to be. If Fastclick already solves those issues and doesn't get into trouble running alongside Angular, sounds good to me.

@damrbaby
Copy link
Contributor

@shepheb any update on this? I will go ahead and submit a PR that removes the ngClick override if you'd lke. I'd hate to see angular 1.2 shipping with a broken ngTouch module, which will cause all sorts of issues for people developing mobile apps. I've had 100% success with Fastclick and that should either be the recommended approach, or perhaps the fastclick code should be bundled in angular-touch itself.

@revolunet
Copy link
Contributor

Anyone here having a FastClick+angular integration ?
I discovered a new bug in ngTouch/ngClick and wondering if i should investigate/fix it or switch implementation directly.

@cburgdorf
Copy link
Contributor

+1 on removing the ngClick override.

@tromex
Copy link

tromex commented Sep 19, 2013

Before using ngTouch I was using FastClick and it was working OK out of the box.
I was attaching fast click by using this line of code at the end of the body:

FastClick.attach(document.body);

+1 on removing the ngClick override...

@bfricka
Copy link

bfricka commented Sep 20, 2013

+1 Here as well. This is not an uncommon use case as you have things like a hamburger menu button with child links.

mikec added a commit to mikec/angular.js that referenced this pull request Feb 28, 2015
Form element focus should not be prevented by ng-click.
The issue was introduced by angular#2989, and happens on mobile browsers that support touch events.
Plunker to demonstrate the issue: http://embed.plnkr.co/CxeuWocoPE70vzrevrIi

Closes angular#4030, angular#6432
mikec added a commit to mikec/angular.js that referenced this pull request Jun 29, 2015
Form element focus should not be prevented by ng-click.
The issue was introduced by angular#2989, and happens on mobile browsers that support touch events.
Plunker to demonstrate the issue: http://embed.plnkr.co/CxeuWocoPE70vzrevrIi

Closes angular#4030, angular#6432
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants