oscarfonts / mapbox-gl-cordova-offline

Offline vector maps in Cordova using Mapbox GL JS
Other
60 stars 36 forks source link

Draggable Marker not moving #32

Closed eduboxgithub closed 5 years ago

eduboxgithub commented 5 years ago

When a add a marker and set the property "draggable=true" the marker does not move when I'm testing on the device (Android).

import * as MapboxGLOffline from 'mapbox-gl-cordova-offline';

let marker = new MapboxGLOffline.Marker( { draggable: true })
                     .setLngLat(new MapboxGLOffline.LngLat(+coords.lng, +coords.lat))
                     .addTo(map);

Do I need to do something special?

Is it because the mapbox version? It it possible to update it to the latest version 0.51?

eduboxgithub commented 5 years ago

Don't know if this is correct but reading the changelog https://github.com/mapbox/mapbox-gl-js/blob/master/CHANGELOG.md the ability to drag markers was only introduced in 0.46.0:

https://github.com/mapbox/mapbox-gl-js/pull/6687

How can we update the Mapbox version dependency?

oscarfonts commented 5 years ago

Have you tried to update the mapbox dependency and build the project? A PR if succeeded, or any feedback in case of problems will be kindly appreciated. :)

eduboxgithub commented 5 years ago

I was trying to make a few changes to update to 0.52, but I noticed there's already an PR with that change.

https://github.com/oscarfonts/mapbox-gl-cordova-offline/pull/36#issue-246059134

michogar commented 5 years ago

Hi @eduboxgithub

yes, for a project I needed to update the library with a more updated version of Mapbox. Mapbox has changed a method in ajax.jx and now it cans load files using window.fetch api when this api is available In our case, Cordova webview has this API and Mapbox tries to use it. But the problem is that this API doesn't allow load files using protocol file:// which is used to load sprites and glyphs in this library. To avoid this I made a workaround setting the window.fetch = undefined but this is a very ugly and heavy hack because maybe another library could use this api and this change could break these other libraries.

I have made a fork of this repo and I have updated the library to use 0.52 mapbox version. You can test it here, the version updated is the v0.2.1

https://github.com/northings/mapbox-gl-cordova-offline

The inclusion of the use of window.fetchis from mapbox 0.49.version, so, if you couldn't include this ugly hack in your app you shouldn't update to a higher version of Mapbox.

There is a branch updated until mapbox 0.49 version here:

https://github.com/northings/mapbox-gl-cordova-offline/tree/feature/mapbox.v0.49.0

I am still thinking which could be the best way to avoid the problem with the use of window.fetch, but I haven't found an elegant solution yet.

Cheers!!.

eduboxgithub commented 5 years ago

@michogar Hello, thank you so much for your feedback.

I was doing some extra testing with the forks you provided, and I noticed a new version of mapbox was release (v0.53). I was reading their changelog and I think they fixed the issue regarding the fetch/file:// problem.

https://github.com/mapbox/mapbox-gl-js/blob/master/CHANGELOG.md

Allow file:// protocol in XHR requests for Cordova/Ionic/etc (#7818)

https://github.com/mapbox/mapbox-gl-js/pull/7818

michogar commented 5 years ago

Cool!!, these are good news. I'll try to test the library with that version of Mapbox ASAP. If you advance in the integration of the latest Mapbox version, please, maybe could you make a PR?.

Thank you very much, and enjoy mapbox-gl-cordova-offline!!

michogar commented 5 years ago

Hi @eduboxgithub

I've just updated the library with the latest Mapbox GL JS version, 0.53.1. Please check if problems with markers disappear and we can closing the issue.

Best.