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

Adds onMouseDown workaround to TypeaheadOption #235

Merged
merged 1 commit into from
Jun 19, 2017
Merged

Conversation

kuzzmi
Copy link
Contributor

@kuzzmi kuzzmi commented May 28, 2017

onClick is not fired when clicked on a TypeaheadOption. This adds a onMouseDown workaround with the same handler to handle clicks by default.

Adds tests to test onClick and onMouseDown events, onClick test is set as pending until resolved.

Since the whole master branch test suit fails as mentioned in #229, these tests couldn't be verified, but should be fine as soon as the main issue is resolved.

onClick is not fired when clicked on a TypeaheadOption.
This adds `onMouseDown` with the same handler to handle
clicks.
@kuzzmi
Copy link
Contributor Author

kuzzmi commented May 28, 2017

This was also mentioned in the #205

@fmoo
Copy link
Owner

fmoo commented Jun 19, 2017

This feels like a hack. Do we know why click events aren't propagating properly? is it because we're incorrectly stopping cancelling / stopping propagation elsewhere in the stack?

@kuzzmi
Copy link
Contributor Author

kuzzmi commented Jun 19, 2017

It is, and it's not at the same time. It's hard to tell why onClick doesn't work, as tests are broken, however this is the only way to make it working right now. The only reason why IMO it might make sense to have this workaround is that currently clicking doesn't work at all and people fork this repo exactly for applying this "fix".

I'd be happy to fix the root cause, however I couldn't identify it, and it will take a while to fix the tests, so we can isolate this root cause.

@fmoo
Copy link
Owner

fmoo commented Jun 19, 2017

Cool, I'll merge this, but can you open a followup Issue to "Remove the onMouseDown hack from #235" ?

@fmoo fmoo merged commit e683187 into fmoo:master Jun 19, 2017
@kuzzmi
Copy link
Contributor Author

kuzzmi commented Jun 19, 2017

Added #237 issue to get rid of this hack.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants