Skip to content

Droppable: update ddmanager when scope changes through setOption. Fixes #9287 #1124

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Droppable: update ddmanager when scope changes through setOption. Fixes #9287 #1124

wants to merge 1 commit into from

Conversation

NiGhTTraX
Copy link
Contributor

No description provided.

@mikesherov
Copy link
Member

@NiGhTTraX, thanks for contributing! This change looks really good.

Before I can land this, you'll need to do a couple of thing!

Thanks again,a nd let me know if you have any questions!

@NiGhTTraX
Copy link
Contributor Author

Signed, formatted and squashed :) Let me know if I missed anything.

mikesherov
mikesherov reviewed Oct 29, 2013
View reviewed changes
tests/unit/droppable/droppable_options.js
@@ -35,6 +35,48 @@ test("{ addClasses: false }", function() {
ok(!el.is(".ui-droppable"), "'ui-droppable' class not added");
el.droppable("destroy");
});

test("scope", function() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test( "scope

@mikesherov
Copy link
Member

Thanks again! I left some comments that need addressing. After this round of changes, I can land this!

@mikesherov
Copy link
Member

Also, update ticket reference to 9287

@NiGhTTraX
Droppable: update ddmanager when scope changes through setOption. Fix…
369f89a 
…ed #9287 - droppable: Scope option cannot be changed after initialization.
@NiGhTTraX
Copy link
Contributor Author

Updated with changes. The spacing problems you pointed were deliberate as I copied code from other tests and that's how I found it. Noted for future contributions though :)

@mikesherov
Copy link
Member

@NiGhTTraX, yeah, I apologize for the inconsistent state of the styles. General rule is if you're adding or modifying code, clean it up as you go along. I'll land this shortly.

@mikesherov
Copy link
Member

thanks! landed in ffab89e

@mikesherov mikesherov closed this Oct 29, 2013
@NiGhTTraX NiGhTTraX deleted the fix_scope branch October 30, 2013 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@NiGhTTraX @mikesherov