Responsive, Accessible Product Component

Community feedback

• 

  T

  Grace•31,250

  @grace-snowPosted 3 months ago

  Great solution overall, you should be pleased with this.

  Here are a few issues I noticed:

  • the footer should be outside of main.
  • try not to capitalise text in HTML. Use css to capitalise text if it’s a styling choice like this.
  • look up how to write better alt text. There are some good posts in the resources channel on discord about this.
  • you don't need to add aria-hidden on decorative images. Having the empty alt attribute is enough.
  • I don't think the price should be in an ins element. It's just text, it's not been "inserted" like in a document highlighting edits. I'd expect a paragraph, either one containing both the current and old price or two paragraphs, with the old price wrapped in an s or del (strictly speaking the strike through is more appropriate than delete).
  • try to think about the context of where this component would be used. It's a single product card. It is extremely unlikely it would act as the main heading for a whole page of content, and that tells you that this should not have a h1. Use a lower importance heading level.
  • if a link opens in a new window like in that footer link, it needs to communicate that to screen readers either with some Sr only text in a span or via the title attribute on the link (eg "(opens in a new tab)").
  • if you're gonna make the body a flex element it should have a column direction.
  • get into the habit of including a full modern css reset at the start of the styles in every project.
  • line height is usually unitless. It's not a problem it being in rem, just unusual and makes css longer.
  • if you want to improve performance you can use the aspect ratio property on the img.
  • it's better to define media queries in rem or em not px, so the layout can adjust well for all users including those who have a different text size.
  • personally I would use grid and not flex on the component as its shorter to make it have two equal columns on larger screens.

  Marked as helpful

  5

  ---

  

  P

  Steven Stroud•11,910

  @StroudyPosted 3 months ago

  @grace-snow Once again the wealth of knowledge you have is great, I can't express how grateful I am for your feedback and explanations. Thank you for pointing these out so I can look at them in more detail <3 Unbelievable member of the team/community.

  1

  

  P

  Steven Stroud•11,910

  @StroudyPosted 3 months ago

  @grace-snow I have taken every bit of feedback you have given and made positive changes, I am always going to reference back to this for future projects. When I become a Frontend Developer I am going to speak highly of you.

  Summary of Changes

  1. Text Capitalization:

     • Removed capitalized text in HTML and used text-transform: uppercase; in CSS.

  2. Accessibility for Decorative Images:

     • Removed aria-hidden="true" from decorative images and used an empty alt="".

  3. Improved Alt Text:

     • Reviewed and improved alt text based on best practices.

  4. Corrected Price Markup:

     • Changed <ins> to <p> for the current price and used <s> for the old price.

  5. Adjusted Heading Levels:

     • Changed <h1> to <h2> for the product card heading.

  6. Accessibility for Links:

     • Added title attributes to links that open in new tabs (e.g., title="(opens in a new tab)").

  7. Flexbox Layout:

     • Changed flex-direction to column on the body.

  8. Added CSS Reset:

     • Included a full modern CSS reset at the start of the stylesheet.

  9. Unitless Line Height:

     • Made line-height unitless for better readability.

  10. Aspect Ratio for Images:

      • Used the aspect-ratio property for images.

  11. Media Query Units:

      • Converted media queries from px to rem units for better responsiveness.

  2