powmedia / backbone-forms

Form framework for BackboneJS with nested forms, editable lists and validation
MIT License
2.17k stars 413 forks source link

form.render() uses stale model data on rerender #361

Open 0xgeert opened 10 years ago

0xgeert commented 10 years ago

As I understand it, it should be possible to call form.render() multiple times, not just once on init. Is this correct?

Given his assumption, form.render() does not work as intended for a lot of editors. Consider the case in which the model which the form was based on was changed outside of the forms control. E.g.: through some other view or programmatically.

To update the form with the model you'd do: form.setValue(user.toJSON())

This correctly reflects the new state of the model in the form. However, for various reasons, you might want to do a form.render() afterwards.

This does not work as intended as the newly set values through form.setValue() are completely discarded by form.render().

See this fiddle (which includes an explanation as to the why as well) http://jsfiddle.net/4XZMb/2079/

Should form.render() not be used in this way, or is this a bug?

0xgeert commented 10 years ago

related to #357 , which was too specific

exussum12 commented 10 years ago

Why dont you change the value of the form programmatically ? form.getEditor('name').setValue('value');

It seems harder to maintain 2 lists and merge them rather than using the same value ? this would also eliminate the need for a re render

0xgeert commented 10 years ago

As mentioned in the fiddle, getting rid of previously shown validation html is one reason I want to do a rerender. I realize now this can be done for every field by doing field.clearError(), but it's a bit beside the point imho. I just want to be sure I can call form.render() to do a clean rerender based on the latest values whenever the need arises..

exussum12 commented 10 years ago

http://jsfiddle.net/4XZMb/2080/ does a complete re render after setting the value to something else, Wait 3 seconds to see it happen

But calling render multiple times is ok.

Let backbone forms manage the backbone model, If you do everything else in the form object and then just commit when you need to, that seems to be the best way

As a side note, I check the field on blur so errors show up and go away instantly.

powmedia commented 10 years ago

Although there are workarounds, it would be great if render() was safe to use for whatever reason

0xgeert commented 10 years ago

Let backbone forms manage the backbone model

A View (which a Form is) should correctly reflect what the Model represents, not the other way around. If the model is updated by another View (multiple Views per Model occurs a lot) I don't want to second guess if that's going to be a problem.

@powmedia: having quickly tested on texteditor the following appears to solve the issue:

setValue: function(value) {
    this.value = value; //this line added, which is the same as 'setValue' in the superclass.
    this.$el.val(value);
},

Doing something similar for all editors may (naively?) be the way to go, although I'm obviously not as familiar with the intrinsics as you are. Any obstacles you believe to be ahead? I can do a push otherwise.

exussum12 commented 10 years ago

Ive just done the same thing, And it passed all current unit tests.

Thats a fair point about the form being a view for the model

powmedia commented 10 years ago

Would be worth including then I think; do you have it available as a pull request? On Feb 20, 2014 12:27 AM, "Scott Dutton" notifications@github.com wrote:

Ive just done the same thing, And it passed all current unit tests.

Thats a fair point about the form being a view for the model

Reply to this email directly or view it on GitHubhttps://github.com/powmedia/backbone-forms/issues/361#issuecomment-35517346 .

0xgeert commented 10 years ago

Going to do it after the weekend.

Blasz commented 9 years ago

Not sure why #363 wasn't landed to fix this issue.