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

Fixing issue #1318 #1319

Closed
wants to merge 3 commits into from
Closed

Fixing issue #1318 #1319

wants to merge 3 commits into from

Conversation

ymoran00
Copy link

Some event object have readonly data property - I've added test for that.
#1318

@kontrollanten
Copy link

kontrollanten commented Jan 28, 2018

Thanks @ymoran00. This will solve #1297 as well. What's the status of this?

kontrollanten added a commit to kontrollanten/algolia-places-react that referenced this pull request Jan 28, 2018
Workaround while waiting for madrobby/zepto#1319
kontrollanten added a commit to kontrollanten/algolia-places-react that referenced this pull request Jan 28, 2018
Workaround while waiting for madrobby/zepto#1319
src/event.js Outdated
e.data = data
var dataPropDescriptor = Object.getOwnPropertyDescriptor(e, 'data')
if (!dataPropDescriptor || dataPropDescriptor.writable)
e.data = data

Choose a reason for hiding this comment

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

It seems that when using strict mode dataPropDescriptor will be undefined when e is InputEvent (even though data property exists). Seems like a bug to me. To workaround this I would propose to wrap this line around a try/catch and add a comment about this.

Copy link

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

This would fix #1256 too

@latel
Copy link

latel commented May 14, 2018

why this fixed is not merged?

@ymoran00
Copy link
Author

Do I need to do anything? Why isn't it merged already?

@Kocal
Copy link

Kocal commented Oct 16, 2018

Ping @mislav / @madrobby, thanks.

Kocal added a commit to Yproximite/algolia-places-with-fixed-zepto that referenced this pull request Oct 16, 2018
Kocal added a commit to Yproximite/algolia-places-with-fixed-zepto that referenced this pull request Oct 16, 2018
Kocal added a commit to Kocal/autocomplete.js that referenced this pull request Oct 31, 2018
Haroenv pushed a commit to algolia/autocomplete that referenced this pull request Oct 31, 2018
@madrobby madrobby closed this Jun 15, 2021
@madrobby madrobby deleted the branch madrobby:master June 15, 2021 22:42
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.

7 participants