remove cursor on unsync map

jieter / Leaflet.Sync

Synchronized view of two maps.

http://jieter.github.io/Leaflet.Sync/examples/dual.html

BSD 3-Clause "New" or "Revised" License

235 stars 53 forks source link

remove cursor on unsync map #29

Closed RalucaNicola closed 8 years ago

RalucaNicola commented 8 years ago

This pull request contains code for:

on map unsync remove corresponding cursor
tests in examples (dual-cursor and multiple-cursor) - using window.setTimeout to unsync maps after some seconds

jieter commented 8 years ago

If you click on details next to travis-CI, You'll see the reason why the PR failed. Also note the space/tab inconsistencies you introduced in the examples.

By tests I meant unit tests: those are ran every time a change is made and help to keep it all working like we want without any need for a person checking it. It would be nice if this could be tested automatically too. Can you try to make one? Just

jieter commented 8 years ago

Hmm, I might have introduced a more difficult situation for you since I rebased the sync-cursor branch on master. This also contains extra checks for indentation in example files...

RalucaNicola commented 8 years ago

I saw that error: "missing space before opening brace", so I have to check my syntax then...I will have to read about this travis-CI tool...and about unit testing :) I will have a look at it in the evening. This is all new for me.

RalucaNicola commented 8 years ago

so tonight I was learning about these unit tests and how mocha and phantomjs work, however I still have a lot to learn. But I managed to set it all up so that I can test it locally, I deleted some space that eslint was complaining about and now I will try to focus on how to actually write tests. It's not really clear to me how to think about them:

for example I was thinking one test should be to see if the cursor that is in the map._cursor array is actually on the second map. I added this small test in test/spec.js.
I don't know how to check if the cursor on the second map is at the same position as the mouse on the first map.
at the unsync tests I should probably just check that the _cursor array is empty(? ...as unsync is testing only the case of unsyncing 2 maps)

jieter commented 8 years ago

Hmm, branch has conflicts, I think you need to rebase on the cursor-sync branch (or create a new branch off of cursor-sync and apply the changes manually).

Regarding the tests: I'd say start a new describe block for the cursor sync functionality. Checking for the cursors in map._cursors sounds like a good start. You could ask the cursor for its position (cursor.getLatLng()), but it's going to be [0, 0] unless you actually simulate mouse movements on the first map. We could add that to the tests using Prosthetic-hand, but that might be a bit complex for now.

RalucaNicola commented 8 years ago

@jieter: i can't manage to solve this :( I need some more git practice before I can do this properly. So I will just close the pull request for now and will try to figure out in the next weeks how it works. In case I have some good results I will get back to you.