Design comparison
Solution retrospective
I wasn't able to align the div with the prices at the center with display: flex; align-items: center;
so I used align-items: baseline
instead.
Any help on what might have caused it not to work will be highly appreciated.
Community feedback
- @asdiAdiPosted about 2 years ago
Hi Valarie Oyieke, it seems that the
p
margins inside the.price
class are the root of the problem.- First, add a
.price > p
selector and addmargin: 0
property inside it - Then add a
margin-top: 32px
property inside the.price
class - Finally, you can change the
align-items
property intocenter
inside the.price
class
I hope this helps you and Good luck with future challenges!
Marked as helpful1@ValarieOyiekePosted about 2 years agoHello Adi, Thank you so much. It worked π
1@grace-snowPosted about 2 years ago@asdiAdi @ValarieOyieke I do not recommend this approach at all. I will add feedback on the challenge below
0 - First, add a
- @grace-snowPosted about 2 years ago
There are quite a few issues I'm seeing on this, and I'll try to go through methodically
Html
- use the picture element not two img tags
- remove the section element, you are misusing it. A div is fine as all you're trying to do is encapsulate the text part of the card for layout purposes.
- alt text on meaningful images (like the product image) must be an actual description of the image. Tell me what it looks like briefly as if I can't see it
- alt text on decorative images (like the cart icon) should be left intentionally blank
- perfume is not a heading. It is a paragraph. It is just ribbon text. You can wrap that paragraph and the heading after it both inside one hgroup element if you want.
- well done for using the del element and for using a button element on this!
Css
- to center the card on the screen use flex or grid properties along with min-height 100vh
- remove all that position absolute 50% stuff. That is not how you'd center a component with Content inside. You'd rarely use that technique at all in modern development. At the moment it is causing your card to be cut off on mobile. You have to understand what positioning absolutely does by removing content from the dom
- use classes on what you want to style. I've advised you change some of the html on this. See how you now will have to change css as well. Separate those concerns by using class selectors in css
- use a modern css reset at the start and that will remove the browser styles like margins for you
- never use px for letter spacing. Use em so it scales with the font size
- you can set border radius on the whole component (the card) with overflow hidden so the corners of the img gets cropped to the radius. Then no need to set individual corner radius on images at each breakpoint
- I don't see any issue with price alignment
- the button can be flex or inline flex and also use align-items and gap properties instead of margin
- the del element is not actually announced to screenreaders or other assistive technology, so you either need to add accessibly hidden text in spans on the html or in pseudo element content. This post explains how to add content to del
Marked as helpful0@ValarieOyiekePosted about 2 years ago@grace-snow Thank you so much. Let me work on everything
0 - @BrijenMakwanaPosted about 2 years ago
Hey Valarie Oyieke, very nice. One suggestion from me is that to centre the
.wrapper
class, you can apply following styling.margin-left:auto; margin-right:auto;
Good luck for the next challenges.
0
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