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

Support validation on blur or submit? #3

Open
la-yumba opened this Issue Dec 11, 2017 · 13 comments

Comments

Projects
None yet
4 participants
@la-yumba

la-yumba commented Dec 11, 2017

Hi,

I generally like the approach, but I feel that Initial should also contain a payload of type String. This would store the user's input while the user is first typing, but before validation occurs.

I assume validation should occur on blur or on submit, since it's considered bad user experience to show, say, "Invalid email" as soon as the user has entered the first character in an email field.

@ericgj

This comment has been minimized.

Show comment
Hide comment
@ericgj

ericgj Dec 11, 2017

Owner

Thanks for the feedback. It's a good point, in many cases you don't want to validate "instantly" (onInput). How about another state, Unvalidated String ?

Owner

ericgj commented Dec 11, 2017

Thanks for the feedback. It's a good point, in many cases you don't want to validate "instantly" (onInput). How about another state, Unvalidated String ?

@la-yumba

This comment has been minimized.

Show comment
Hide comment
@la-yumba

la-yumba Dec 11, 2017

Well, yes - Unvalidated is a better name than Initial for the case where you're postponing validation. But in that case I would question whether there is a need for the Initial case, since it could always be replaced with Unvalidated "", or Valid defaultValue

la-yumba commented Dec 11, 2017

Well, yes - Unvalidated is a better name than Initial for the case where you're postponing validation. But in that case I would question whether there is a need for the Initial case, since it could always be replaced with Unvalidated "", or Valid defaultValue

@ericgj

This comment has been minimized.

Show comment
Hide comment
@ericgj

ericgj Dec 11, 2017

Owner

I see what you mean, but there may be a case where you want to distinguish between "the user hasn't touched this field at all" and "the user entered something and then deleted it". And it means less of a breaking change to add a new state.

Owner

ericgj commented Dec 11, 2017

I see what you mean, but there may be a case where you want to distinguish between "the user hasn't touched this field at all" and "the user entered something and then deleted it". And it means less of a breaking change to add a new state.

@ericgj

This comment has been minimized.

Show comment
Hide comment
@ericgj

This comment has been minimized.

Show comment
Hide comment
@ericgj

ericgj Dec 14, 2017

Owner

Any more comments? Otherwise I will go ahead and release a new version soon.

Owner

ericgj commented Dec 14, 2017

Any more comments? Otherwise I will go ahead and release a new version soon.

@la-yumba

This comment has been minimized.

Show comment
Hide comment
@la-yumba

la-yumba Dec 14, 2017

Yes, I think that's better, although personally I'm not really sure that you'd want to distinguish between "the user hasn't touched this field at all" and "the user entered something and then (without ever leaving the field) deleted it"... I just can't think of a scenario right now; maybe there is some edge case, but is it worth it? having an extra case to handle for every case expression?

Also, to be precise adding a state is a breaking change (client code will break because case expressions will be incomplete)

la-yumba commented Dec 14, 2017

Yes, I think that's better, although personally I'm not really sure that you'd want to distinguish between "the user hasn't touched this field at all" and "the user entered something and then (without ever leaving the field) deleted it"... I just can't think of a scenario right now; maybe there is some edge case, but is it worth it? having an extra case to handle for every case expression?

Also, to be precise adding a state is a breaking change (client code will break because case expressions will be incomplete)

@ericgj

This comment has been minimized.

Show comment
Hide comment
@ericgj

ericgj Dec 14, 2017

Owner

Thanks - I have opened up the question on the elm discourse: https://discourse.elm-lang.org/t/update-to-elm-validation-library/255

Owner

ericgj commented Dec 14, 2017

Thanks - I have opened up the question on the elm discourse: https://discourse.elm-lang.org/t/update-to-elm-validation-library/255

@cjduncana

This comment has been minimized.

Show comment
Hide comment
@cjduncana

cjduncana Dec 22, 2017

Hello Eric,

I see that a week has passed since this has been opened, and I wanted to ask for any update on this issue. I've commented on this issue in the Discourse page, and I was looking forward to this update, because there was a conversation in Slack where another user wanted to adopt my version of Validation and asked me to create a package for it.

When I saw that you were asking for feedback, I thought that my package would become unnecessary, so I halted the package until you came to a decision, but I haven't seen that happen. What do you think about this? Should I wait more, or should I publish my package? Do you see a better alternative?

