-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Draggable: Whitespace and naming cleanup of connectToSortable #1323
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
Conversation
item: inst.element | ||
}); | ||
item: draggable.element | ||
}); |
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.
It could just be the diff, but this looks like it's indented too much.
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.
Github now turns tabs into 8 spaces. This is indented one extra time from the var. incorrect?
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.
With a single variable, we don't do the extra level of indentation, which I realize is strange.
var foo = {
a: "b"
};
var foo = {
a: "b",
},
bar = {};
I don't think we've ever been clear about this in the style guide.
Since a sortable grows or shrinks when a draggable element is added to it, refresh the cached positions of sortables whenever an element is added or removed from the sortable. Refs #9675
faf0682
to
46f1af3
Compare
46f1af3
to
bc5cd92
Compare
Refs #9481 Refs #9675 Closes jquerygh-1323
bc5cd92
to
bfb6507
Compare
inst.cancelHelperRemoval = true; //Don't remove the helper in the draggable instance | ||
this.instance.cancelHelperRemoval = false; //Remove it in the sortable instance (so sortable plugins like revert still work) | ||
if ( sortable.isOver ) { | ||
sortable.isOver = 0; |
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.
While you're cleaning this up would it be possible to change isOver
to be a boolean rather than a 0
or 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.
I'll do that in a followup. Good catch.
Ah I missed that you merged this. Nevertheless this looks good. |
Refs #9481 Refs #9675 Closes gh-1323
No description provided.