@vanzasetia
Posted
Hi! 👋
The link to the site is broken. It gives me a 404 page. Try to fix the issue.
Anyway, I am going to give feedback based on the code.
- The HTML markup is nice. It's simple and has no issue. Good job! 👏
- I recommend improving the alternative text for the QR code. So, you can tell what the QR code is all about. In this case, it contains a URL (https://www.frontendmentor.io/). So, to make it clear, "QR code to frontendmentor.io" would be good.
- I don't recommend changing the
htmlor the:rootfont size. It can cause huge accessibility implications for those users with different font size or zoom requirements. I suggest reading this article by Josh Comeau where he tells about the problem of the 62.5% trick (and more!). Also, I recommend reading what an accessibility expert (Grace Snow) has said about it. - Remove
min-height: 100%;from thehtmlelement. It doesn't need it. - The CSS is nice. It has low specificity.
I would say you have done a great job on your first challenge. Also, based on the design comparison your solution looks very close to the design. 👏
I hope this helps! Keep up the good work! 👍
Marked as helpful
@vanzasetia
Posted
Also, your point is still 0 even though you've submitted a solution. @mattstuddert, it looks like there's an issue with the point system for this account.
@mattstuddert
Posted
@vanzasetia, thanks for the heads up, Vanza! We'll take a look into it 👍
@mattstuddert
Posted
@vanzasetia, OK, Vivi's points are where they should be now and we've identified the issue. Thanks again for highlighting this, Vanza.
Great work on the challenge as well, Vivi! 🙌
Marked as helpful
@vanzasetia Hi Vanza!
First of all, thank you so much for taking the time to formulate a feedback to my solution! Based on your suggestions, I did the following:
- ☑️ Fixed link to site
- ☑️ Changed alt text for the QR code
- ☑️ Deleted the 62.5% trick and changed the rem values according to one of the alternatives on the article you linked
- ☑️ Removed
min-height: 100%;
Thank you for your suggestions and for pinging Matt to fix the point issue! Thank you also for linking the article and Ms. Grace's comments 😊
@mattstuddert Thank you so much for the kind words!
@vanzasetia
Posted
@vivian-mca Amazing! Great job with the update! 👏
Sorry, I forgot to mention that you need to add padding to the body element to prevent the card from touching the edges of the browser.
Other than that, everything looks good to me! Keep it up! 👍
@vanzasetia
Posted
@mattstuddert No problem! Glad to help! 😄
