Skip to content

Commit

Permalink
fix(ng:options): select correct element when '?'-option was previousl…
Browse files Browse the repository at this point in the history 
…y selected

Closes angular#599
  • Loading branch information
@TEHEK
TEHEK committed Oct 12, 2011
1 parent 74379df commit fec16a4
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/widget/select.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ angularWidget('select', function(element){
if (existingOption.id !== option.id) {
lastElement.val(existingOption.id = option.id);
}
if (existingOption.selected !== option.selected) {
if (existingOption.element.selected !== option.selected) {

This comment has been minimized.

Sign in to view

Copy link
@TEHEK

TEHEK Oct 18, 2011

Author Owner

existingOption was taken from cache (optionGroupsCache). When user switches selection to something else, the existingOption.element.selected is changed, but existingOption.selected is not.

It worked well if there was no '?' element in optionGroups (added on line 343). When added, it caused existingOptions and optionGroups to grow. When user switched from '?' to something else, the optionGroups was shrinked by 1 element, but existingOptions was not. The compiler started replacing labels and ids in <option> tags, but the selection was not updated.

Jeez. This commit was like 8 symbols, but the explanation and debugging took sooo long :)
lastElement.prop('selected', (existingOption.selected = option.selected));
}
} else {
Expand Down

1 comment on commit fec16a4

@IgorMinar
Copy link

Choose a reason for hiding this comment

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

awesome! but we'll need a test for this stuff so that we don't reintroduce it again. can you add that? it should be as simple as constructing a select element and options model, making sure that the selected model is undefined, then selecting an option element with browserTrigger(optionElement) and asserting that the right option element has the selected property.

Please sign in to comment.