3-column preview card component

Community feedback

• 

  Shashree Samuel•9,260

  @shashreesamuelPosted over 2 years ago

  Good job with replication of the design, keep up the good work.

  Cheers, Happy coding 👍

  Marked as helpful

  0
• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hello @jay-kamble ,

  I have some suggestions regarding your solution:

  • 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 didand 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 min-height: 100vh instead of height: 100vh allows the body to to grow taller if the content outgrows the visible page.*/

    • width:100vw; If you set a page width, choose 100% over 100vw to avoid surprise horizontal scrollbars.

  .card {
      display: flex;
      /* margin: auto; */ remove the margin 
      max-width: 832px; /*an explicit width is not a good way   consider using max-width instead of width . */
      text-align: left;
  /*border-radius and overflow hidden to the main container that wraps the three  cards so you don't have to set it to individual corners*/
       overflow: hidden;
        border-raius: 10px; 
  }
  

  • In line-height: 2.5rem, use unitless value for the line-height , this is the preferred way to set line-height and avoid unexpected results due to inheritance.Read more in MDN.

  • It's recommended to use em and rem units .Both em and rem are flexible, Using px won't allow the user to control the font size based on their needs.

  Hopefully this feedback helps.

  Marked as helpful

  0
• 

  Abby Nyakara•190

  @AbbyNyakaraPosted over 2 years ago

  The solution is nicely executed. However, just a few things:

  • If you have a css style sheet, you could just transfer all the styling there. I see you left the attribution section in the html file.

  Marked as helpful

  0