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

fix(zepto): apply patch to handle read-only event #263

Merged
merged 1 commit into from
Oct 31, 2018

Conversation

Kocal
Copy link
Contributor

@Kocal Kocal commented Oct 31, 2018

Summary

This PR applies this patch from @ymoran00.
Some events are read only and zepto can not update data property.

I can't reproduce it at the moment because I am on smartphone, but this problem was really painful.

Also it seems that it doesn't break anything, this patch worked fine on our project at work (it uses place.js).

Result

autocomplete


By the way, should I remove build files from this pull request?
I wanted to ship them but they were desynchronized with actual source files.
The build is done before publishing on npm right?

Thanks.

@coveralls
Copy link

coveralls commented Oct 31, 2018

Coverage Status

Coverage remained the same at 88.924% when pulling 533373a on Kocal:fix/zepto-data-event into b6539d3 on algolia:master.

@Haroenv
Copy link
Contributor

Haroenv commented Oct 31, 2018

Yes, can you revert the dist files please? It’s only updated at publish. Thanks for this PR!!

@Kocal
Copy link
Contributor Author

Kocal commented Oct 31, 2018

Dist files have been removed, I also added a gif in the original post 👍

Copy link
Contributor

@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 looks correct to me, thanks for doing this PR!

@Haroenv Haroenv merged commit 917d5a7 into algolia:master Oct 31, 2018
@Kocal Kocal deleted the fix/zepto-data-event branch October 31, 2018 09:35
@Kocal
Copy link
Contributor Author

Kocal commented Oct 31, 2018

Thanks for merging.

Sorry for asking, but do you have an idea on when a new version will be published?
At the moment, I forked places.js and manually edited dist files to apply this patch 😞

@Haroenv
Copy link
Contributor

Haroenv commented Oct 31, 2018

I will release Autocomplete tomorrow

@Kocal
Copy link
Contributor Author

Kocal commented Oct 31, 2018

Okay, thanks you! 🙂

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.

3 participants