Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

Responsive HTML-CSS Design

BatuhanGQsktβ€’ 70

@BatuhanGQskt

Desktop design screenshot for the QR code component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Community feedback

P
visualdennisβ€’ 8,375

@visualdenniss

Posted

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

@BatuhanGQskt

Posted

@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

@ecemgo

Posted

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

@BatuhanGQskt

Posted

@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

@ecemgo

Posted

@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

@BatuhanGQskt

Posted

@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

@ecemgo

Posted

@BatuhanGQskt I'm happy to help! πŸŽ‰

1

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord