New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set a default selected object, if no "prompt" exists or if no default selection binding exists. #10997

Closed
wants to merge 9 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@grapho

grapho commented Apr 30, 2015

Related to issue: #10984

This workaround should solve an issue where an Ember.Select will default to the last array object (or <option>) in certain browsers.

THis issue would occur for single type Ember.Selects which have no "prompt" property defined, and a selection or value property bound to a null value.

This is done in the _setDefault() method, and only on('didInsertElement').. allowing all the correct observers to fire and not interfere with subsequest selection changes.

Seth added some commits May 1, 2015

Seth
Update select.js
Since `setDefaults()`  does a little more "checking" I decided to let it delegate to the `_changeSingle()` / `_changeMultiple()`, removing the need for the `_change()` method.

Also I noticed i used the older set_property syntax earlier... which seems to be why the build failed at first.
Seth
Update select.js
This works i swear O:)

I just messed up a couple lines O:)
Seth
Update select.js
travis is so thorough, yikes
Seth
Select should always default to the first option
removing _change() was a mistake.. lolz
@grapho

This comment has been minimized.

Show comment
Hide comment
@grapho

grapho May 1, 2015

@rwjblue @mixonic ... well I tried... and Travis kicked my butt. He is very thorough.

I think it is close, but there are a few subtleties I am missing.. causing the build to fail.

The work around was not as straightforward as i originally thought.

Could I get a couple eyes on it to see what i missed?

grapho commented May 1, 2015

@rwjblue @mixonic ... well I tried... and Travis kicked my butt. He is very thorough.

I think it is close, but there are a few subtleties I am missing.. causing the build to fail.

The work around was not as straightforward as i originally thought.

Could I get a couple eyes on it to see what i missed?

@mixonic

This comment has been minimized.

Show comment
Hide comment
@mixonic

mixonic May 2, 2015

Member

I have unwound this same ball of wax :-/ I have not tried your approach of just cleaning up the bad behavior after render.

I don't suggest doing this in setDefaults unless it is used exclusively in didInsertElement. I'm not sure how you can do this though- the selectedIndex is already set to a non -1 value in didInsertElement and I'm not sure how you can know that it is not to be trusted.

Member

mixonic commented May 2, 2015

I have unwound this same ball of wax :-/ I have not tried your approach of just cleaning up the bad behavior after render.

I don't suggest doing this in setDefaults unless it is used exclusively in didInsertElement. I'm not sure how you can do this though- the selectedIndex is already set to a non -1 value in didInsertElement and I'm not sure how you can know that it is not to be trusted.

@grapho

This comment has been minimized.

Show comment
Hide comment
@grapho

grapho May 4, 2015

@mixonic setDefaults() is actually called exclusively on didInsertElement via its init method and changing the selection property triggers an observer to set the selectedIndex afterward in selectionDidChangeSingle and setSelectedIndex methods. It seemed like it would work fine and indeed it seems to in real life.. though travis seemed to find a singular failed test in a case where the selection binding was bound to a value of "0"?

All that being said... I am willing to drop the issue here, because it seems to be more of a browser/rendering issue for chrome as you said? Coding to compensate for that does not seem to be the long term solution.

grapho commented May 4, 2015

@mixonic setDefaults() is actually called exclusively on didInsertElement via its init method and changing the selection property triggers an observer to set the selectedIndex afterward in selectionDidChangeSingle and setSelectedIndex methods. It seemed like it would work fine and indeed it seems to in real life.. though travis seemed to find a singular failed test in a case where the selection binding was bound to a value of "0"?

All that being said... I am willing to drop the issue here, because it seems to be more of a browser/rendering issue for chrome as you said? Coding to compensate for that does not seem to be the long term solution.

@stefanpenner

This comment has been minimized.

Show comment
Hide comment
@stefanpenner

stefanpenner Jul 8, 2015

Member

Select is deprecated for 2.0. If there is interest in making this work in the 1.13 branch, feel free to let me know and I will gladly reopen.

FYI: as of 1.13.3 you can just use the DOM via HTMLBars directly to get some really slick and wonderful selects example

Member

stefanpenner commented Jul 8, 2015

Select is deprecated for 2.0. If there is interest in making this work in the 1.13 branch, feel free to let me know and I will gladly reopen.

FYI: as of 1.13.3 you can just use the DOM via HTMLBars directly to get some really slick and wonderful selects example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment