Here is some foundational feedback to help you. Note you will need to refactor all projects with some of these things.
- All content should be contained within landmarks. This needs a
main
wrapping the component, and afooter
for the attribution. - You don't need to use the picture element in this. The picture element is used when you want to serve different images at different screen sizes. This only has one image. When you do need to use the picture element in future, you would make the mobile image the default inside the
img
element and you would use amin-width
media query in the source tag (media query defined inrem
orem
not px) to serve the larger screen image. - The Imogen this is extremely important content. That means it needs a proper description. In this case the alt should say what the image is (QR code) and where it goes to (two frontendMentor.io).
- There should be no hgroup in this.
- You don't actually need an extra div to wrap the image or the text. It's no problem having it in there but as a general rule keep the HTML as simple as possible.
- Remove the
br
from the heading. That is not how you create line brakes. Some screen readers will announce this as "break" in the middle of the heading, which is not what you want. Instead let the lines break naturally. In some cases in certain challenges if you need lines to break earlier you would give the element a max widthrem
orch
units, and margin-inline auto if that limited width element also needs horizontal centering. - Never set font-size in px. Use rem.
- Get into the habit of always including a full modern CSS reset at the start of your styles. Andy Bell or Josh Comeau both have good ones you can look up and use.
- It's better for performance to link fonts in the HTML head instead of CSS imports.
- Recommend use the flex column method to centre components like this in the viewport and not the grid place-content center method. This will prevent overflow people viewing a site on smaller screens who have a larger size. It's a more robust method.
- There should be no widths or heights in this challenge. All the card needs is a single max width in rem.
- The image must not have a width pixels. All it needs is what is included in a CSS reset anyway (Display block and Max width 100%).
- There is no benefit to using grid on this card. All it needs is a text align center. The only benefit using grid or flex on this card would be if you wanted to use the gap property instead of margin to create vertical spaces. But really that's unnecessary.
- The image doesn't need small padding and margins. Let the image be Full size. Can optionally have a margin bottom. But as you've already given margin top to the H2 even that is unnecessary.
- Your card is actually off centre at the moment. This is because the footer (attribution) is wider than the card and the card sits to the left inside it's grid area. If you use the flex column approach to center your component as I suggest above this would be solved.