-
-
Notifications
You must be signed in to change notification settings - Fork 141
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
Add set test #401
Add set test #401
Conversation
assert.equal(c.get('org.usa.mn'), 'undefined'); | ||
assert.equal(dummyModel.org.usa.ny, 'nil2'); | ||
assert.equal(dummyModel.org.usa.mn, 'undefined'); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@josemarluedke Does this test look right to you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep. Looks good! Interesting that it is passing. Not sure how to capture that bug in a test then.
README.md
Outdated
<Input @value={{changeset.lastName}} /> | ||
<input | ||
value={{model.firstName}} | ||
oninput={{action (changeset-set this.changeset "model.lastName") value="target.value"}}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@josemarluedke Do these README updates look ok to you? ie. the model
as the value
as noted #398 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this change is correct. I believe forms should always reflect the in-flight model. This way, we can revert changes and have forms reflect it without any issues. Also, If you have a form in a tab layout for example, and you hide half of the form then go back to that, the form would show the incorrect data, the user would think they have lost their changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reverted. Thanks a ton for the insight.
This reverts commit b9349c4.
adopted-ember-addons/validated-changeset#9 (comment)