Skip to content
Submitted about 2 years ago

responsiveness was done with flexbox

Greatemmet•20
@GREATEMMET
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...Product preview card component
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Solution retrospective

it was difficult working with the desktop image, as i was trying to fit it into a 500px height container. i set the height of the image to be 100% but it ignored the height container and maintained its predefined height. Tried different values, went back and forth my code to see of i made any mistakes but i found nothing. i later had to set the container to take the height of the image. if there is a way around this, i'd really love to know.. All feedbacks are welcome...

Community feedback

  • DJ Drakos•150
    @dj-drakosPosted about 2 years ago

    Hey @GREATEMMET, good work on this.

    I took a peek at your repo, and noticed you're now using the width property to control the image size, which seems to be working! Nice.

    Without playing around with your code, I'm wondering if the issue you're having has to do with how CSS treats the height property. If the parent element (.desktop-img) doesn't have an explicitly defined height, CSS will ignore the height you set on the child (img). That explicitly defined height could be set on the .desktop-img div, or be inherited from its parent... Lots of different approaches.

    CSS-Tricks has a good post that goes more in depth.

    Marked as helpful
    0
    Greatemmet•20
    @GREATEMMETPosted about 2 years ago

    @dj-drakos Thank you so much for your feedback.. Does this mean that after predefining the height of the parent container, I should set the value of img height to inherit or 100%..

    0
    DJ Drakos•150
    @dj-drakosPosted about 2 years ago

    @GREATEMMET No problem, happy to help.

    I think in this case, you've solved the issue using the .desktop-img { width: 400px } img { width: 100%} rules... but I'm going to go on a journey and give you my thoughts on your follow up question.

    If you do decide to refactor and try using the height property instead, either declaration would work. I would choose height: 100% because it makes more sense to us as humans and is less brittle.

    .desktop-img { height: 600px } img { height: 100% } tells me that you, the developer, want the image to take up 100% of the space of its container. I can get an idea of your intentions handling the architecture of the layout, which makes it easier for me to read and work with your code, while staying true to your design choices.

    .desktop-img { height: 600px } img { height: inherit } tells me that you want the image to inherit the same value from its parent, but doesn't give me the same hints to your intention. It also complies differently, as the child value is now dependent on the parent. What would happen if the .desktop-img class got set to height: 100%?

    Marked as helpful
    0
    Greatemmet•20
    @GREATEMMETPosted about 2 years ago

    @dj-drakos makes a lot of sense.. i rewrote the css style again and used img width 100% and it worked perfectly fine. thanks once again... i have pushed the update to my github aswell so i hope it automatically updates on my completed challenges.

    0
  • ali•540
    @alishirani1384Posted about 2 years ago

    Hi @GREATEMMET you did a very excellent job on this challenge to make it center both vertically and horizontally you can add this code to your body: body{ min-height: 100vh; display: flex; align-items: center; justify-content: center; flex-direction: column; }

    Marked as helpful
    0
    Greatemmet•20
    @GREATEMMETPosted about 2 years ago

    @alishirani1384 Thanks for the feedback.. i have been using the max-height property for my body and i noticed it was given me what i wanted.. i am going to try the min-height property instead.. i think this should solve the issues i am having with the body.. Also i have taken note of the flex properties being used in the body{}.. i shall apply this

    0
  • Lucas 👾•104,340
    @correlucasPosted about 2 years ago

    👾Hello @GREATEMMET, Congratulations on completing this challenge!

    Your solution its almost done and I’ve some tips to help you to improve it:

    Add the website favicon inserting the svg image inside the <head>. <link rel="icon" type="image/x-icon" href="./images/favicon-32x32.png">

    A better way to work this solution image, the product image is by using <picture> to wrap it on the html instead of using it as <img> or background-image (with the css). Using <picture> you wrap both images (desktop and mobile) and have more control over it, since you can set in the html when the images changes setting the screen size for each image.ote 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.

    Here’s the documentation and the guide to use this tag: https://www.w3schools.com/tags/tag_picture.asp

    See the example below:

    <picture>
  <source media="(max-width:650px)" srcset="./images/image-product-mobile.jpg">
  <img src="./images/image-product-desktop.jpg" alt="Gabrielle Parfum" style="width:auto;">
</picture>

    👨‍💻Here's my solution for this challenge if you wants to see how I build it: https://www.frontendmentor.io/solutions/product-preview-card-vanilla-css-and-custom-hover-state-on-hero-85A1JsueD1

    ✌️ I hope this helps you and happy coding!

    0

Please log in to post a comment

Log in with GitHub

Join 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