@gmagnenat
Posted
Hi, congrats on completing the challenge !
I checked your repository and live url and have a few recommendations that you can use to improve your solution.
Does the solution include semantic HTML?
- you need to wrap your main content inside a <main> landmark. The main element represents the main content of the body of a document or application. -w3c
- for increased performance you should place your title higher in the <head> just after the
<meta name="viewport".. - when using a
target="_blank"you should add a screen reader only indication that it will open in a new tab as it can be confusing for some users. But i'm not sure a clickable link is mandatory here. - you don't really need the extra div to wrap your <h2>
- the 3 spans you added in the
.bottomdiv can be just one <p> element with the full text.
Is it accessible, and what improvements could be made?
- Currently this solution isn't accessible for users who change their browser default font size.
- You should change font size to relative units to make your text scale with user preferences Why font-size must NEVER be in pixels
- You don't need the 80% width as it's only 80% of the screen size but doesn't scale with root font-size. Set a max-width instead only on the card and in relative unit (rem) so it can scale correctly.
Does the layout look good on a range of screen sizes? Yes
Is the code well-structured, readable, and reusable?
- You need to add a modern css reset at the top of your stylesheet in all your project. It will help you with browser default styles and fix some issues before you start customizing. check this one for example modern css reset
- you should focus on low specificity in your css before increasing it only when needed.
- you can add more meaningful class name instead of .middle .top .bottom. For example
.card__title,card__content. Imagine if we add 20 elements in this card, how would you name them if you are using top, middle, bottom?
Does the solution differ considerably from the design?
- The solution looks a bit bigger then the design. With a css reset it will help with proper font sizing and line-height.
I hope you find something useful in these comments to improve this solution and your future challenges. Let' me know on discord if you have any question.
Happy coding !
@lawrence500
Posted
@gmagnenat thankyou for the feedback I will use this to improve my current solution also I was not aware that using px for font size is bad so thankyou for letting me know
