Skip to content

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

Merged
merged 8 commits into from
Aug 26, 2014

Conversation

mikesherov
Copy link
Member

No description provided.

item: inst.element
});
item: draggable.element
});
Copy link
Member

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.

Copy link
Member Author

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?

Copy link
Member

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
@mikesherov mikesherov force-pushed the connectToSortable2 branch 2 times, most recently from faf0682 to 46f1af3 Compare August 26, 2014 13:48
@mikesherov mikesherov merged commit bfb6507 into jquery:master Aug 26, 2014
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;
Copy link
Member

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?

Copy link
Member Author

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.

@tjvantoll
Copy link
Member

Ah I missed that you merged this. Nevertheless this looks good.

mikesherov added a commit that referenced this pull request Oct 15, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants