Design comparison
SolutionDesign
Community feedback
- @denieldenPosted almost 2 years ago
Hello Yusuf, You have done a good work! 😁
Some little tips to improve your code:
- add
maintag and wrap the card for improve the Accessibility - also you can use
articletag instead of a simpledivto the container card for improve the Accessibility imgelement must have analtattribute, it's very important!- add descriptive text in the
altattribute of the images - remove all unnecessary code, the less you write the better as well as being clearer: for example the
h3title - remove all
marginfrom.yusufclass - use flexbox to the body to center the card. Read here -> best flex guide
- after, add
min-height: 100vhto body because Flexbox aligns child items to the size of the parent container - instead of using
pxuse relative units of measurement likerem-> read here
Keep learning how to code with your amazing solutions to challenges.
Hope this help 😉 and Happy coding!
0 - add
- @joaskrPosted almost 2 years ago
Hi Yusuf! Good job with the challenge! If you want to further improve it, here are some suggestions that can help you:
Layout
- Your component could use some more padding on top and bottom and a little bit less on both sides - that would make it look more like a rectangle than a square.
- You are currently using Times New Roman as a font while the style guide suggests using Outfit. You can find this font on google fonts: Outfit and it will make the solution look more like the design
- You can use a bigger font size for both the heading and paragraph as it's quite tiny and hard to read. I believe that the suggested font size in the style guide for paragraphs was 15px.
HTML validation and accessibility
- It is important to add alt text for images as it improves the accessibility for users with screen readers. In your code it can look like this:
<img src="images/image-qr-code.png alt="qr code"> - You should wrap your content in landmark elements such as <main> <header> <footer> instead of using <div> as it also makes your code more accessible - in your code div with class "yusuf" could be a <main> tag. ``
- You should have a level one heading on your page, so consider replacing <h3 class="mentor"> with <h1>
Code
- Your CSS classes name should describe the element in the class. You are using class names such as "yusuf" or "mentor" which are not descriptive.
- You are using <br> tag to divide text. This is not considered a good practice as it is not semantic. <br /> should be used for line breaks only, and not to apply the style to a page. If you need extra space between paragraphs, you can achieve that by adding extra padding to the paragraphs or modifying the width of the paragraph. Here you can see a discussion on this topic on Stack Overflow
- Consider using flexbox to position your content on the website - it can help you achieve for example horizontal and vertical centring of content. Here's a nice guide
- You are using pixels for setting font sizes and margins. In responsive web design, it is the best practice to use rem or em units as they are more responsive. Here's a nice article on font units in CSS and on the difference between rem and em
I know I wrote a lot, but don't feel discouraged - these are small fixes that can improve your solutions. Of course, feel free to reach out if you have any questions. Keep on coding and good luck with future challenges!
0
Please log in to post a comment
Log in with GitHub
Join our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord