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

[AutoComplete] Fix ghost clicks on new requests #6907

Closed
wants to merge 1 commit into from

Conversation

amangeot
Copy link

Fixes ghost clicks happening when touch tapping AutoComplete's menu items on iOS touch devices.

The ghost clicks could lead to unexpected navigation with items underneath the AutoComplete's menu items

@muibot muibot added PR: Needs Review PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI and removed PR: Needs Review labels May 20, 2017
@mbrookes
Copy link
Member

@amangeot Your change is causing tests to fail. Could you take a look? Thanks!

@@ -266,6 +266,9 @@ class AutoComplete extends Component {
};

handleItemTouchTap = (event, child) => {
// Prevent ghost clicks
event.preventDefault();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about exposing a property so it can be implemented on userspace. I'm worried about the side effects here?

Copy link

@max-b max-b Jun 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm working on a similar pull request for #6938 and I'm wondering if you could be a little clearer what you mean here? Would you mean doing something like passing the event object to the exposed onNewRequest property?

@amangeot
Copy link
Author

amangeot commented Jun 7, 2017

Yes it sounds better adding a onItemTouchTap(event). Maybe a onItemTouchTap(event, index, request) ?

I'll try to push an update as soon as I can

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 9, 2017

I feel like the best API would be the one already exposed by the Menu onItemTouchTap. basically: onItemTouchTap(event, child). What do you think? I would rather keep things a simple as possible.

Also, I'm wondering if a previously added event.preventDefault haven't introduced the regression discussed in #6891 (comment)

@max-b
Copy link

max-b commented Jun 10, 2017

@oliviertassinari ah oh I think I see what you mean there. I'm not sure what kind of time @amangeot has, but I think I understand what you're saying and if I find a few minutes this weekend I might make a go of fixing this. Same for #6938

Would a good PR then also update the docs?

@jony89
Copy link
Contributor

jony89 commented Jun 10, 2017

I'll just say what I said in #6891, ghost click events are clicks. if you want to prevent those then you should verify that you are preventing a click event.

The above check-in is preventing both click and touch events. I tried to solve both issues in #6891

@oliviertassinari
Copy link
Member

I'm closing the pull request as has been inactive for some time. I believe that we do no longer have the issue on the next branch as we migrated away from react-tap-event-plugin.

@amangeot amangeot deleted the fix-autocomplete-ghosts branch September 30, 2017 15:45
@oliviertassinari oliviertassinari added the component: autocomplete This is the name of the generic UI component, not the React module! label Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: autocomplete This is the name of the generic UI component, not the React module! PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants