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 parent fires twice #66

Open
bryanspears opened this issue Jun 30, 2015 · 4 comments
Open

Update parent fires twice #66

bryanspears opened this issue Jun 30, 2015 · 4 comments

Comments

@bryanspears
Copy link

Due to: https://github.com/AmpersandJS/ampersand-input-view/blob/master/ampersand-input-view.js#L68

Parent update gets duplicate call. At least it appears to be duplicated. I don't see any differences on the object passed.

Not sure what a proper fix would be. One suggestion would be to mimic the event pattern and send another argument that tells the parent which event is being fired.

@pgilad
Copy link
Member

pgilad commented Jun 30, 2015

Absolutely agree. change:valid should not trigger a parent.update().

@cdaringe
Copy link
Member

cdaringe commented Jul 1, 2015

@pgilad, do you mean change:value?

@pgilad
Copy link
Member

pgilad commented Jul 1, 2015

No I think update should update the parent on value changes, not on valid changes?

@cdaringe
Copy link
Member

cdaringe commented Jul 1, 2015

http://ampersandjs.com/learn/forms#form-input-view-conventions

It reports changes to its parent when it deems appropriate by calling this.parent.update(this) **note that it passes itself to the parent. You would typically do this when the this.value has changed or the this.valid has changed.

in the grand scheme, i suppose that a value change transitively drives valid change, so two events may not be reqd. im not sure if that covers all cases tho. e.g. can valid ever change without value changing? the form itself should be checking child fields for validity on update regardless, supporting your guys' points

can valid ever change without value changing
let's agree to an answer on this, and if it comes out with a 'yes', lets drop the valid evt and update the docs. gotta run!

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

No branches or pull requests

3 participants