Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

3-column preview card component challenge

@info1922

Desktop design screenshot for the 3-column preview card component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


Hi developers My four challenge, any feedback would be appreciated :)

Community feedback

PhoenixDev22 16,950

@PhoenixDev22

Posted

Hello @info1922 ,

  • For any decorative images, each img tag should have emptyalt=""and aria-hidden="true" attributes to make all web assistive technologies such as screen reader ignore those images. In this case, all images are decorative only.

  • swap the buttons for anchor tags. Clicking those "learn more" buttons would trigger navigation not do an action so button elements would not be right. And for future, it is essential if you include a button in a form element without specifying it's just a regular button, it defaults to a submit button., though, so it's a good idea to make a habit of specifying the type.

  • you can add a <h1> with class="sr-only" (Hidden visually, but present for assistive tech). and use h2 instaed h1.

.sr-only {
	border: 0 !important;
	clip: rect(1px, 1px, 1px, 1px) !important;
	-webkit-clip-path: inset(50%) !important;
		clip-path: inset(50%) !important; 
	height: 1px !important;
	margin: -1px !important;
	overflow: hidden !important;
	padding: 0 !important;
	position: absolute !important;
	width: 1px !important;
	white-space: nowrap !important;            
}

  • using vh (viewport height) units to allow the body to set a minimum height value based upon the full height of the viewport.This also allows the body to to grow taller if the content outgrows the visible page.

body { min-height: 100vh; }

  • border-radius and overflow hidden to the container that wraps the three cards.so you don't have to set it to individual corners.

  • Never use px for font-size.

  • using widths in percentage. Not a great idea as you're losing control of the layout

  • You can use flexbox properties and min-height: 100vh; to the body to center the component on the middle of the page.(no need for padding ).

  • an explicit width is not a good way . Remove the width from the main component and change it to max width instead. That will let it shrink a little when it needs to.

Overall , your solution is good . Hopefully this feedback helps

Marked as helpful

1

@info1922

Posted

Hi @PhoenixDev22 Thank you very much for the feedback, it helps me to continue improving.

1

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord