Responsive HTML-CSS Design

Community feedback

• 

  P

  visualdennis•8,375

  @visualdennissPosted over 1 year ago

  Hello Batuhan,

  nice work with the card. Few more suggestions:

  • you don't need height: 1.5em; and height: auto; , in fact, height: auto is the default. Also you should avoid giving fixed-heights anw.

  • max-width: 300px; or 285px seems a bit closer to the design. Also there seems to be a bit too much space above the h2 title, so you might consider reduce the margin top.

  • Also consider using a common css reset at the start of your projects: https://www.joshwcomeau.com/css/custom-css-reset/ It can help a lot with many issues

  Hope you find this feedback helpful!🤗

  Marked as helpful

  0

  ---

  

  BatuhanGQskt•70

  @BatuhanGQsktPosted over 1 year ago

  @visualdenniss thank you for your review. I actually don't like fixed-heights and widths and trying to avoid them, but I missed this one. Thank you for pointing that out. I have removed the fixed-height, and thank you for the link.

  1
• 

  Ecem Gokdogan•9,380

  @ecemgoPosted over 1 year ago

  Some recommendations regarding your code that could be of interest to you.

  In order to fix the accessibility issues:

  • You need to replace <div id = "card"> with the <main> tag and <div class="attribution"> with the <footer> tag. You'd better use Semantic HTML, and you can also reach more information about it from Using Semantic HTML Tags Correctly.
  • Each main content needs to start with an h1 element. Your accessibility report states page should contain a level-one heading. So, you need to use a <h1> element in the <main> tag instead of using <h2>. You can replace your <h2>Improve your front-end skills by building projects</h2> element with the <h1>Improve your front-end skills by building projects</h1> element.

  Hope I am helpful. :)

  Marked as helpful

  0

  ---

  

  BatuhanGQskt•70

  @BatuhanGQsktPosted over 1 year ago

  @ecemgo thank you for your review. It is very helpful, and I applied the changes on my local computer. As you can tell I am kinda new to web development. After changing <div id="card"> to <main id="card">, I had the issue that my CSS style for <main> and <... id = "card"> broke the model of the website. I come up with a solution replacing codes from #card { ... } to main { ... } and main { ... } to body { ... }. However, I am not sure that will be the best practice. If you can tell me or provide me with any website for those kinds of practices, I would appreciate it. Thank you for all the help, and sources you have provided.

  1

  

  Ecem Gokdogan•9,380

  @ecemgoPosted over 1 year ago

  @BatuhanGQskt

  I've updated your code and uploaded to WeTransfer, you can reach your updated code from here and download your code

  I only added <footer> tag in your html and removed main class from your CSS file. Finally, updated your body class in your CSS file as well.

  In this way, you can examine your code more easily.

  However, I've forgotten to change your <h2> tag in your html file, so you can change it like this: <h1>Improve your front-end skills by building projects</h1>

  1

  

  BatuhanGQskt•70

  @BatuhanGQsktPosted over 1 year ago

  @ecemgo thank you for spending your time. I have changed the code with a slightly different approach but looked at your solution as well. Thank you so much hope you have a great coding.

  1

  

  Ecem Gokdogan•9,380

  @ecemgoPosted over 1 year ago

  @BatuhanGQskt I'm happy to help! 🎉

  1