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

fix(select): placeholder (empty option) is lost in IE9 #2165

Closed

Conversation

henrikhulgaard
Copy link

Fixes a check inside render for select elements with ngOptions, which compares the selected property of an element with it's desired state.
In instances where no element should be selected, this resulted in the first option in the select element having it's selected attribute set from undefined to false.
In most browsers, this has the effect of displaying the first item in the list. In IE9 however, this causes the select to display nothing.
In other browsers this would still cause unnecessary changes in selected state, but no visible issue would manifest.

Closes #2150, #1826

Fixes a check inside render for select elements with ngOptions, which compares the selected property of an element with it's desired state.
In instances where no element should be selected, this resulted in the first option in the select element having it's selected attribute set from undefined to false.
In most browsers, this has the effect of displaying the first item in the list. In IE9 however, this causes the select to display nothing.
In other browsers this would still cause unnecessary changes in selected state, but no visible issue would manifest.

Closes angular#2150, angular#1826
@mhevery
Copy link
Contributor

mhevery commented Mar 16, 2013

PR Checklist (Minor Bugfix)

  • Contributor signed CLA now or in the past (if you just signed, leave a comment here with your real name)
  • PR doesn't introduce new api
  • PR doesn't contain a breaking change
  • PR contains unit tests
  • [-] PR contains e2e tests (if suitable)
  • [-] PR contains documentation update (if suitable)
  • PR passes all tests on Travis (sanity)
  • PR passes all tests on ci.angularjs.org (cross-browser compatibility)
  • PR is rebased against recent master
  • PR is squashed into one commit per logical change
  • PR's commit messages are descriptive and allows us to autogenerate release notes (required commit message format)
  • All changes requested in review have been implemented

@ghost ghost assigned mhevery Mar 16, 2013
@henrikhulgaard
Copy link
Author

The CLA has been signed and submitted to [email protected] on Mon, Mar 4, 2013 at 2:50 PM.

@ghost
Copy link

ghost commented Mar 18, 2013

@mhevery Could you clarify the remaining checklist items please?

The PR is marked good to merge according to Travis and I'm not sure how to trigger a build of it on Jenkins.

Is there anything else which needs to be completed?

@pkozlowski-opensource
Copy link
Member

@chad-configit don't worry about tests on Jenkins, this is something we trigger on our end just before merging to check if tests pass on all supported browsers. I've also marked Travis state accordingly.

@ghost
Copy link

ghost commented Mar 18, 2013

Unfortunately I've just discovered a side effect to this PR; deselection in a multi-select isn't working as it should.

I shall recommit with the correction.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The placeholder (null option) in an ngSelect is broken in certain scenarios on IE9
3 participants