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

Add validation and visibility on errors #22

Open
22 tasks done
mikemajara opened this issue Feb 4, 2022 · 5 comments
Open
22 tasks done

Add validation and visibility on errors #22

mikemajara opened this issue Feb 4, 2022 · 5 comments

Comments

@mikemajara
Copy link
Contributor

mikemajara commented Feb 4, 2022

The library as is doesn't show some (if not all) errors.

Given Google manages that within the browser we need to parse that out of the form-raw.json and add it to the validation object available in react-hook-form to validate previous to submit.

I've been exploring how this can be done (check in my fork's master)

image

This would be the list for Short Answers, which would be the first thing I'd focus on

  • Number
    • gt
    • gte
    • lt
    • lte
    • eq
    • neq
    • between
    • not between
    • is number
    • whole number
  • Text
    • Contains
    • Doesn’t contain
    • Email
    • URL
  • Length
    • Max char count
    • Min char count
  • Regular Expression
    • Contains
    • Doesn’t contain
    • Matches
    • Doesn’t match
  • Long text
    • Length ☝️
    • Regular Expression ☝️
@mikemajara
Copy link
Contributor Author

I've made some progress but I would like a doublecheck before I keep moving forward @francisconeves97

@francisconeves97
Copy link
Owner

Hey @mikemajara! This is an amazing contribution and idea. 🚀 Currently I don't have much time to help you on this, but I will try to when I have got the time.

Maybe an idea to get this thing started is to start by only adding one type of validation. Maybe a simple one like length validation on the short text input. This way it would be much easier for me to review your work 🙏 after you have that one I can review the PR and maybe give some ideas

After the first validation is in place we can start implementing the others using the same strategy.

What do you think of this?

@francisconeves97
Copy link
Owner

Looking at your fork I see that you already did an amazing job and lots of work, thank you for that. I would only give a suggestion, and let me know what you think:

I see that you are storing the validation function on the parsed json. I think this has some problems:

  • If we want to use the parsed json for other use case, imagine for example building a similar library for flutter, those functions won't be compatible (not saying that someone will build that library, but just to give an example 😛)
  • If some of the validation functions are buggy, which can easily be the case since we are writing a function on a string and we can mess up the syntax, we would have to rebuild the json object

In my opinion I think the json object should only contain metadata about the google form, and not code. I would maybe add a new type:

interface Validation {
  type: ValidationType
}

type LengthRule = 'MAX' | 'MIN'

interface LengthValidation extends Validation {
  type: 'LENGTH'
  rule: LengthRule
  length: number
}

interface TextField extends BaseField {
  type: 'SHORT_ANSWER' | 'LONG_ANSWER'
  options: any
  validation: LengthValidation
}

This is from the top of my head, so please feel free to change or discuss new ideas 🙏 But I think this would follow the pattern that was used for the different field types as well.

@mikemajara
Copy link
Contributor Author

I see that you are storing the validation function on the parsed json. I think this has some problems:

I agree, in general it can pose several problems and risks. We should be able to return it in an object structure within the app itself, if the form itself would be parsed within the browser. I believe this cannot be done, therefore to expand on what already existed I found this the least intrusive way.

If we want to use the parsed json for other use case, imagine for example building a similar library for flutter, those functions won't be compatible (not saying that someone will build that library, but just to give an example 😛)

You are right. At this stage however I think it'd be a negligible cost and I would focus on JS, and cross that bridge when needed.

If some of the validation functions are buggy, which can easily be the case since we are writing a function on a string and we can mess up the syntax, we would have to rebuild the json object

Again, you are right, but the functions are basic enough, and Google has validation in their forms (see image below).
image

With all that being said, we could just implement in a way that the JSON would only contain metadata, it'd certainly be a cleaner approach, but we'd need then to translate that with functions in client, any suggestions to that approach?

I can also merge to a branch feat/validation or similar in this same repo if you'd like.

@francisconeves97
Copy link
Owner

Yeah that is a nice point, maybe this would be the way to implement this with less friction considering how the code is structured right now.

With all that being said, we could just implement in a way that the JSON would only contain metadata, it'd certainly be a cleaner approach, but we'd need then to translate that with functions in client, any suggestions to that approach?

I agree with this suggestion, and after all it should be what google is doing. They have this metadata of the form, and they dynamically build the validations on the client.

Regarding alternative suggestions, I am not as familiar as you with the validation code but building on top of the types I mentioned before I developed this poc branch:

https://github.com/francisconeves97/react-google-forms-hooks/compare/validation-poc

Let me know what you think of this approach. Don't mind the types and some parts of the code, just a rough sketch of what we could do. I think it wouldn't be that much intrusive, since we would be creating a new reusable layer to build the validation functions.

Let me know what you think 🚀

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

No branches or pull requests

2 participants