Design comparison
Solution retrospective
Screenshot shows the mobile version. Is there a quick fix for it? Check the browser version for a desktop layout.
I see that I got some errors, the first one I was experimenting and forgot to remove that code.
If someone could elaborate on the "inline" errors, I would appreciate :)
Is there a proper/specific way to structure css code, or is mine just fine?
Also, I wonder if there is a way to reduce the amount of css/ make it less repetitive.
Any general advice would be nice, thank you!
P.S. Sorry If i did not follow some of your suggestions from the previous post. I will add them eventually.
Community feedback
- @HassiaiPosted almost 2 years ago
Wrap the prices in a div, the new price in <h2> tag and the old price in a <p> tag instead of the inline- tag. give the div a class for the styling in the css. in the css give it display: flex, a gap value and align-items, center. to center align the old price with the new price i dont think you can achieve this with the inline element.
in the html: <div class=" prices"> <h2><h2> <p></p> </div> in the css: .prices{ display: flex; align-items: center; gap: 1em; }
Wrap <div class="attribution"> in a footer tag to fix the accessibility issues,
Use rem or em as unit for the padding, margin, width and preferably rem for the font-size for more on CSS units Click here
Hope am helpful.
Well done for completing this challenge. HAPPY CODING
Marked as helpful1@grace-snowPosted almost 2 years ago@Hassiai price should not be a heading element, that would not make sense. Instead this should be a paragraph (or two)
The old price should be wrapped in a
s
ordel
element and should have some visually hidden text inside to identify that as the old price. This is because screenreader users are not told when text has a line throughMarked as helpful1 - @grace-snowPosted almost 2 years ago
Hello
I hope this also helps
- you need to write a proper alt description for the image. Describe what it looks like as if someone couldn't see it. Alt should not include words like image or picture because they are already an image element
- capitalise perfume in the css not in the html
- see my previous comment for correct markup for prices. Inline is not a html element at all
- you must use a button element for the button. Always use interactive elements for interactive behaviour.
- the cart icon is decorative so should have an empty alt attribute
- attribution needs to be in a footer element
CSS
- nothing in this except the cart icon should have a fixed width or height. It's really important not to limit height on elements that contain text content. Let the browser decide how tall it needs to be
- max width only on the component
- height 100% on the image on desktop along with the object fit property
- font size must never be in px! Use rem. Be very careful using em units, it's extremely rare to use that for font size
- button should not have a height and should have a width of 100%. Control it's height with padding
- no negative margins needed on this
- no need to limit the width of the text elements inside the card. The padding on the text half of the card should be enough
- 1440px is WAY too big for a desktop media query. You should be switching layout at the point where there is room for the layout to change
- media queries should be declared in rem
- next time and in every project moving forward, work mobile first and use a min width media query instead
- unset the img height on mobile and give it a min height in rem instead
- all the same principles or styles on mobile - no heights, no negative margins, only a max-width on the component, no fixed widths etc
Marked as helpful0
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord