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

fix(ngOptions): don't change user selections while <select> open in fire... #6680

Closed
wants to merge 1 commit into from

Conversation

heymatthew
Copy link

Sample patch that fixes issue #6679 but may regress parts of the code base.

I'm finding it difficult to verify the test suite because these unit tests are failing for me in New Zealand (see issue #5017). But time zone errors aside, I believe my patch breaks subtly with this output:

Chrome 33.0.1750 (Linux) select ngOptions binding should select correct input if previously selected option was "?" FAILED
  Expected '1' to equal '0'.
  Error: Expected '1' to equal '0'.
      at null.<anonymous> (/home/mbgray/workspace/angular.js/test/ng/directive/selectSpec.js:965:31)
  Expected false to be truthy.
  Error: Expected false to be truthy.
      at null.<anonymous> (/home/mbgray/workspace/angular.js/test/ng/directive/selectSpec.js:966:63)

I'm submitting it anyway because it might be a starting point for conversation and other fixes. I'm not as comfortable this deep in the framework and didn't know how to take this further.

…irefox

I doubt this will be merged, pushing this out for review by someone
more confortable with angular core.

Closes #6679
Regresses #599
@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 (#6680)

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!

@Narretz
Copy link
Contributor

Narretz commented Mar 14, 2014

I think this is a fix for the same thing: #5994 (but without the regression)

@heymatthew
Copy link
Author

@Narretz yep, looks like the same issue. I didn't see that one, cheers for pointing it out. I'll close #6680 in favor of #5994.

@IgorMinar IgorMinar self-assigned this Mar 17, 2014
@IgorMinar
Copy link
Contributor

since this is a dupe, I'm closing it. thanks!

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants