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

[Discussion] Fine tuning validation #12

Closed
serkandurusoy opened this issue May 27, 2016 · 18 comments
Closed

[Discussion] Fine tuning validation #12

serkandurusoy opened this issue May 27, 2016 · 18 comments
Assignees
Labels
Type: Feature New features and feature requests

Comments

@serkandurusoy
Copy link
Contributor

serkandurusoy commented May 27, 2016

Autoform (aldeed) provides options to fine tune the validation lifecycle.

It allows for setting when validation should occur. For example, after a keyup, or blur, or submit, or combinations like submitThenBlur and submitThenKeyup (default) which makes for a very configurable UX.

Furthermore, some form libraries (like formsy and redux-form) provide a concept called pristine which denotes the form has not been touched yet.

When the form is first rendered, it is pristine. And all the fields are pristine.

And if a field's value has changed (even if it has not yet been validated) that field becomes pristine: false and the form becomes pristine: false while other fields remain pristine.

What this allows is to create custom inputs or tap into the form/input lifecycle where we know if the input's or the form's internal state has changed or not and using that information for a customized UX.

note: conversation carried over from #11

@serkandurusoy
Copy link
Contributor Author

@radekmie you mentioned validate=onChangeAfterSubmit

is this a part of the current api? I could not see a reference to that, so I'm curious. Or is this something you are thinking of implementing?

@radekmie
Copy link
Contributor

@serkandurusoy yes, it is. I've just discovered it's not described in README. More informations are available in packages/uniforms/src/components/forms/ValidatedForm.js. I'll update README later.

@serkandurusoy
Copy link
Contributor Author

Wow this is great news!

What about getting the state of the form and/or individual inputs in terms of their values have been changed or not? I think their error status is already available but I think we don't have a very straightforward way of knowing if values have been edited, right?

@radekmie
Copy link
Contributor

Again, it's an idea for functionality. I'll think about it and later make a draft of API.

@serkandurusoy
Copy link
Contributor Author

Hmm I think we can provide an onChange(key, value) function and listed to that to set some state, but yeah, it is not directly in the api.

thanks for giving it consideration!

@radekmie
Copy link
Contributor

radekmie commented May 27, 2016

Okay, I think it could be like this - every field will receive a changed prop (PropTypes.bool.isRequired) - that should be enough (for fields). Also, every form could have changed field in it's state (same type), that could be a part of context.uniforms.state. Does it makes sense for you?

@radekmie radekmie self-assigned this May 27, 2016
@serkandurusoy
Copy link
Contributor Author

Yep, a changed prop on fields and changed on context.state for form is quite sufficient.

@radekmie
Copy link
Contributor

For the following model:

{
    a: {
        b: 1,
        c: [
            2
        ]
    }
}

We've got there the following cases:

  1. When a.b has changed, then both a and a.b are marked as changed.
  2. When a.c.0 has changed, then all a, a.c and a.c.0 are marked as changed.
  3. When a.c.1 appears, then both a and a.c are marked, but what with a.c.1 - has it changed?
  4. When a.c.0 is removed, then added again, is it changed?

@serkandurusoy
Copy link
Contributor Author

1 and 2 makes sense.

3, I think yes a.c.1 has changed because previously, a.c[1] was undefined and now not! so I would expect it to be reported as changed

4, yes it has changed. because if you remove a.c[0] it has changed at that point. readding it does not guarantee equality because it would be a new empty value initially.

Whatabout reording arrays? It would be best if you would be able to keep track of individual array items as they get reordered

@radekmie radekmie added the Type: Feature New features and feature requests label May 28, 2016
@radekmie
Copy link
Contributor

Reordering is also change, becouse model's field x is now x' and field y is now y' (I think). There's another problem - what about reordering equal values?

@serkandurusoy
Copy link
Contributor Author

I would say that also constitutes a change.

Perhaps the thinking would be, if user has interacted with the form, whatever the event target has been, and bubbling up the parent up to the form itself, has been changed.

@serkandurusoy
Copy link
Contributor Author

At that point, I think it should be up the developer to keep track of those changes and decide accordingly.

Even react itself has a shouldComponentUpdate directive becuase deep equality checking is expensive so we should not be caring to compare before and after states in deep structures.

@radekmie
Copy link
Contributor

Let's summarize: if field has changed (adding or removing array item is a change too), it's marked as changed. That status bubbles up to the form component.

@serkandurusoy
Copy link
Contributor Author

Yep, I think that's it.

@radekmie
Copy link
Contributor

Okay, I'll implement and document it later. Thanks for idea and discussion.

@serkandurusoy
Copy link
Contributor Author

Sure thing! Thank you for the package and being so awesome with feature requests!

@radekmie
Copy link
Contributor

I've wrote few tests, but it would be better to test it within exact use case (or write more tests).

@serkandurusoy
Copy link
Contributor Author

Sure thing! I am going to be using this in a project in the following days
and I'll definitely report back my findings if I see anything out of place.
Thanks!

On Sun, May 29, 2016 at 9:10 PM, Radosław Miernik notifications@github.com
wrote:

I've wrote few tests, but it would be better to test it within exact
use case (or write more tests).


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#12 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AEbz3I5PQna0KzzBXWYVs-brcSO4z3Apks5qGdaWgaJpZM4IouYG
.

@Monteth Monteth added this to Closed in Open Source (migrated) Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New features and feature requests
Projects
Archived in project
Development

No branches or pull requests

2 participants