-
Notifications
You must be signed in to change notification settings - Fork 54
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
attempt to fix issue #46 #47
Conversation
* This will able us to correctly track its use
* When unsyncing 2 maps in a set of 3, cursor disappear on map not unsynced. * This fixes this by removing the cursor from the list of _cursors to follow (using the previous commit) and not remove the cursor on the synced map (because it might used by another map) and its callback
Thanks for the fix, travis ci reports a style issue, I'll merge next week if you fixed that |
I have also added a line to hide cursor when unsyncing to mimick previous behavior. This is questionnable. Tell me what you think about it. |
@solsticedhiver due to another merge into master, now there are conflicts on this PR (I don't know if you get an email automatically). See that some events were organized in that other merge. |
@jjimenezshaw Ok. I have merged the conflict. |
if (this._cursors) { | ||
this._cursors.forEach(function (cursor, indx, _cursors) { | ||
if (cursor === map.cursor) { | ||
_cursors.splice(indx, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing a semicolon here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jieter you are right... but eslint has not detected it (tests are ok). Shouldn't be eslint configuration more strict? (sure, this is for another PR, not now)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I agree. I also noticed some other oddities in unchanged lines, we should have a look at that. I can fix this line then too, so merging ;)
@solsticedhiver merged, thanks! |
released leaflet.sync@0.2.1 |
my attempt to fix an issue with synCursor that breaks with more than 2 maps and unsynced.