3 column preview card component

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hello @Wissemh ,

  I have some suggestions regarding your solution:

  HTML

  • About<h1>it is recommended not to have more than one h1 on the page . You can add a <h1> with class="sr-only" (Hidden visually, but present for assistive tech). THen swap those <h1> by<h2>.

  .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;            
  }
  
  

  • For any decorative images, each img tag should have empty alt="" as you did and add aria-hidden="true" attributes to make all web assistive technologies such as screen reader ignore those images in this challenge , all the images are decorative.

  • 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 . So use <a> instead.

  • Using <section> tag for each card is not really a good choice . Section is not meant to be used anytime you feel tempted to use a div . section is for a bigger chunk of content often titled by <h2> Read more about usage notes

  • border-radius and overflow hidden to the container that wraps the three cards, to get the rounded corners.

  • width: 80%using widths in percentage. Not a great idea as you're losing control of the layout. Use max-width instead, let it grow to a point. Then a little margin on the component or padding on the body to stop it hitting screen edges.

  • height: 30em;It's rarely ever a good practice to set heights on elements . Let the content inside the card element dictate the height of it.

  • Remove overflow: hidden from the body as . It hides real content potentially, and will eventually create problems with readability and responsivity, it allows for images and text leaving the viewport halfways and is in general bad for accessibility. It won't adapt well on tiny screens.

  • It's recommended not to use px for font size

  • 375px is very late to start the mobile layout. Start it as soon as there is room for the layout.

  hopefully this feedback helps.

  Marked as helpful

  0
• 

  Naveen Gumaste•10,480

  @NaveenGumastePosted over 2 years ago

  Hello Wissem ! Congo 👏 on completing this challenge

  • Just one thing this challenge maybe tough for you, if you know code, then just redo this whole challenge and refer the design and all of its freature

  happy Coding😀

  0

  ---

  

  Wissem•180

  @WissemhPosted over 2 years ago

  @Crazimonk Hello Naveen I am sorry but I don't understand why should I redo it ? What's wrong with it ? Thanks

  0

  

  Naveen Gumaste•10,480

  @NaveenGumastePosted over 2 years ago

  @Wissemh Ok oh This is my solution have a look =>my Solution I think you understand now

  0