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

Update API docs for FormElement#value #1108

Merged
merged 1 commit into from
Jun 13, 2020
Merged

Conversation

simonihmig
Copy link
Contributor

cc @basz

@basz
Copy link
Contributor

basz commented Jun 12, 2020

lgtm

Copy link
Contributor

@jelhan jelhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

I'm only not sure if we should recommend to override default event handler or to add your own. But that's not directly related to this PR. To be honest I'm not even fully sure if <BsForm:: Element {{on "change" this.foo}} /> works and if we consider it public API...

*
* ```hbs
* <form.element @controlType="email" @label="Email" @value={{this.email}} />
* ```
*
* Note: you lose the ability to validate this form element by directly binding to its value. It is recommended
* Note two things:
* * the binding is uni-directional (DDAU), so you would have to use the `onChange` action to subscribe to changes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does {{on "change" this.updateMyValue}} work as well? I think / hope so but haven't checked myself. Not sure if we should recommend that one instead of overriding onChange.

Copy link
Contributor Author

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 can work, as the element that ...attributes is passed to is .form-group IIRC. Which obviously has no change event.

But that's what onChange is explicitly for. When using property it is already wired up to essentially mutate model, but when using @value you would want to use it.

@simonihmig simonihmig merged commit e73bad9 into master Jun 13, 2020
@simonihmig simonihmig deleted the update-form-value-docs branch June 13, 2020 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants