montagejs / montage

Montage is an elegant, open source HTML5 framework maintained by Montage Studio that rivals native SDKs, yet is easier to learn. It offers modular components, two-way data binding, and much more. Join us on irc.freenode.net#montage. Sign up for our beta to build Montage applications in the cloud.
http://montagestudio.com/montagejs
Other
1.5k stars 215 forks source link

Adds setters to MutableEvent's touch events #2001

Closed tejaede closed 5 years ago

tejaede commented 5 years ago

@marchant If I recall, we are agreed that this PR is safe and can be merged into master. If that is still the case, will you please merge?

tejaede commented 5 years ago

@marchant @johnnykahalawai

Regarding context, my understanding is that Leaflet sets event.touches and event.changedTouches and fails if those setters are absent. Jonathan may know more specifics.

johnnykahalawai commented 5 years ago

@marchant ,

Without the change, this code: https://github.com/Leaflet/Leaflet/blob/b2dd61d02fea3c5cb8c155737afe8a53ca25d400/src/dom/DomEvent.Pointer.js#L104

Will throw an error:

Cannot set property touches of # which has only a getter

marchant commented 5 years ago

Thanks for the context, makes sense!

marchant commented 5 years ago

I suppose there is nothing in Montage code that will use the setter? So perhaps we should remove the array check and splice apply altogether?

I do not see anything in the leaflet code that expects this. Right, good point, this is new, so no backward compatibility worries here in montage

tejaede commented 5 years ago

Makes sense to me. I removed the conditionals from the setters.