@vanzasetia
Posted
Hi, John! 👋
Congratulations on completing your first Frontend Mentor challenge! 🎉
You've gotten some awesome feedback especially on solving the issues that have been reported. I have some feedback as well.
- The alternative text of the QR code is not giving enough information. If you try a screenreader and then close your eyes, then the screenreader pronounces the image as "Frontend Mentor, image.". Then, what do you think the screenreader users would think about the image?
- Remove all
br
elements from the paragraph. Let lines wrap where they need to. Screen readers will read outbr
elements. You can't accommodate every screen size, so it's rare you'll ever need to use them. - Remove
width
andheight
from thebody
. Never use100vw
on anything as it doesn't account for scrollbars when present. It may only ever introduce potential overflow/scroll bugs.
That's it! I hope this helps. 😊
Marked as helpful
@HeavyMetalCoffee
Posted
@vanzasetia Thank you for your feedback!! Excellent, I didn't even stop to think about the little information I provided for the alternate text for the QR code. I always understood the less, most descriptive phrase as possible. I believe I have taken that just a bit too far...lol. As for the <br> tags, I did not know this, which means I need to align my paragraphs better. You make a great point about 100vw. The last challenge I did, the 3-Column preview, I ran into such an issue. Thank you again!!
@vanzasetia
Posted
@HeavyMetalCoffee You're welcome! 😉
So, what do you think is a better alternative text for the QR code?
@HeavyMetalCoffee
Posted
@vanzasetia A descriptive text such as "QR code to visit Frontend Mentor".
@vanzasetia
Posted
@HeavyMetalCoffee That's a great alternative text! So, why don't you update the alternative text on the live site? Also, if possible fix all the issues.
Improving your code or fixing bugs is one of the skills that you will need as a developer. So, it's better to get used to it. 🙂
@HeavyMetalCoffee
Posted
@vanzasetia I just updated and uploaded!! Thank you, I appreciate the feedback. Check it out!!
@vanzasetia
Posted
@HeavyMetalCoffee I took a look at the updated solution. One thing, that I would like to suggest is to swap the header
with div
instead. header
is a landmark element that is used to wrap certain page content like navigation, logo, etc.
Other than that, everything looks good to me! 🙌