base-apparel-coming-soon with flexbox

Community feedback

• 

  P

  Remus D. Buhaianu•3,125

  @MiculinoPosted almost 3 years ago

  Good job on completing the challenge, Morden!

  I know it's difficult in the beginning, but with each project, you'll become a better developer ;)

  I had a look at your live solution and I also looked through your Github code and I have a few suggestions / observations.

  Here they are, in no particular order:

  1. Your HTML and CSS code formatting is a bit hard to read through. You might want to go through it again and polish it a bit because not only it'll make it easier for you to code, but it'll also make it easier for other developers who want to read your project's code.

  2. There are a few issues with the alignment / spacing in your layout. Even without taking the original design into account, it looks a bit off. Part of being a good frontend developer is also developing an eye for detail and good UI.

  3. There's no reason to use the min-height property on the body element.

  4. You can reset the margin and padding properties for all the elements by using the * selector. So you can add margin: 0 and padding: 0 to the CSS declaration you already have for the * selector.

  5. There might be too many media queries there. I think your code can be simplified so you don't have to change these properties in so many media queries.

  All in all, you've done a good job on this challenge and as Aakash pointed out, you did well by adding the custom message for the input element. That kind of attention to detail is what makes you a better developer and improves the overall quality of your projects.

  Keep up the good work and looking forward to seeing more of your work here on Frontend Mentor! ;)

  Marked as helpful

  1

  ---

  

  MordenWebDev•500

  @MordenWebDevPosted almost 3 years ago

  @Remus432 I have redone it today. please leave some review about code and this project.

  0
• 

  Aakash Verma•6,710

  @skyv26Posted almost 3 years ago

  Hi! I saw your work, I noticed issues that are written below:

  1. input field padding more than required, I mean just look at your field, your placeholder is in center and it look little bit bad.

  2. You haven't change the image in mobile view as per requirement, So i would suggest you to do work according to given requirement.

  You did good work by adding custom validation with custom message. I hope you understand above 2 issues.

  Good Luck

  Marked as helpful

  1

  ---

  

  MordenWebDev•500

  @MordenWebDevPosted almost 3 years ago

  @skyv26 redone it pls leave any comment

  1

  

  Aakash Verma•6,710

  @skyv26Posted almost 3 years ago

  @MordenWebDev Hi! I just checked again, I appreciate your efforts, but again you left some issues.

  1. your custom error message is really away from the input box, try to reduce the gap, so that it seems perfect and likely be part of input.

  2. Why you have used mobile image as a background to .heroImage class div and you added again an img tag with desktop image inside this div.

  You did it really wrong. You need to fix it.

  I think you are confused. I could help you. Join Slack and message me there.

  Marked as helpful

  0

  

  MordenWebDev•500

  @MordenWebDevPosted almost 3 years ago

  @skyv26 I tried to fix it. I adjusted some things as you have mentioned. modified for the hero image. pls leave a review about it. and thankyou.

  0