3-column preview card component challenge hub

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted almost 3 years ago

  Hello Ganesh-Rathor ,

  I have some suggestions regarding your solution:

  Body

  • Wrap the body content <main >.

  • Within body sits main and footer.

  • The footer element would hold the FM attribution

  • Whenever you include interactive elements(links , buttons , inputs , textarea ), make sure you include clearly visible focus-visible styles .

  • For any decorative images, each imgtag should have empty alt="" 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 .

  • you can add a <h1> with class="sr-only" (Hidden visually, but present for assistive tech). AND use <h2> instead of <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;            
  }
  
  

  This fairly modern technique will hide or clip content that does not fit into a 1-pixel visible area. Like off-screen content, it will be visually hidden but still readable by modern screen readers.

  • No need for all those divs simply you need a container to wrap the three cards . And in each card <img> <h2> <p> <a>.

  • 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.

  • instead of giving border-radius to the two cards, I would suggest to give overflow:hidden; and border-radius to the container that wraps all the cards .

  • You should use em and rem units .Both em and rem are flexible, scalable units which are translated by the browser into pixel values, depending on the font size settings in your design. Using px will not allow the users to control the size of the page based on their needs.

  • capitalise in html, let css text transform take care of that. Remember screen readers won't be able to Read capitalised text as they will often read them letter by letter thinking they are acronyms

  Hopefully this feedback helps.

  Marked as helpful

  2
• 

  Rio Cantre•9,690

  @RioCantrePosted almost 3 years ago

  Hello there!Good job in making this far. As for the design you did good and for the issues I would suggest the following...

  • For the margin, it's already removed once you put the value into 0, but I would say use margin: auto;
  • To make the responsive design for mobile you can try this...

  @media only screen and (max-width: 600px) {
      .Preview-container {
            display: block;
            margin: auto;
            justify-content: center;
            align-items: center;
  }
  
  

  • Add hover state on each button. You can do it like this...

  .btn-2 {
    color: hsl(184, 100%, 22%);
    font-size: 15px;
  }
  .btn-2:hover {
    color: white;
    background: hsl(184, 100%, 22%);
    border: 2px solid white;
  }
  

  I think from here you just need to adjust the properties. Keep up the good work!

  Marked as helpful

  0
• 

  Travolgi 🍕•31,400

  @denieldenPosted almost 3 years ago

  Hi Ganesh, good job!

  To set margin: 0 to all elements you can use the css selector *:

  * {
       margin: 0;
  }
  

  or you can use the css reset: Read here

  Remove the height property from Preview-container class for fix the height cards.

  Hope this help :)

  Marked as helpful

  0
• 

  Ganesh•280

  @Ganesh-RathorPosted almost 3 years ago

  thanks to all for there knowledgeable feedback on my project. But I have one question on frontend review on my live site https://ganesh-rathor.github.io/challange-coloum-component/ the 3-coloumn-component-image was looking nicer and there is no less margin-bottom of button . But in the frontend review page it show much more margin on bottom of batton how it is happened

  0