Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

fix(select): Fix several issues when moving options between groups #10166

Closed
wants to merge 1 commit into from

Conversation

lgalfaso
Copy link
Contributor

  • When an option was moved to a previous group, the group that loose the option would remove the label from the controller
  • When an entire option group was removed, the options in the group were mot removed from the controller

@googlebot
Copy link

CLAs look good, thanks!

scope.mySelect = 'C';
});
expect(selectCtrl.hasOption('C')).toBe(true);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

two empty blanks betweets its please. one day we'll have a jscs rule for this :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this test is based on #10161 (at 1.3 this is working, but I will need to backport this to 1.2). Anyhow, will add the extra checks

@lgalfaso
Copy link
Contributor Author

updated the PR with the proposed changes

@pkozlowski-opensource
Copy link
Member

@lgalfaso I've added it to the 1.3.5 milestone. Feel free to move it to 1.3.4 if you think it is close to being ready to merge. I'm restarting Travis to see what it thinks about your PR :-) (BTW, Travis is reallly flaky today...)

@lgalfaso
Copy link
Contributor Author

@pkozlowski-opensource if I get a 👍 from a core member team, I would like to commit this for 1.3.4 (and backport it so it makes it to 1.2.28)

});


it('should be able to detect when an element is replaced with an elements from a previous group', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

"an element" or "elements"?

@Narretz
Copy link
Contributor

Narretz commented Nov 26, 2014

I reviewed it too because I work on another select bug, and it's good practive in understanding this beast ;)
LGTM, only two little nitpicks

* When an option was moved to a previous group, the group that
loose the option would remove the label from the controller
* When an entire option group was removed, the options in the
group were mot removed from the controller
@lgalfaso
Copy link
Contributor Author

Updated the PR

@petebacondarwin
Copy link
Member

LGTM - I'll fix up that one element.controller bit and merge
Thanks @lgalfaso

@lgalfaso lgalfaso closed this in 30694c8 Nov 27, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants