-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fix for Dragging Problem #907 #909
Conversation
Please remove the |
package.json
Outdated
@@ -6,7 +6,7 @@ | |||
"devDependencies": { | |||
"git-rev-sync": "^1.8.0", | |||
"happen": "^0.3.1", | |||
"jake": "~0.5.16", |
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.
Why does jake version change?
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.
Honestly, I don't know. IMHO jake itself did the change. I will revert this back.
src/MarkerCluster.js
Outdated
storageArray = storageArray || []; | ||
|
||
for (var i = this._childClusters.length - 1; i >= 0; i--) { | ||
this._childClusters[i].getAllChildMarkers(storageArray); | ||
} | ||
|
||
for (var j = this._markers.length - 1; j >= 0; j--) { | ||
if (ignoreDraggedMarker && this._markers[j].__dragStart) |
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.
tabs for indentation
src/MarkerClusterGroup.js
Outdated
delete e.target.__dragStart; | ||
if (dragStart) { | ||
this._moveChild(e.target, dragStart, e.target._latlng); | ||
} |
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.
Tabs for indentation
@@ -26,14 +26,16 @@ export var MarkerCluster = L.MarkerCluster = L.Marker.extend({ | |||
}, | |||
|
|||
//Recursively retrieve all child markers of this cluster | |||
getAllChildMarkers: function (storageArray) { | |||
getAllChildMarkers: function (storageArray, ignoreDraggedMarker) { |
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.
This will need to be included in the documentation.
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, certainly. Should I write a proposal?
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.
I have added a sentence to README.md, also indentation is now fixed
ok, will remove dist changes. I only had them first in for the fiddle wo use it. |
Restore original jake version
missing comma added
Thanks, have tweaked the readme a bit. |
Is now everythink ok with the PR? |
Could we get a spec test, or if not possible then a manual test in Should be the last thing I ask for, thanks! |
Add manual test for 808 and 907
I tried to add a manual test, but as with all tests in examples/old-bugs I get these errors, which prevent the tests from working (and me from verifying the added test): |
I think for the manual tests you probably need to build the dist/ files (but not commit them) and point at those? |
Is now everything ok? Can I do something to get this PR accepted? |
Merged and 1.4.1 released. |
Fix for Dragging Problem #907