Responsive Product Preview Card

@ahmedafsa for that challenge, Flexbox is just what he needs for changing the alignment of the two sections.

For the mobile devices:

.flex-container {
    display: flex;
    flex-direction: column;
}

For the desktop breakpoints

.flex-container {
    flex-direction: row;
}

This is the best option for a beginner, than facing the 2-D structuring of the Grid Layout.

@OnyedikachiOzoani id choose grid over flex for this as the code is shorter for getting two equal width columns. Theres nothing wrong with either approach but grid is slightly shorter

@ahmedafsa this is great feedback, just a few minor ammendments

1. The mobile styles should almost always be the default. This means switching to min width media queries and making the mobile image the default in the picture element too. Much better for performance and UX.
2. Set max width on the component in rem not px. This ensures the component works for those of us who change our base font size
3. Media queries (in both the css and in the picture element) must be defined in rem or em not px. Again this means that those of us who change our base font size don't get a broken layout on some screen sizes. It means we will see the mobile and desktop layouts at appropriate sizes for the font size we are using.
4. In the picture element you don't need two sources like that. Make the mobile image the default in the img element. Make the desktop img url sit in the source tag with an appropriately sized (in rem or em) min width media query

Greetings @grace-snow

I implemented points 2 and 3 of your feedback. I can see the improvement in the page on mobile screens.

As for point 1, I will keep that in mind for future challenges. My approach was to have the desktop style be the default.

As for point 4, I will keep the code the way it is. the solution works as intended, and I am happy with the result. Your approach can work, back when I submitted this solution, I didn't know how to.

I appreciate your feedback, and i hope to receive more in the solutions I will provide in the future.

Hello @ahmedafsa

You are right. the best way would be to use CSS Grid for this challenge, due to its nature. I will keep this approach in mind for the future.

I appreciate your feedback, and I hope you will share more in my future solutions.

@Singraft I'm afraid your image method element is not working as intended. If you check the network tab in devtools, filtered to show images only, it should only be downloading the one image you want to serve. But at the moment, it is downloading both. The whole point of using the picture element is to avoid this and give the performance boost of only downloading necessary assets.

Also, I notice there are white corners showing on the component overlapping the corners of the image. This is because

• You are setting incorrect border radius on the component on mobile
• You should be setting the component to have overflow-hidden so it crops out the overlapping image corners. You are setting border radius again on the img element when it doesn't need it at all.

There are other things I notice in this that you need to fix. You may not understand yet why they are big problems but if you don't break the habits now you'll be in a world of pain later on in bigger challenges.

1. Do not set height on the component. Never limit the height of any element that contains text. This breaks badly for me when I change font styles. Additionally, we build components that can consume different amounts of content from authors/editors. As soon as you limit height, that component will need it's height adjusting every time there's a content change!
2. Similarly, the component should not have a width. Use max width instead, and make sure it's in rem so it works well no matter what the user's font size. (Also note it is a WCAG accessibility requirement for sites to work at 320px wide without needing horizontal scroll - another reason why max-width is preferable)
3. Only use rem for border radius if you want it to scale with font size. Often that's not desirable, in which case you may prefer to use px
4. The white area of the card (.info-area) should have padding on all sides
5. Remove all these extra divs!! Always keep the HTML nodes to the minimum required. This is creating a load of extra elements for no reason. Inside the white area of the card, all the content can and should be direct children
6. As this is a product card component, it is highly unlikely to contain the page heading (h1). It's much more likely to be placed on a grid of cards or sidebar of another page alongside other product cards. That tells you the heading level should at least be h2 in this
7. Always use headings appropriately and in the correct order to communicate content structure. A price will never be a heading. The prices are a paragraph, with a strikethrough or delete element for the old price.
8. Speaking of the old price, make sure it is communicated to screen readers that it is the old price. For example, include a span with sr-only class on it saying "original price" or similar before the strike through text.
9. If using flex, use gap! Don't place random paddings or margins on certain child elements when you have gap available to use.
10. Probably should have been the first point tbh - Always use a modern CSS reset at the start of the styles in every project. Get into that habit. Andy Bell has a good one.

@grace-snow I know you're my senior but I still believe readability of the code is better.

@OnyedikachiOzoani it sounds like you've missed the point. My feedback is not about readability it's about performance. It makes no difference whether flex or grid is used but neither approach is more or less readable than another. (That is down to your own experience / preferences which one you personally find more readable). One is shorter than the other that is all