Design comparison
Solution retrospective
Hola, subo mi solución tratando de limpiar mas el proyecto segun recomendaciones de la comunidad, generé el archivo readme.md de manera de que mejorara la presentación del proyecto.
Community feedback
- @mauricio-fontouraPosted about 2 years ago<main class= "wrapper"> <section class="card-container"> <div class="card"> <div> <img src="./images/image-product-desktop.jpg" alt="img product"> </div> <div class="card-body"> <h2 class="card-title">Perfume</h2> <h2 class="card-subtitle">Gabrielle Essence Eau De Parfum</h2>
</div> <section class=" price-container"> <div class="card-price"> <h1 >$149.99 </h1> <p> $169.99</p> </div> <div class="card-button"> <button> <img src="./images/icon-cart.svg" alt="" /> </button> </div> </section> </div> </section> </main><p> A floral, solar and voluptuous interpretation composed by Olivier Polge, Perfumer-Creator for the House of CHANEL. </p>
Marked as helpful0 - @mauricio-fontouraPosted about 2 years ago
Hello my friend @cyeguez,
Congrats on completing the challenge
You can also try to separate the written content and images within the HTML with div, section or article. depending on the project. This way you will have more vision of where you will separate the content by block or better contains. For example:
Marked as helpful0 - @hyrongennikePosted about 2 years ago
Hi @cyeguez,
Congrats on completing the challenge
Just a few suggestions. You can make the following change to your CSS rule to center the card on the page.
.card { min-width: 320px; max-width: 800px; height: auto; display: grid; grid-template-columns: 1fr 1fr; border-radius: 15px; grid-template-rows: 1fr; }
it's just the
height
and thegrid-template-rows
that was added.The below are all for the text and spacing the content in the right column
.card-body { padding: 3rem; } .card-subtitle { font-size: 4rem; margin: 1.5rem 0; } p.card-button { margin-top: 2rem; margin-bottom: 0; }
Hope this is helpful.
Marked as helpful0 - @correlucasPosted about 2 years ago
👾Hello Cesar, congratulations for your first solution!👋 Welcome to the Frontend Mentor Coding Community!
Amazing solution! I’ve just opened the solution’s live site and I liked the job you’ve done a lot. I’ve some suggestions for you:
The approach you've used to center this card vertically is not the best way, because using margins you don't have much control over the component when it scales. My suggestion is that you do this alignment with
flexbox
using the body as a reference for the container.The first thing you need to do is to remove the margins used to align it, then apply
min-height: 100vh
to make the body height size becomes 100% of the screen height, this way you make sure that whatever the situation the child element (the container) align the body withdisplay: flex
andalign-items: center
/justify-items: center
.body { display: flex; background-color: var(--Cream); min-height: 100vh; flex-direction: column; align-items: center; justify-content: center; }
THE PICTURE TAG is a shortcut to deal with the multiple images in this challenge. So you can use the
<picture>
tag instead of importing this as an<img>
or using a div withbackground-image
. Use it to place the images and make the change between mobile and desktop, instead of using adiv
orimg
and set the change in the css withdisplay: none
with the tag picture is more practical and easy. Note that for SEO / search engine reasons isn’t a better practice import this product image with CSS since this will make it harder to the image. Manage both images inside the<picture>
tag and use the html to code to set when the images should change setting the devicemax-width
depending of the device desktop + mobile.Check the link for the official documentation for
<picture>
in W3 SCHOOLS:https://www.w3schools.com/tags/tag_picture.asp
I saw that for some properties you’ve used
rem
and for otherspx
. In this case it is better to use only one kind of unit to have a better organization for your code.relative units
asrem
orem
that have a better fit if you want your site more accessible between different screen sizes and devices.REM
andEM
does not just apply to font size, but to all sizes as well.✌️ I hope this helps you and happy coding!
1
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