Stephen Funderburk• 180
@Mr-Funderburk
Posted
Nice job!
There are two different images we were supposed to use for mobile/desktop; though I don't think your solution is a bad one. The idea behind using two different images is to ensure the images load quickly on mobile devices. For me, I found it easiest to create a div the size of the image and set the background to be the image for desktop/mobile with media queries.
It's my understanding that we are trying to get our solution to look like the design that is given to us. Therefore, I would spend a bit more time on your CSS.
- There's an icon missing on the button -- in fact, this should have been a styled anchor tag since we're not using a form here.
- The typeface and font size of the "new" price should be changed to match the display
- The spacing you have is a lot larger than the display, however, I wouldn't say that would "lose points" on this. I do think too much space creates a disconnect; so I would bring it in a bit.
- As you've seen from the report that is generated when you submit, you're missing semantic tags (header, main, etc.). I don't fully agree with the standards because these are just snippets of what would be a larger project. However, I know that will be brought up.
- I wouldn't say it's strictly wrong to put everything in a div/container, however, it will create a lot of elements that you don't necessarily need. The image for example doesn't need a container. You could have changed its display and treated it as a block element without the use of a div. The class 'product-info' could have been left off and just targeted the h4/'title' class. Instead of a div for 'product-description', this could have been a paragraph tag, and the span isn't needed.
Overall, I would say great work!
1
