Submitted over 1 year ago

HTML CSS

Rodrigo Oliveira Silva•10

@RodrigooliveiraBRPR

Preview site View code

This is a solution for...QR code component

HTML
CSS

1newbie

Design comparison

SolutionDesign

Community feedback

Ecem Gokdogan•9,380

@ecemgoPosted over 1 year ago

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

HTML

The html structure should be like that:

<body>
  <main class="card">
   <img src="images/image-qr-code.png" alt="image-qr-code">
    <h1>Improve your front-end skills by building projects</h1>
    <p>Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p>
  </main>
   <footer class="atribuition"Challenge by <a href="https://www.frontendmentor.io?ref=challenge" target="_blank">Frontend Mentor</a>. 
    Coded by <a href="https://github.com/RodrigooliveiraBRPR">Rodrigo De Oliveira Silva</a>.
  </div>
</body>

If you want to use the recommended font-family for this project, you can add the following between the <head> tags in HTML file:

<link rel="preconnect" href="https://fonts.googleapis.com">
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link href="https://fonts.googleapis.com/css2?family=Outfit:wght@400;700&display=swap" rel="stylesheet">

CSS

After adding them to the HTML, you can add this font-family to the body
If you want to make the card centered both horizontally and vertically, you'd better add flexbox and min-height: 100vh to the body

body {
  background-color: hsl(212, 45%, 89%);
  min-height: 100vh;
  align-items: center;
  font-size: 15px;
  /* font-family: "Marck Script", cursiva; */
  /* font-family: "Outfit", sem serifa; */
  /* display: flexbox; */
  display: flex;
  flex-direction: column;
  justify-content: center;
  font-family: "Outfit", sans-serif;
}

If you use max-width, the card will be responsive

.card {
  max-width: 300px;
}

In addition to that above, in order to make the card responsive and the image positioned completely on the card, you'd better add width: 100% to the img

img {
  /* width: 96%; */
  width: 100%;
  border-radius: 20px;
}

You'd better update texts in this way:

h1 {
  color: hsl(218, 44%, 22%);
  margin: 20px 0;
  font-size: 20px;
  line-height: 1.3;
}

p {
  color: hsl(220, 15%, 55%);
  margin-bottom: 20px;
  line-height: 1.3;
}

You don't need to define .container and text and you can remove it

/* .container {
  max-width: 350px;
  margin: 0 auto;
} */

/* text {
  padding: 22px 10px;
} */

Finally, if you follow the steps above, the solution will be responsive.

Hope I am helpful. :)

Abdul Khaliq 🚀•72,560
@0xabdulkhaliqPosted over 1 year ago
Hello there 👋. Congratulations on successfully completing the challenge! 🎉

I have other recommendations regarding your code that I believe will be of great interest to you.

HTML 🏷️:

This solution may cause accessibility errors due to lack of semantic markup, which causes lacking of landmark for a webpage and allows accessibility issues to screen readers, due to accessibility errors our website may not reach its intended audience, face legal consequences, and have poor search engine rankings, highlighting the importance of ensuring accessibility and avoiding errors.

What is meant by landmark ?, They used to define major sections of your page instead of relying on generic elements like <div> or <span>. They are use to provide a more precise detail of the structure of our webpage to the browser or screen readers

For example:

The <main> element should include all content directly related to the page's main idea, so there should only be one per page

The <footer> typically contains information about the author of the section, copyright data or links to related documents.

So resolve the issue by replacing the <div class="container"> element with the proper semantic element <main> in your index.html file to improve accessibility and organization of your page

.

I hope you find this helpful 😄 Above all, the solution you submitted is great !

Happy coding!
0
IryDev•1,580
@IryDevPosted over 1 year ago
Hey, well done for completed your first challenge on the platform😁

I have some suggestions to help you to improve your solution :

Put the main content of your website into the <main>tag in order to improve accessibility

if you want to correctly center your card :

Give the body a min-height of 100vh

Add the property , display: flex; justify-content: center; align-items: center; to make the card center on the vertical axis and horizontal axis

HTML :

<body> <main> The main content of your website </main> </body>

CSS :

body { min-height: 100vh; display: flex; align-items: center; justify-content: center; }

I hope you'll find this helpful, and your solution is really good 😄
0

Please log in to post a comment

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