Hi @Isaac-Novaes great work on this one - love the fancy animations! š I'm no JS expert but here's a few suggestions to consider.
-
I would suggest setting the input type to 'email' rather than text, then adding the 'no-validate' class with javascript - this means you can take advantage of built in validation and if JS fails you will still have the browser validation to fall back on.
-
With this change, you can simply use
form.checkValidity()to see if the input is valid or not rather than checking all the different use cases you have in your code at the moment. -
Alternatively, if you are not keen on that, you could use a simple regex to check for valid emails, again this would make things simpler. (I do this as the inbuilt validation isn't great, but it's better than nothing).
-
Lastly, I would try and put as much CSS styles in am
errorclass and add/remove that in your JS rather than setting all the styles etc this is really just to make things simpler and easier to change (e.g. if you want to chance thebox shadowyou can do this in CSS rather than JS.
In case it's any help, my attempt at writing some JS to deal with this is this is here: https://github.com/dwhenson/frontend-mentor/blob/master/28.%20fylo-landing-page/js/main.js
Let me know if anything is not clear - this pattern comes up again and again in FEM challenges so it's worth spending a bit of time to come up with a solution that works for you.
Cheers Dave
Marked as helpful
@isaacnovaes
Posted
Hey there @dwhenson
Thank you so much for your feedback! Thanks to it, I could refactor and improve my JS file as well as study a little more about form validation.
Well, for your considerations:
-
I agree with setting the input type to "email". To have only my JS code acting on the form validation, I set the boolean form attribute
novalidate -
I want to check only two cases. First, if the email input field is empty. Second, if the email is valid. This way I can provide an informative message for the user on what is wrong. By doing that, I think that the UX is enhanced
-
Yeap, I also did it. It makes everything simpler š
-
Well, since I am changing only one CSS property, I would say that the simplest thing would be to simply change it via JS (one line of code), instead of creating a class on my SASS file, then code my JS file to add this class to an HTML element. Do you agree with me?
BTW, when I tried to check your solution for this challenge, I get this error:
404: NOT_FOUND Code: DEPLOYMENT_NOT_FOUND ID: arn1::44w5p-1629712335845-1467a91fd197)
It would be nice to take a look at it, mate
Once again, I appreciate your effort in feedbacking my code
@Isaac-Novaes - I had all my projects in one repo, so Vercal would only let me keep a few up at a time, the codes's here on github though and it's not too complex: https://github.com/dwhenson/frontend-mentor/tree/master/18.%20base-apparel-coming-soon
On point two, ah, yes, I think I get you here. Providing a more specific error message is nice. And on four, yes, actually when I check back at my code I'm doing it this way too!
Cheers Dave
