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

fix(ngOption): accept 0 as an option group #7024

Closed
wants to merge 1 commit into from

Conversation

lgalfaso
Copy link
Contributor

@lgalfaso lgalfaso commented Apr 7, 2014

Accept 0 and an option group

Close #7015

BREAKING CHANGE

When using option groups at an ng-option, the literal 0 will
be handled as a group with label '0'. Use the literal string ''
or undefined for options that should not be in any option group

@mary-poppins
Copy link

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#7024)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

Accept `0` and an option group

Close angular#7015

BREAKING CHANGE

When using option groups at an ng-option, the literal `0` will
be handled as a group with label `'0'`. Use the literal string `''`
or undefined for options that should not be in any option group
@caitp
Copy link
Contributor

caitp commented Apr 7, 2014

Is this really a "breaking" change? seems like that was probably accidental, not an intentional thing. But it's all good, good job coming up with a fix

@lgalfaso lgalfaso added cla: yes and removed cla: no labels Apr 7, 2014
@lefos987 lefos987 added this to the Backlog milestone Apr 7, 2014
@lgalfaso
Copy link
Contributor Author

lgalfaso commented Apr 7, 2014

@caitp Now we accept all falsy values (with the exception of '') as valid option groups. Even when I doubt that anybody put 0 by mistake, the chances of someone putting null are bigger enough for the notice

@caitp
Copy link
Contributor

caitp commented Apr 7, 2014

seems unlikely, if they're using ngOptions, they probably aren't using 'null' as an expression, since it would apply to all optgroups...

@lgalfaso
Copy link
Contributor Author

lgalfaso commented Apr 7, 2014

ok, let me know if you would like me to amend the commit message

@Narretz Narretz removed this from the Backlog milestone Sep 14, 2015
@petebacondarwin
Copy link
Contributor

Sorry @lgalfaso this needs a hefty rebase since ngOptions has changed so much.

@petebacondarwin
Copy link
Contributor

But it seems that your new unit test does not fail with the refactored code, so I think this issue is actually already fixed.

@petebacondarwin
Copy link
Contributor

Ah! Actually, your unit test uses "0" rather than 0, so it is not failing. This is still an issue.

petebacondarwin added a commit to petebacondarwin/angular.js that referenced this pull request Sep 18, 2015
Now one can use `''`, `0`, `false` and `null` as option groups. Previously
all of these falsy values were treated as the option not being a member of
a group.

Closes angular#7015
Closes angular#7024

BREAKING CHANGES
If your data contains falsy values for option groups, then these options
will now be placed into option groups. Only option groups that are `undefined`
will result in the option being put in no group. If you have data that
contains falsy values that should not be used as groups then you must
filter the values before passing them to `ngOptions` converting falsy
values to `undefined`.
@petebacondarwin
Copy link
Contributor

Replacing with #12888

petebacondarwin added a commit that referenced this pull request Sep 20, 2015
Now one can use `''`, `0`, `false` and `null` as option groups. Previously
all of these falsy values were treated as the option not being a member of
a group.

Closes #7015
Closes #7024
Closes #12888

BREAKING CHANGES
If your data contains falsy values for option groups, then these options
will now be placed into option groups. Only option groups that are `undefined`
will result in the option being put in no group. If you have data that
contains falsy values that should not be used as groups then you must
filter the values before passing them to `ngOptions` converting falsy
values to `undefined`.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ngOptions group by does not display groups with value 0
7 participants