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

Leaflet 1.1.0 #45

Merged
merged 10 commits into from
Jul 22, 2017
Merged

Leaflet 1.1.0 #45

merged 10 commits into from
Jul 22, 2017

Conversation

jjimenezshaw
Copy link
Contributor

@jjimenezshaw jjimenezshaw commented Jun 30, 2017

Migration to leaflet 1.1.0. :

Tip to see the different version in a map: The minus character in the zoom control Leaflet/Leaflet#5501

@@ -17,7 +17,7 @@
// with the top right corner of the target map.
// The values can be less than 0 or greater than 1. It will sync
// points out of the map.
L.Util.offsetHelper = function (ratioRef, ratioTgt) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just adding a function to L.Util does not work in my Ubuntu Chrome (I have not tested in other browsers). Any idea how to do it? The only option I see is to include it directly in L, not in L.Util, but it breaks backwards compatibility 😞

Copy link
Owner

@jieter jieter Jun 30, 2017

Choose a reason for hiding this comment

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

I think this is due to Leaflet's new rollup built system. I think moving it out of the utils again is the way to fix this.
Hmm, I'd say no to L.offsethelper, can't we do L.sync.offsetHelper, that would be better thanp putting it in the global L.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about this approach?

Copy link
Owner

Choose a reason for hiding this comment

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

Looks good.

@jieter
Copy link
Owner

jieter commented Jul 1, 2017

We also need to update the readme to state that this version and greater will support leaflet 1.1.0 and greater.
And maybe also a note in the current readme before we merge this.

Apart from that, it looks good.

@jjimenezshaw
Copy link
Contributor Author

Could you do the note in the current readme before we merge this? Thanks.

@jieter
Copy link
Owner

jieter commented Jul 2, 2017

Yes, I'll do that, probably tomorrow.

@jieter
Copy link
Owner

jieter commented Jul 2, 2017

Just added the comment to the readme.

@jjimenezshaw
Copy link
Contributor Author

Grrrr. I found a new bug that happens in Firefox (and IE) 😞 . Unfortunately I use mainly/only Chrome. It makes a strange shake when you drag the map and then zoom in (and sometimes out) with the wheel or zoom control (+/-).
It happens also in with Leaflet 1.0.3.
I will do a pull request for 1.0.3 with a workaround, before this one for 1.1.0. It is not perfect, but improves a lot this behaviour. Ok?

@jieter
Copy link
Owner

jieter commented Jul 3, 2017 via email

@jjimenezshaw
Copy link
Contributor Author

This is interesting: the changes I have done to fix this shake or flicker in Firefox when zooming after drag (with inertia), make it compatible between 1.0.3 and 1.1.0 (we do not need onclick anymore, but we trigger the setView on moveend after a drag.). Good news.
(this change implies keeping state between events, that is not very nice... but I don't see a nicer way)
I am still testing in detail in different browsers/devices, searching for more corner cases.

I understand that package.json should have "leaflet": "^1.0.3", but what about the examples? What should I say in README.md?

@jjimenezshaw jjimenezshaw changed the title [WIP] Leaflet 1.1.0 Leaflet 1.1.0 Jul 15, 2017
@jjimenezshaw
Copy link
Contributor Author

@jieter I think I have done all needed for the migration to 1.1.0, fixing the bug discovered in Firefox, and being compatible with both 1.0.3 and 1.1.0.

@jieter
Copy link
Owner

jieter commented Jul 15, 2017

Nice work! I'll merge and release next week when I'm back home

@jjimenezshaw
Copy link
Contributor Author

Thanks. (#47 will probably conflict with this PR.)

@jieter jieter merged commit ff15ac3 into jieter:master Jul 22, 2017
@jieter
Copy link
Owner

jieter commented Jul 22, 2017

@jjimenezshaw merged, thanks!

released 0.2.0

@jjimenezshaw jjimenezshaw deleted the leaflet-1.1.0 branch July 23, 2017 12:26
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.

None yet

2 participants