Hope to hear from you soon,
Chris Duncan

cjduncana commented Dec 22, 2017

Hello Eric,

I see that a week has passed since this has been opened, and I wanted to ask for any update on this issue. I've commented on this issue in the Discourse page, and I was looking forward to this update, because there was a conversation in Slack where another user wanted to adopt my version of Validation and asked me to create a package for it.

When I saw that you were asking for feedback, I thought that my package would become unnecessary, so I halted the package until you came to a decision, but I haven't seen that happen. What do you think about this? Should I wait more, or should I publish my package? Do you see a better alternative?

Hope to hear from you soon,
Chris Duncan

@ericgj

This comment has been minimized.

Show comment
Hide comment
@ericgj

ericgj Dec 22, 2017

Owner

Thanks for following up about this Chris. I have been on a trip, and then got sick :( I saw the back and forth on Discourse. My plan was to look at your example code using Pristine and Dirty, and also look at what UX people are saying now about how to best do form validation.

If you or someone else want to use your model right away, I'd say go ahead and publish it, maybe with a comment in the README about what it enables that my model doesn't. We can potentially merge it later. I know there is a tendency in the Elm community to want to have one library for every bit of functionality, and try to come to agreement before duplicating functionality, which I think is generally great, a refreshing change from JS.

But for UI/UX stuff, often in one case you need a "fuller" model for more subtle behavioral things, while for someone else this seems totally unnecessary. Look at how many autocomplete menus there are out there :) Another option down the road is to have separate modules in one library, e.g. Validation, Validation.Dirty.

Personally I want to do a little more research -- and not sure how much time I will have to do this over the next week. So I'd suggest if you need it right away, go ahead and publish it, or inline it within your app / share it as a snippet.

What do you think?

Owner

ericgj commented Dec 22, 2017

Thanks for following up about this Chris. I have been on a trip, and then got sick :( I saw the back and forth on Discourse. My plan was to look at your example code using Pristine and Dirty, and also look at what UX people are saying now about how to best do form validation.

If you or someone else want to use your model right away, I'd say go ahead and publish it, maybe with a comment in the README about what it enables that my model doesn't. We can potentially merge it later. I know there is a tendency in the Elm community to want to have one library for every bit of functionality, and try to come to agreement before duplicating functionality, which I think is generally great, a refreshing change from JS.

But for UI/UX stuff, often in one case you need a "fuller" model for more subtle behavioral things, while for someone else this seems totally unnecessary. Look at how many autocomplete menus there are out there :) Another option down the road is to have separate modules in one library, e.g. Validation, Validation.Dirty.

Personally I want to do a little more research -- and not sure how much time I will have to do this over the next week. So I'd suggest if you need it right away, go ahead and publish it, or inline it within your app / share it as a snippet.

What do you think?

@cjduncana

This comment has been minimized.

Show comment
Hide comment
@cjduncana

cjduncana Dec 22, 2017

I think that this great! I am in no hurry to move this forward. I was just worried that it was forgotten. Thanks for taking your time and I hope that you get well soon.

I don't believe that it is necessary to have a separate module for the following reason. Any action and manipulation that can be done with the current implementation, can also be done with my proposed implementation, and the added tag allows to better represent all the possible states of the input data.

The downside I can see would affect users that do not need that specific tag, but only if they use a case statement. If they do not use a case statement, then the handling of the new tag will be hidden to the user by your library. I personally think that the pros outweigh the cons, but that is your decision to take. ;)

cjduncana commented Dec 22, 2017

I think that this great! I am in no hurry to move this forward. I was just worried that it was forgotten. Thanks for taking your time and I hope that you get well soon.

I don't believe that it is necessary to have a separate module for the following reason. Any action and manipulation that can be done with the current implementation, can also be done with my proposed implementation, and the added tag allows to better represent all the possible states of the input data.

The downside I can see would affect users that do not need that specific tag, but only if they use a case statement. If they do not use a case statement, then the handling of the new tag will be hidden to the user by your library. I personally think that the pros outweigh the cons, but that is your decision to take. ;)

@ngw

This comment has been minimized.

Show comment
Hide comment
@ngw

ngw Mar 2, 2018

