Responsive Parfume Card in React + CSS Flexbox

Community feedback

• 

  T

  Grace•31,250

  @grace-snowPosted over 1 year ago

  Hi

  You’ve got some big problems here I’m afraid. It’s super important to not forget the essentials of semantically meaningful and properly structured html.

  Changes needed:

  • the image is very valuable content on a product card. It must be in the html and have a proper description
  • the incorrect image is loading in mobile. Use the picture element so the browser can do its job and load the correct one
  • never have text in a div or span alone. You are missing almost all meaningful elements in this
  • the old price will need wrapping in an s or del element and will need some additional visually-hidden text to make it clear that’s the old price. Assistive tech users are not told about line through styling so would not know which is the new vs old price
  • it is invalid html to have a paragraph inside a button
  • you must never ever put font size in px. Use rem so that user preferences are respected
  • the card must not have an explicit height or width, only a max width. No height is very important, again so that it doesn’t break if users prefer a different font size
  • not essential but you should be working mobile first

  I also recommend you have a repo per project as is usual. Git histories get very messed up when using the mono repo approach you seem to be following

  Marked as helpful

  1

  ---

  

  P

  visualdennis•8,375

  @visualdennissPosted over 1 year ago

  @grace-snow Hi,

  Thanks for the useful and detailed suggestions as well as pointing out my mistakes! I've noted them down on a new word in google docs and I'll definitely try to apply them as much as i can in newer the projects that i'll be building in the future.

  As for Git and Repo structure. Thank for the tip. But for this specific project (fm challenges) i'm not making much use of the git history as the challenges are relatively small and encapsulated projects. Also they are all part of a bigger website/app so that i can easily switch between them or showcase. For this reason it is not so much practical for me to create new react app for every challenge, especially when they are small, and deploy them manually and separately.

  0