Skip to content
OverviewSolutions
5
Comments
3

Xavier

@xavCS140 points

Computer Science student @ University of Warwick

I’m currently learning...

HTML, CSS, JS (soon), Java

Latest solutions

Latest comments

  • Xavier•140
    @xavCS

    Hey, nice attempt at this challenge, a few things:

    box-sizing: content-box;

    This declaration in your CSS is not really needed since the browser uses this by default and actually I'd recommend you go with this declaration instead:

    html {
    box-sizing: border-box;
}

*, *::before, *::after{
    box-sizing: inherit;
}

    https://developer.mozilla.org/en-US/docs/Web/CSS/box-sizing https://developer.mozilla.org/en-US/docs/Web/CSS/Containing_block A couple of useful MDN docs on this topic

    Your button is fine it just wanted a darker colour in the hover state, but using a styled <a></a> tag is better as it will link to a different place on the site to buy this product.

    I believe your image issue is coming from setting explicit heights and widths as well as the fact that you're trying to style the image whilst using the background-image property. It would be easier and more readable if you just import your image on the site using the <img></img> tag and set a max-width: 100%; on it. This will make it responsively sized by default as it will take as much space as it can within the container.

    You can check my solution out for some more insight:

    Hope this helped.

    Marked as helpful
  • Xavier•140
    @xavCS

    Hey, nice job - I do have a few comments on your code that could hopefully help you out.

    <div class="card-content">
      <h3 class="desc">PREVIEW</h3>
      <h1 class="title">Gabrielle Essence Eau De Parfum</h1>

    In this codeblock you've started with a h3 then gone to a h1, I would emphasise that you try to stick to the order of headers, where h1 is top level and h3 is 2 sub headings lower. This is more of an accesibility thing for people in general and more specifically screen readers! Using headers out of order breaks their semantic meaning so you'd really rather avoid that. For styling purposes, just resize and redesign with CSS.

    Small MDN docs page on headings https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements

    Another thing is your button, for this small project it's fine but you can imagine it would probably lead somewhere to buy the product, so using an anchor tag would be better to get that href in for effective browsing

    Your CSS is nice and tidy which is always a plus! Hope this helped

    Marked as helpful
  • Xavier•140
    @xavCS

    Overall I think you pretty much got the design and placement bang on, just a few margins and line spaces that could be adjusted I think if instead of setting the display:none on the relevant image when the viewing mode changes, you could instead use a <picture></picture> element. I'm suggesting this because the way you have it set the browser still loads both images, which isn't a good use of bandwidth, with the picture element you can dynamically tell the browser which one to load instead of loading all available images. Hope this was useful

    Marked as helpful