3-column preview card component

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hello @aladin002dz ,

  I have some suggestions regarding your solution:

  • There should be two landmark components as children of the body element - a main (which will be the component ) and a footer (which will be the attribution).<Footer> should not be in the <main >. HTML5 landmark elements are used to improve navigation .

  • 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 <h3> 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="" and 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.

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

  • the content is hiting the screen edges : giving padding to the body or margin to the container that wraps the three cards prevent the content from hitting screen edges.

  • using min-height: 100vh instead of height: 100vh allows the body to to grow taller if the content outgrows the visible page.

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

  General point : Really important to keep css specificity as low/flat as possible. The best way to do styling is single class selectors.

  Hopefully this feedback helps.

  Marked as helpful

  1

  ---

  

  Mahfoudh Arous•315

  @aladin002dzPosted over 2 years ago

  @PhoenixDev22 thanks a lot for the excellent feedback 🙏, I learned a lot from it, very helpful! I didn't implement all your brilliant recommendations, except the one of h1 I am studying it. I can't thank you enough for this informative and rich feedback 😊🙏🙏🙏

  0
• 

  A1ex•500

  @a1excpunkPosted over 2 years ago

  good static component, but responsiveness is ... well not even a trend but a necessity. also, there are some small details, like

  • anchor tag instead of a button tag (for some it might be a preference thing, but when it's a button it should be in the button tag)
  • button background colour

  1

  ---

  

  Mahfoudh Arous•315

  @aladin002dzPosted over 2 years ago

  @a1excpunk thanks a lot for your feedback 🙏! but I did implement the responsiveness, I couldn't share a screenshot here, on ./sass/style.scss line 115: @media (max-width: 768px) {. for the anchor tag, I didn't want to wrap the link in another HTML element, to reduce code. thanks again for your feedback 😊🙏

  1

  

  A1ex•500

  @a1excpunkPosted over 2 years ago

  @aladin002dz maybe less code but sometimes it's not that critical, especially with 3 extra tags. but it's up to you at this point. :) now responsiveness works, I guess it was some glitch. :D you are welcome and keep going. :)

  1