@ringm
Posted
Hi Meg! Great job on the form validation and the overall style of the page. It looks really good.
I downloaded the repository and took a look at the code, I have some suggestions that I think would really improve your work:
-
Try making the background color and img stretch the whole with and height of the viewport, and leave just the content constrain to the max-width of 1440px. That way you will not have a white box surrounding the site when viewed on large devices.
-
You could add some
:hoverstyles on thebuttons, if you take a closer look to the design images, you will notice there's one that has a color accent for the green button. -
I think the font inside the
inputsis not the font family provided by the project. -
The script that validates the form could be factorized with a loop, instead of making a chain of if-else statements, it will look much cleaner! Instead of selecting each
inputby it's ID, you could.querySelectorAllthediv's with the class '.input'. That way you'll get a collections of all the div's with the nested inputs, error-icons and error msg. Then with a for loop, you iterate thgough them and check if the input's are empty. If it's not very clear you can check my approach in my solution. -
Finally, I would suggest avoiding setting fixed values for widht's ang height's, you're preventing your site from being fluid and responsive!
If you need any help please feel free to ask any questions. Great work! :)
@takameg
Posted
@ringm Hi Martin, Thank you very much for checking my code in very detail!
-
For background, I have moved background to body, and it does not get white space outside anymore. I left width of container as "width" instead of "max-width", since I get big space on the left side and I can not figure it out why yet.
-
For button:hover, I have added the code to brighten the color a bit. I did not notice that instruction. Thank you for pointing it out.
-
For font-family of input, I have changed it. I had overlooked. Thank you for pointing it out again.
-
For javascript part, I might need more time to check about .querySelectorAll and for loop.
-
For fixed values, I will take note of that moving forward.
This is just a quick note to thank you m(_ _)m I will check and work on improving the javascript part soon.
@takameg
Posted
@ringm Hi Martin, I have amended javascript part, and I think it became much simpler now. I learned many new things from your advice. Thank you very much again!
@ringm
Posted
@takameg really glad to help! Looking forward to your next projects:D
