PhoenixDev22• 16,950
@PhoenixDev22
Posted
Hi genos-ux,
Congratulation on completing this challenge. Excellent work! I have some suggestions regarding your solution if you don’t mind:
- About
<h1>it is recommended not to have more than one h1 on the page. Multiple<h1>tags make using screen readers more difficult, decreasing your site’s accessibility. In this challenge , as it’s not a whole page, you can have<h1>visually hidden withsr-only. Then you can swap those<h1>with<h2>
- In my opinion, the images are much likely to be decorative. For any decorative images, each img tag should have empty
alt=""andaria-hidden="true"attributes to make all web assistive technologies such as screen reader ignore those images.
- You don’t need to wrap the
<img>and<a>by<p>.
- Really important to keep css specificity as low/flat as possible. It’s not recommended to use the ids to target the DOM elements for styling purposes, better to use classes so that it could be more manageable and reusable.IDs have a much higher specificity than classes) IDs have many uses in a webpage aside from being a CSS selector. For example as page anchors, fragment identifiers or to link labels to form fields.
line-height: 40pxUse a unitless line-height value to Avoid unexpected results. You can read more in mdn
- In order to center the card on the middle of the page , you can use the flex or grid properties and
min-height: 100vhto the ``<body>`. Add a little padding to the body that way it stops the component from hitting the edges of the browser.
- Then you can use flexbox properties to the container that wraps the three card and give it
flex-direction: rowfor the desktop and column for the mobile.
- If you make each column into a flex column. Then set everything inside the cards to have some margin in one direction
marin-bottom: ;only the link wouldn't need it and usemargin-top:autoon thelearn morelink that will push it to the bottom of the cards.
- Don't 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.
Hopefully this feedback helps.
0
