Skip to content
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

Allow empty string for optionValuePath on fm-select #43

Merged
merged 5 commits into from
Jan 18, 2016

Conversation

g-cassie
Copy link
Collaborator

This change makes it possible to set optionValuePath='' and have the entire object from the content list be set to the value of the property. This is a very common pattern with Ember-Data.

This behaviour was working prior to 8feb869 when we switched over to the built in get helper. This restores the previous functionality and adds a test.

@g-cassie
Copy link
Collaborator Author

These failures seem to also exist on master... I think there is something going on with Ember-Try. I noticed on @jelhan 's PRs that the builds were failing on beta - maybe this is biting us now?

Edit: @Emerson I think you have may broken some of @jelhan 's tests when you added the fm-config.showErrorsByDefault property.

@jelhan
Copy link
Contributor

jelhan commented Jan 16, 2016

@g-cassie Tests are fixed in #45. Especially 8b1aef4 should be necessary to get the tests running.

g-cassie added a commit that referenced this pull request Jan 18, 2016
Allow empty string for optionValuePath on fm-select
@g-cassie g-cassie merged commit 8ec7602 into Emerson:master Jan 18, 2016
@Emerson
Copy link
Owner

Emerson commented Jan 18, 2016

So nice to see the green "Passing" text again – thanks @g-cassie

@g-cassie
Copy link
Collaborator Author

That was mostly @jelhan on that one. @Emerson can you take a look at our discussion in #37 when you have a chance? I think I will start out on that later today.

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

Successfully merging this pull request may close these issues.

3 participants