Hello! Well done at diving into your first challenge. there are some important learnings from this first challenge that will set you up well for future. I hope this feedback is helpful.
- The stylesheet link goes in the head not body.
- Remove the header landmark. This challenge had no header and you must never leave empty landmark elements on the page. These are very important elements for accessibility (assistive tech users use landmarks and headings to understand the contents of a page and navigate within it).
- This is missing a main landmark. Every page must always have a
main
. Change container to be a main not div. (NB: This is a single component challenge so these landmarks are not strictly needed but it's best to get into the habit of adding it straight away so you don't forget later). - The image alt description isn't quite right. Think about the context of how this card component would be used on a real site. It's likely there could be several cards like this on a page so the image alt must give an accurate description of this specivific image. It should say what the image is (QR code) and where it goes (to FrontendMentor.io). There is a read post in the resources channel on discord about how to write good alt text if you want to know more.
- It's really valuable to make sure you Indent your code consistently so it's easier to read and spot bugs. Your code editor can even do this formatting automatically for you with prettier.
- The starter code came with a gitignore file. Do not remove that from your repo! It stops files like DS STORE which should not be there from being tracked by git.
- It is better for performance to link fonts in the html head instead of css imports.
- 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.
- Justify content center is missing from the body in css.
- The components max width should be in rem not px. This is important because many people change their default text size. If you set a max width in px that is an explicit limit but would look awful and narrow for those users with larger text. If you use rem instead the layout will scale correctly for all users.
- I think you may be a little confused between padding vs margin. I wouldn't expect a heading to have padding as its not a box, it's a text element and has no concept of needing "internal" space.
- ⚠️ Font size must never be in px
- There is no need to use position fixed or absolute on the footer. This is causing a horrible bug where the footer overlaps the card because you're taking the footer out of its normal place in the document.
- I think your heading text size is too large in comparison to the design. Try to adjust that.
Marked as helpful
@tybaglue
Posted
@grace-snow Thanks for the comments, i've tried to change the code according to ur suggestions.