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

fix(autocomplete): tap outside options panel on iOS does not close panel #11625

Merged
merged 1 commit into from
Jun 28, 2019

Conversation

Splaktar
Copy link
Member

PR Checklist

Please check that your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added or this is not a bug fix / enhancement
  • Docs have been added, updated, or were not required

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Enhancement
[ ] Documentation content changes
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Clicking outside of the options dropdown panel for md-autocomplete does not dismiss the panel on iOS.

Issue Number:
Fixes #9581.

What is the new behavior?

Clicking outside of the options dropdown panel for md-autocomplete dismisses the panel on iOS. It is a similar approach to what the md-datepicker uses.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@googlebot googlebot added the cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ label Jan 31, 2019
@Splaktar Splaktar added browser: Safari os: iOS This issue is specific to iOS labels Jan 31, 2019
@Splaktar Splaktar added this to the 1.1.13 milestone Jan 31, 2019
@Splaktar Splaktar self-assigned this Jan 31, 2019
@Splaktar Splaktar added type: gestures ui: mobile P2: required Issues that must be fixed. pr: merge ready This PR is ready for a caretaker to review labels Jan 31, 2019
@Splaktar Splaktar assigned mmalerba and jelbourn and unassigned mmalerba Jan 31, 2019
@Splaktar Splaktar assigned josephperrott and unassigned jelbourn Feb 10, 2019
@Splaktar Splaktar assigned jelbourn and unassigned josephperrott Feb 12, 2019
@Splaktar Splaktar modified the milestones: 1.1.13, 1.1.14 Feb 13, 2019
@AdriVanHoudt
Copy link
Contributor

Is there any way you can land this in the next patch release, this completely breaks forms on iOS :/
If there is something I can do to speed this up let me know!

@AdriVanHoudt
Copy link
Contributor

I tested it and this fix doesn't seem to do it.
If instead of adding the click handler to the document I add it to the .md-scroll-mask, it seems to behave as expected

@Splaktar
Copy link
Member Author

It has to go through the presubmit process and so far that hasn't gone through without an issues. There hasn't been time yet to review what needs to change (if anything) to get it through presubmit, but we're not holding 1.1.13 for this as we need to get some regression fixes released ASAP.

@Splaktar
Copy link
Member Author

@AdriVanHoudt I took that approach initially with this fix, but it had some unintended, negative side effects.

Can you please provide reproduction steps and a CodePen demo for the case where this fix isn't working for you? Also please provide the device and OS version.

@AdriVanHoudt
Copy link
Contributor

I'll see if I can get a minimal case to work/break, I tested it directly in our application.
The device I tested on is an iPhone 6 with iOS 12.1.4

What is the easiest way to test this branch in a CodePen?

@Splaktar
Copy link
Member Author

Splaktar commented Feb 21, 2019

Here's the issue that I hit when trying to click the scroll-mask.

Uploading your angular-material.js to Google Drive and then getting a sharable link with view permission can make the file available for use in CodePen via a URL like https://drive.google.com/uc?export=download&id=1Scog7Et6gtP67X9zqsVrKTfVGd_fWX-j where the part after id= is the same as that of the link from the Drive UI (https://drive.google.com/open?id=1Scog7Et6gtP67X9zqsVrKTfVGd_fWX-j). You just need to use the uc?export=download& API to make it work with CodePen.

Here's a CodePen from the issue using a build from this PR.

@AdriVanHoudt
Copy link
Contributor

That's a nice way to host code :D

To clarify: I tested this with md-chips with the md-autocomplete as a custom input. I didn't think about mentioning this earlier sorry.
I found out that the demo on https://material.angularjs.org/latest/demo/chips for the md-chips with the custom md-autocomplete input has the same problem so I forked that and added this version to it.
https://codepen.io/anon/pen/pGXpYw
It still has some weird behaviour, not sure what is happening. Also Safari devtools + iPhone is not the greatest debugging exp

@Splaktar Splaktar assigned andrewseguin and unassigned jelbourn Feb 26, 2019
@Splaktar Splaktar added pr: merge ready This PR is ready for a caretaker to review and removed pr: presubmit-failures labels Jun 6, 2019
@Splaktar Splaktar assigned mmalerba and unassigned josephperrott Jun 13, 2019
@Splaktar Splaktar assigned jelbourn and andrewseguin and unassigned mmalerba and jelbourn Jun 20, 2019
@andrewseguin andrewseguin merged commit f81349a into master Jun 28, 2019
@Splaktar Splaktar deleted the wip/scrollMaskNoBlur branch June 28, 2019 22:02
@AdriVanHoudt
Copy link
Contributor

