@PhoenixDev22
Posted
Hello Webdevsonu,
Congratulation on finishing this challenge. Great job on this one! I have few suggestions regarding your solution:
HTML
- Never use
<div>
and<span>
alone to wrap a meaningful content. Just keep in mind that you should usually use semantic HTML in place of the div tag unless none of them (the semantic tags) really match the content to group together. By adding semantic tags to your document, you provide additional information about the document, which aids in communication.
- Page should contain
<h1>
. In this challenge, as it’s not recommended to have more than<h1>
, you may use<h1>
visually hidden with class=”sr-only”. You can find it here then you can use<h2>
instead of a generic div.
- Images must have alt attribute. 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 .
- You have used
<br>
, using<br>
is not only bad practice, it is problematic for people who navigate with the aid of screen reading technology. Screen readers may announce the presence of the element. This can be a confusing and frustrating experience for the person using the screen reader. You can read more in MDN.
- It’s not recommended to use
<br>
to increase the gap between lines of text Or make empty space between elements, usepadding / margin
styling via CSS. And about using <br> in the<p>
to make the paragraph break in new line, You may usemax-width
to<p>
and remove those<br>
.
- 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.
- 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.
line-height:22px
Use a unitless line-height value to avoid unexpected results. You can read more in mdn
- Consider using max-width to the component that wraps the three cards instead of setting to individual cards.
height: 350px;
It's not recommended to set fixed height to component, you almost never want to set it. let the content of the component define the height.
- Remember a modern css reset on every project that make all browsers display elements the same.
- 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.
Hopefully this feedback helps.
Marked as helpful