Responsive Car Cards using HTML and CSS (flex)

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted almost 3 years ago

  Hello @d4wk0m ,

  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)

  • 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 case, all images are decorative only.

  • no need for the <span> in the <a>.

  • use text-transform: uppercase; to the <h2>, to match the design.

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

  .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 widths in percentage. Not a great idea as you're losing control of the layout.

  • consider using max-width to the container that wraps the three card. That will let the component grow up to a point and be limited.

  • Remove the height:500px. you almost never want to set height let the content define the height.

  • line heights should be unit less .

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

  • Basically, you probably shouldn't use vh/vw for font sizes. You should use other percentage-based units like em, rem or %. Reserve viewport-width & viewport-height for large scale layout needs like column sizes or footers. use rem for font-size

  • A general point - try to put classes directly on anything you want to style. Then you won't need as many nested css selectors.

  • you can use flexbox properties and min-height: 100vh to body to center the component on the middle of the page.

  Hopefully this feedback helps.

  0