Is this still going? I'd really need this feature right now. I'm a newbie with Elm but glad to help if I can.

ngw commented Mar 2, 2018

Is this still going? I'd really need this feature right now. I'm a newbie with Elm but glad to help if I can.

@la-yumba

This comment has been minimized.

Show comment
Hide comment
@la-yumba

la-yumba Mar 2, 2018

@ngw I'm not sure whether this is going to be implemented in this library, but in case it's helpful I'd like to point you to my course on form validation in Elm where I show how to validate onInput, onSubmit, and onBlur. The videos are for subscribers only but the code samples are open source.

la-yumba commented Mar 2, 2018

@ngw I'm not sure whether this is going to be implemented in this library, but in case it's helpful I'd like to point you to my course on form validation in Elm where I show how to validate onInput, onSubmit, and onBlur. The videos are for subscribers only but the code samples are open source.

@ericgj

This comment has been minimized.

Show comment
Hide comment
@ericgj

ericgj Mar 3, 2018

Owner

Hi @ngw, does the "Unvalidated" model I proposed work for you? Example using it with onBlur here.

type ValidationResult value
    = Initial
    | Unvalidated String
    | Valid value
    | Invalid String String

@cjduncana and I had some more discussion to see if we could merge our efforts, but there was no conclusion. To summarize (and feel free to correct this cjduncana):

  • The only structural difference is he has Pristine String where I have Initial.

  • He pointed out that there's no easy way of getting the string representation of a Valid value (unless that value happens to be a String), for setting the initial input value.

  • My suggestion was to add a helper function toStringWithDefault : String -> (val -> String) -> ValidationResult val -> String, which converts a valid value to a string using the passed function, or otherwise uses a default string. And a function for the typical (empty-string) case, toString = toStringWithDefault "".

  • He prefers to have the valid string value contained within Initial, AKA Pristine.

  • I am wary of doing it that way because it's like saying we should have type Maybe a = Nothing String | Just a because we need to get a default string value in some cases. Also, what if your default string is context-specific, say it should not just be a static value but depend on the day of the week. Then you could potentially end up in a situation where the String captured in Pristine String is different than the default string you are calculating and passing in from the outside.

  • cjduncana explained that he sees this case as different from Maybe. "Maybe is explicit in that it represents values that may be absent. Pristine, Dirty, Invalid, and Valid represents the state of an input. Input always have a String when Pristine, but most of the time it is the empty string, but we also see default values or hints used instead."

  • I think we left it that cjduncana's approach had a bit broader scope, was more about modelling the "whole input process" or "state of the input" rather than simply "validating the input", and so possibly having separate libraries was worth it.

Owner

ericgj commented Mar 3, 2018

Hi @ngw, does the "Unvalidated" model I proposed work for you? Example using it with onBlur here.

type ValidationResult value
    = Initial
    | Unvalidated String
    | Valid value
    | Invalid String String

@cjduncana and I had some more discussion to see if we could merge our efforts, but there was no conclusion. To summarize (and feel free to correct this cjduncana):

  • The only structural difference is he has Pristine String where I have Initial.

  • He pointed out that there's no easy way of getting the string representation of a Valid value (unless that value happens to be a String), for setting the initial input value.

  • My suggestion was to add a helper function toStringWithDefault : String -> (val -> String) -> ValidationResult val -> String, which converts a valid value to a string using the passed function, or otherwise uses a default string. And a function for the typical (empty-string) case, toString = toStringWithDefault "".

  • He prefers to have the valid string value contained within Initial, AKA Pristine.

  • I am wary of doing it that way because it's like saying we should have type Maybe a = Nothing String | Just a because we need to get a default string value in some cases. Also, what if your default string is context-specific, say it should not just be a static value but depend on the day of the week. Then you could potentially end up in a situation where the String captured in Pristine String is different than the default string you are calculating and passing in from the outside.

  • cjduncana explained that he sees this case as different from Maybe. "Maybe is explicit in that it represents values that may be absent. Pristine, Dirty, Invalid, and Valid represents the state of an input. Input always have a String when Pristine, but most of the time it is the empty string, but we also see default values or hints used instead."

  • I think we left it that cjduncana's approach had a bit broader scope, was more about modelling the "whole input process" or "state of the input" rather than simply "validating the input", and so possibly having separate libraries was worth it.

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