Thanks for this 🔥

@Splaktar
Copy link
Member Author

Splaktar commented Jul 2, 2019

It looks like there is an issue with this change. I'm going to be investigating it to see if we can solve it rather than reverting this PR.

@Splaktar
Copy link
Member Author

Splaktar commented Jul 2, 2019

I can reproduce the issue in https://material.angularjs.org/HEAD/demo/autocomplete on macOS and Chrome 75.

Here's the behavior, after opening an autocomplete options panel by clicking the input with the mouse, when the mouse first moves over the edge of the panel, the panel flickers (hides and then reappears).

Splaktar added a commit that referenced this pull request Jul 2, 2019
stop propagation of input's click events up to the document
- similar to what we already do with the clear button (X)
Closure/JSDoc typing improvements

Relates to #11625
@Splaktar
Copy link
Member Author

Splaktar commented Jul 2, 2019

PR #11757 posted to address the regression.

andrewseguin pushed a commit that referenced this pull request Jul 2, 2019
…ut (#11757)

stop propagation of input's click events up to the document
- similar to what we already do with the clear button (X)
Closure/JSDoc typing improvements

Relates to #11625
Splaktar added a commit that referenced this pull request Jul 3, 2019
Splaktar added a commit that referenced this pull request Jul 3, 2019
andrewseguin pushed a commit that referenced this pull request Jul 3, 2019
Splaktar added a commit that referenced this pull request Aug 24, 2019
- open options pop-up on `touchstart`
  - since a `click` is often not sent on touch devices
  - this usually happens when the start/end point are not the same
- use `touchend` on the document to close the options panel on iOS
  - iOS mostly does not send `click` events for taps on the backdrop
  - call `doBlur()`` since iOS doesn't blur in this case
- combine some jQuery event handler calls
- combine duplicate `onMouseup()` and `focusInputElement()` functions
- don't let touchstart or touchend events bubble out of the component
- focus the input for `mousedown` events
  - this covers an edge case on touch pads where a `click` isn't sent
- move `isIos` and `isAndroid` logic out of gestures into `$mdUtil`
- add and correct JSDoc

Fixes #11778. Relates to #11625. Relates to #11757. Relates to #11758.
Splaktar added a commit that referenced this pull request Aug 26, 2019
- open options pop-up on `touchstart`
  - since a `click` is often not sent on touch devices
  - this usually happens when the start/end point are not the same
- use `touchend` on the document to close the options panel on iOS
  - iOS mostly does not send `click` events for taps on the backdrop
  - call `doBlur()`` since iOS doesn't blur in this case
- combine some jQuery event handler calls
- combine duplicate `onMouseup()` and `focusInputElement()` functions
- don't let touchstart or touchend events bubble out of the component
- focus the input for `mousedown` events
  - this covers an edge case on touch pads where a `click` isn't sent
- move `isIos` and `isAndroid` logic out of gestures into `$mdUtil`
- add and correct JSDoc

Fixes #11778. Relates to #11625. Relates to #11757. Relates to #11758.
jelbourn pushed a commit that referenced this pull request Aug 26, 2019
…11782)

- open options pop-up on `touchstart`
  - since a `click` is often not sent on touch devices
  - this usually happens when the start/end point are not the same
- use `touchend` on the document to close the options panel on iOS
  - iOS mostly does not send `click` events for taps on the backdrop
  - call `doBlur()`` since iOS doesn't blur in this case
- combine some jQuery event handler calls
- combine duplicate `onMouseup()` and `focusInputElement()` functions
- don't let touchstart or touchend events bubble out of the component
- focus the input for `mousedown` events
  - this covers an edge case on touch pads where a `click` isn't sent
- move `isIos` and `isAndroid` logic out of gestures into `$mdUtil`
- add and correct JSDoc

Fixes #11778. Relates to #11625. Relates to #11757. Relates to #11758.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ os: iOS This issue is specific to iOS P2: required Issues that must be fixed. pr: merge ready This PR is ready for a caretaker to review type: gestures ui: mobile
Projects
None yet
Development

Successfully merging this pull request may close these issues.

autocomplete: suggestion box not closing on blur
7 participants