Semantic HTML5, CSS Custom Properties, Mobile-first workflow

Community feedback

• 

  Sabine•80

  @SabineEmdenPosted 1 day ago

  Hi there! 👋 Good job on completing the challenge.

  Unfortunately, I can't preview your site or view your code. I get a "page not found" error for both links, and your GitHub profile shows no public repositories.

  Happy coding! 😎

  Marked as helpful

  1

  ---

  

  Sai Suhani•20

  @saisuhaniPosted about 21 hours ago

  @SabineEmden

  Hello! 👋

  Thanks for catching that! I’ve updated the repository settings, and you should now be able to access both the preview link and the code on GitHub. Let me know if everything looks good on your end, or if you spot anything else. And please tell me how to improve my code.

  0

  

  Sabine•80

  @SabineEmdenPosted about 11 hours ago

  @saisuhani

  Yes, the links now both work.

  On first glance, your code is not bad, but I have some suggestions for improvement. I'll write you a proper code review.

  Did you know you can use Markdown in the README file? I noticed you removed all Markdown syntax from the README template that came with the project.

  Markdown is a great tool for formatting text. It also works in comments here on the Frontend Mentor platform and on Discord.

  Marked as helpful

  1

  

  Sai Suhani•20

  @saisuhaniPosted about 10 hours ago

  @SabineEmden Oh I didn't know that. I'll try it.

  0
• 

  Sabine•80

  @SabineEmdenPosted about 6 hours ago

  Here are some suggestions how you can improve your solution. I hope they help. Let me know if you have any questions!

  Responsive Design

  This project doesn’t need any media queries. You can see in the Figma design file that the width of the component is 320 pixel in both the mobile and the desktop view. The design doesn’t shift.

  • Remove the media queries from your code. You don’t need them here.
  • Convert the width in the design file to rem and use that as the max-width of the component. (See below for more on rem.)
  • Add some padding to the body, so the component doesn’t touch the edge of the screen on smaller screen sizes.
  • Change the height of the body element from height: 100vh to min-height: 100vh (or use svh). This allows the height of the body to be larger than the height of the viewport if necessary, for example on a mobile device in landscape mode.

  It’s great that you want to make you projects responsive. If you follow the learning paths on Frontend Mentor, you will get lots of opportunities to practice responsive design.

  Just for future reference:

  Let’s say a component had a width of 320 pixel on small screen sizes; on screens wider than 768 pixel, the width would change to 480 pixels. For mobile first design, you would write the CSS rules for the small screen sizes first, as you default, with no media query. You would then use a media query to add a breakpoint and change the width for wider screens.

  /* This is how the component looks on mobile by default */ 
  .component {
  	max-width: 20rem;
  }
  
  /* This changes the layout for screens wider than 48rem */
  @media (min-width: 48rem) {
  	.component {
  		max-width: 30rem;
  	}
  }
  

  The media query has to be below the default styling in the style sheet, or it won’t work. And be careful to use min-width or max-width in your media query, not width. You have @media (width: 1440px) in your code. That would work only at a width of exactly 1440 pixel.

  Semantic HTML and Web Accessibility

  The main reason why we want to use semantic HTML is to make our web pages easier to navigate for people who use assistive technology like screen readers. For this, every page needs to have a main landmark. It allows users to skip directly to the main content of the page. Another important rule is to have only one h1 heading per page. For components like this project, the h1 heading will most likely be somewhere else on the page, not on the component.

  • Wrap <div class=“box”> in a <main> element.
  • Change <div class=“attribution”> to <footer class=“attribution”>. The footer is not part of the main element.
  • Change the h1 heading to h2. This will get flagged in to accessibility validation, but it’s still the right thing to do.

  To make your project accessible, you also need to allow users to change the font size with their browser settings. That’s why we want to have all font sizes and container widths in rem, not px. For margins and padding, it’s your choice if you want them to change with the font size. I normally use px for padding and rem for margins.

  • Change all font sizes to rem, based on 16px as the default browser font size. That is, 14px converts to 14px/16 = 0.875rem.
  • Test if you want to use rem for margins by changing the font size in your browser settings.

  One more thing about HTML structure: You don’t need <div class=“qr”> and <div class=“text”>. Your code will work just the same without them.

  • Remove the two divs.

  CSS Structure and Performance

  Overall, your CSS is well-structured and readable, and I like your use of custom properties for the colors. My only complaint is that you have some CSS in the HTML file that should be moved to the style sheet.

  • Move the two CSS rules for the attribution from the HTML file to your CSS style sheet.

  A tip for positioning the attribution in the footer: With your component wrapped in a <main> element, this code snippet is very helpful:

  body {
    /* more code */
    min-height: 100svh;
    display: flex;
    flex-direction: column;
  }
  
  main {
    flex-grow: 1;
    display: flex;
    justify-content: center;
    align-items: center;
  }
  

  For performance, it’s better to link to Google Fonts in the HTML file instead of using @import in the style sheet.

  For future projects, you may want to look into using a full CSS reset like the ones by Josh Comeau or Andy Bell.

  Happy coding! 😎

  0
• 

  Divine-art-lab•10

  @Divine-art-labPosted 1 day ago

  Does the solution differ considerably from the design?

  0

  ---

  

  Sai Suhani•20

  @saisuhaniPosted about 9 hours ago

  @Divine-art-lab Umm.. What do you mean? I'm sorry I didn't understand.

  0