Four card solution

Community feedback

• 

  CodingDuck•390

  @KaskaS-OPosted about 3 years ago

  Hi!

  Congrats on finishing the challenge :) I have some remarks, hope they'll be helpful.

  1. I see you used 'index.css' name for a stylesheet. It's better not to (index is a default name for a main html file). It can cause some problems when you're referring to a file from some script.

  2. You used h2 and h4 markups for titles. The best is to start with h1 for page's main title and to go with consecutive numbers for subtitles, sections's titles e.t.c. It's important especially for accessibility. I know sometimes it's not comfortable and I myself make exceptions from that rule, but when it's possible it's worth of doing.

  3. For practicing using Git and showing you're comfortable with it (to your future employer looking at your Github) try to use more commits. I know some projects aren't too complex and you can even finish them in short time no walking from the computer, but use the opportunity to familiarize with version control.

  4. When you check your layout with desing using Frontend Challenge design comparison you can see your components are a bit smaller and positioned closer to the top of the page. Your layout looks ok, these are details, but it always can be better, can't it? ;)

  Good luck with next challenges! You'll see it's more fun with every challenge you take :)

  Marked as helpful

  1

  ---

  

  Kristiyan•80

  @kristishhPosted almost 3 years ago

  @KaskaS-O Thank you for the notes! I edited the name of the css file and change it to "main" and did some other css changes. This really upgraded my knowledge on Github because I've never used to clone repos and merge it with the base one!

  0
• 

  darryncodes•6,390

  @darryncodesPosted about 3 years ago

  Hi Kristyan,

  Overall a really solid solution, well done!

  A few quick considerations from me:

  • You might want to think about using a 2 by 2 grid at a tablet breakpoint to stop the content blowing out of the screen. This is an excellent comprehensive guide
  • You might want to centre your design nicely in the viewport by adding the following to the body:

      display: flex;
      flex-direction: column;
      justify-content: center;
      align-items: center;
      min-height: 100vh;
  

  Marked as helpful

  0

  ---

  

  Kristiyan•80

  @kristishhPosted almost 3 years ago

  @darryncodes Thank you for the guide! I did the body thing that you said to do but then on bigger screen the images started to go in the center of the card wrapper so i did something that maybe made it look good. Look now at the live it is really better. Once again I wanna say thank you!

  0