ericgj / elm-validation

Tools for managing validity and error state of data, e.g. from user input.
MIT License
6 stars 3 forks source link

Support validation on blur or submit? #3

Open la-yumba opened 6 years ago

la-yumba commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

PS. See

ericgj commented 6 years ago

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

la-yumba commented 6 years ago

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 commented 6 years ago

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

cjduncana commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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):