Skip to content
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

Resizable: alsoResize more than one element of a jQuery selection #1324

Closed

Conversation

benmosher
Copy link
Contributor

Fixes #4666.

Did not cause any (new) tests to fail. Several tests were failing on my machine when I pulled the project and set up per the readme (including two [unrelated?] resizable tests).

The selector syntax used in the implementations of _store and _alsoResize appear to be semantically identical to the removed code (sans the o.alsoResize[0]) and function as expected in my personal use.

@tjvantoll
Copy link
Member

Hey @benmosher,

Can you please sign our CLA so we can take a look at this? Thanks.

@mikesherov
Copy link
Member

@benmosher Thanks for contirbuting! Besides for signing our CLA, it would be great if this change came with some unit tests. I know the test suite for resizable is kind of anemic at the moment, but it would be very very helpful. Think you can do that?

Thanks again!

@benmosher
Copy link
Contributor Author

Signed the CLA. I will add unit tests if I get a chance 😎

@benmosher
Copy link
Contributor Author

Heads up: added unit tests a while ago. I'm not sure if that generates any alerts or if anything else is needed here.

@@ -1040,14 +1028,7 @@ $.ui.plugin.add("resizable", "alsoResize", {
el.css(style);
});
};

if (typeof(o.alsoResize) === "object" && !o.alsoResize.nodeType) {
$.each(o.alsoResize, function(exp, c) {
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this was supporting an undocumented format of the option where the keys were selectors and the values were contexts to search in. I'm ok dropping this since I don't think this was ever documented, but we should be aware of it.

scottgonzalez pushed a commit to scottgonzalez/jquery-ui that referenced this pull request Mar 3, 2015
scottgonzalez added a commit to scottgonzalez/jquery-ui that referenced this pull request Mar 3, 2015
scottgonzalez pushed a commit that referenced this pull request Mar 10, 2015
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.

5 participants