Semantic HTML5 markup, CSS custom properties, Flexbox

Community feedback

• 

  P

  clickglue•460

  @clickgluePosted about 2 months ago

  Wauw if this is your first project, you've done an excellent job and there will be great things coming! Your style of coding is clean and precise. You keep things simple and well structured, very important. Three tips that you might find usefull: the more complicated the projects become, the important it will become to plan your work in steps. Maybe you already doing that, but it might help. Secondly: many good websites use a CSS reset, to make sure the design shows up correctly. Het is a link. Thirdly, you might consider looking into CSS variables. This helps you making consistent layouts, also in complex projects. Good luck, you will be great!

  0
• 

  T

  Grace•31,270

  @grace-snowPosted about 2 months ago

  The html is pretty good in this, well done! The only improvement to suggest there is to treat this card more like a card component you would build in a real site — that means consider the context of how an where it would be used. A card like this would never be used to serve the main heading on a page, so you know it shouldn't have a h1. Use a lower importance heading level like h2 instead. Also, the alt text on the image would be more specific to the card where it's used. So instead of just "QR code", this should also say where that code goes ("to FrontendMentor.io").

  Last thing in html - make sure you Indent the code consistently so it's easier to read and spot bugs. Your code editor can even do this formatting automatically for you with prettier.

  0

  ---

  

  T

  Grace•31,270

  @grace-snowPosted about 2 months ago

  In css there are a few more important issues to refactor.

  • This solution currently overflows my screen at the top so there's some content cut off that I can't reach. That's caused by using position absolute. Don't use that technique to center the component on the screen. Instead make the body min-height 100svh and a flex column, using flex related properties to center the contents.
  • get into the habit of including a full modern css reset at the start of the styles in every project. Andy Bell or Josh Comeau both have good ones you can look up and use.
  • this card mustn't have a height. That's important to understand that you shouldn't limit the height of elements that contain text. Let the browser do it's job and decide what height is needed based on the content and spacings inside. As soon as you limit the height the card has the potential to break, like if an author changes the amount of text in there or a user changes their default text size or translates the site into another language.
  • the card shouldn't have a width either. Give it a max width in rem instead. That way, it can shrink narrower if it ever needs to like on a small screen.
  • there's no benefit to making the card a flex column unless you're planning on using the gap property. Block elements stack vertically by default so there's no need.
  • this card has no interactive elements, is not clickable, so it shouldn't have a hover style.
  • make sure you understand the difference between padding and margin. The card should have padding but the children within it have vertical margins.
  • the image doesn't need a width or height in this. It can have a width 100% if you like, and aspect ratio 1. But it mustn't have an explicit width or it won't be able to adapt to the size of the card.
  • Font size shouldn't ever be in pixels. Use rem instead.

  Marked as helpful

  0