I agree with all of the above
I'd add
- don't set width on the component. It's overflowing my screen. Instead only use max-width so it can shrink if it needs to
- min height instead of height on the body. Same principle except this time you want the body to be able to get bigger if there's more content
- for shorter code you can set border radius on the whole component and use overflow hidden so the corners of the img gets cropped. This is easier than setting individual corner radius on images at each breakpoint
- button shouldn't have height or explicit width. Let it be display flex or inline flex and use padding instead of height
- youre not using the picture element correctly here. Just as you should work mobile first in styling, do the same with picture and let mobile img be the default src on the img tag. Then you only need ONE source with a min width media query on it to make the switch for larger screens
- capitalise text with text transform and create space with letter spacing, both in css not html
- this must have a heading element for the name of the product
- the old price should be wrapped in a del ele. Screenreaders are not told when text has a line through so you also need to add some visually hidden text in a span or pseudo element to inform those users that it's the old price (has been deleted)
- if using an inline svg for the (decorative not meaningful) cart icon you must add
aria-hidden="true" focusable="false"
- make sure the component has a little margin so it can't hit screen edges (or its wrapping element has a little padding)
- not essential but id recommend putting the attribution in a footer element outside of main
Marked as helpful
@ananfito
Posted
@grace-snow Thanks for the additional suggestions. Adding them to my "to-do" list for project revisions. (-: