Finally did it.

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted almost 3 years ago

  Hello @Prabhat-kumar-1976,

  I have few suggestions :

  • 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 are all decorative).

  • 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.Anchor tags are for navigation . buttons are for actions(submitting a form or toggling a content ).

  • 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; /* 1 */
  	-webkit-clip-path: inset(50%) !important;
  		clip-path: inset(50%) !important;  /* 2 */
  	height: 1px !important;
  	margin: -1px !important;
  	overflow: hidden !important;
  	padding: 0 !important;
  	position: absolute !important;
  	width: 1px !important;
  	white-space: nowrap !important;            /* 3 */
  }
  

  • It's better to use rem and em , don’t use px for width, height, font-size, margin ….

  • Don't use <span> for meaningful content.So you can use <p>

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

  • A general point - try to put classes directly on anything you want to style.Don't use nesting css selectors. Really important to keep css specificity as low/flat as possible.

  Hopefully this feedback helps.

  Marked as helpful

  1

  ---

  

  Aayush Raj•120

  @Prabhat-kumar-1976Posted almost 3 years ago

  Dear @PhoenixDev22, Thanks for your very helpful feedback. Your feedback made me to learn more things as i am very beginner in css so I don't know much about that but your .sr-only reference lead me to learn more and ofcourse I will try to understand your full code (in .sr-only) upcoming near days. Thanks for teaching me more.

  1
• 

  P

  Fluffy Kas•7,715

  @FluffyKasPosted almost 3 years ago

  Heyo,

  It's a very good looking solution! It's pretty close to the design, I only have a few thoughts to add really:

  1. Your border-radius seems to be correct in desktop view but then a bit messed up in mobile view. But this could be solved easily with a media query^^

  2. The images in this challenge are decorative only, so leaving their alt texts empty would be the best solution. You can give them an "aria-hidden=true" too, for good measure :D

  3. I'm also unsure about the "tablet" view (I assume there's a flex-wrap on the component). Perhaps it would look better if the 3rd card that slides down to a new row took up the whole width. But this isn't a biggie, just personal preference.

  All in all, well done on this one :)

  Marked as helpful

  1

  ---

  

  Aayush Raj•120

  @Prabhat-kumar-1976Posted almost 3 years ago

  @FluffyKas Thanks for your very helpful feedback.

  0