Resposive mobile first, CSS Grid

Community feedback

• 

  Martin•1,150

  @ringmPosted over 4 years ago

  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 :hover styles on the buttons, 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 inputs is 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 input by it's ID, you could .querySelectorAll the div'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! :)

  1

  ---

  

  M3gumi•385

  @takamegPosted over 4 years ago

  @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.

  0

  

  M3gumi•385

  @takamegPosted over 4 years ago

  @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!

  1

  

  Martin•1,150

  @ringmPosted over 4 years ago

  @takameg really glad to help! Looking forward to your next projects:D

  1
• 

  Mensur Duraković•195

  @mensur-durakovicPosted over 4 years ago

  On blue box, text goes in 2 rows, but it should not. Maybe you should give it a full width. Othwerise, very nice work. :)

  1

  ---

  

  M3gumi•385

  @takamegPosted over 4 years ago

  @mensur-durakovic (re-post, since accidentally deleted) Thank you for pointing it out. Fixed blue box, and I think it looks okay now. The screenshot sometimes shows a bit different from what I see in the actual browser(like <p> on the left side also shows a bit diff right not). Wondering if it is also a kind of browser dependent issue..?

  0