@PhoenixDev22
Posted
Hi Cassia Moraes,
Congratulation on completing anther frontend mentor challenge. I have some suggestions regarding your solution:
- You should use
<footer>
landmark for the attribution , it should live outside the the <main> as landmarks allow screen reader users to navigate through sections of your website by skipping to content that interests them. Landmarks could be seen as the logical layout of the website's UI, which is divided into e.g. header, navigation, main content, and footer. So the usage makes sense in any case.
- Page should contain
<h1>
. The<h1>
is most commonly used to mark up a web page title. This challenge is supposed to be one component in a web page. To tackle the accessibility issue in the report , you may use an<h1>
visually hidden withclass=âsr-onlyâ.
You can find it here.
- In this challenge, the images are much likely to be decorative. For any decorative images, each img tag should have
aria-hidden="true"
attribute to make all web assistive technologies such as screen reader ignore those images .
- In this challenge, what would happen when the user click those learn more? In my opinion, clicking those "learn more" would likely trigger navigation not do an action so button elements would not be right. So you should use the
<a
. For future use , it's a good habit of specifying the type of the button to avoid any unpredictable bugs.
- It's not recommended to capitalize in html, let css text transform take care of that. Remember screen readers won't be able to Read capitalized text as they will often read them letter by letter thinking they are acronyms.
- Adding
rel="noopener"
orrel="noreferrer"
totarget="_blank"
links. When you link to a page on another site usingtarget=â_blankâ
attribute, you can expose your site to performance and security issues.
- Add
border-radius
andoverflow hidden
to the main container that wraps the three cards so you don't have to setborder-radius
to individual corners.
- width: 375px;an explicit width is not a good way to have responsive layout . Consider using max-width to the card in rem.
- Donât Repeat Your CSS(DRY) is a good general principle to follow and eliminating duplication of css code should naturally be part of coding journey.
- Consider using rem for font size , it' not recommended to use px for font size as absolute units donât scale for example 15px will always be 15px on the same device. Using pixels is a particularly bad practice for font sizing because it can create some accessibility problems for users with vision impairments.
- Remember a modern css reset on every project that make all browsers display elements the same.
Aside these, your solution looks great. Hopefully this feedback helps.
Marked as helpful
@cassiality
Posted
@PhoenixDev22
Hello PhoenixDev22. Thanks for the great tips, it's really new information that I didn't know. It's definitely adding a lot of knowledge.
I will redo with the suggested changes!!