Skip to content
OverviewSolutions
4
Comments
3

Loris PUECH

@ShiironFrance180 points

I’m a mysterious individual who has yet to fill out my bio. One thing’s for certain: I love writing front-end code!

Latest solutions

Latest comments

  • Loris PUECH•180
    @Shiiron

    Hello there !

    First of all congratulation on finish your project !

    I think your code became messy because you don't have a hold on how to implement desktop or mobile design first.

    A growing number of applications and websites do work with 'Mobile first' as we say. This means that we start to work at a lower scale (mobile scale) and change things as resolution goes up.

    So in your code you are using media queries to target specific width in order to change style depending on the device resolution. But with the approach mobile first, the main styling should not be in media queries, but in plain css => That means that changes will only occurs when device resolution goes up. Therefore you should be changing the details inside a media query.

    I will try to give you an example from your code but feel free to tell me if you don't understand the principle : inside both your media queries you have this class (line 156) :

    .container{
    position: relative;
    display: grid;
    max-width: 22em;
    background-color: var(--clr-card-bg);
    padding: 1.5em;
    border-radius: 1em;
    gap: .45em;
    box-shadow: 0 0 3em .01em var(--clr-box-shadow);
}

    and below (line 262) :

      .container{
    position: relative;
    display: grid;
    max-width: 20.5em;
    background-color: var(--clr-card-bg);
    padding: 1.5em;
    border-radius: 1em;
    gap: .45em;
    box-shadow: 0 0 3em .01em var(--clr-box-shadow);
}

    We can see a bunch of attributes that are duplicated since they both apply to the class container (such as position and display). but we can see that the max width will change depanding on the query

    What we want to do is to say "okay, my design suggest that my box will be in position relative and display grid"

    so instead of doing multiple queries, you can simply say :

    .container {
  position: relative;
  display: grid;
  max-width: 20.5em;
}

@media (min-width:20.5em){
  .container {
    max-width: 22em;
  }
}

    This means that starting from 20.5em width, your container will have his max-width changed but without changing all the other attributes.

    I also have a few advices for you :

    • Be careful with indenting code => This makes your code harder to read and understand
    • When talking about design, if you decide to go mobile first you usually work with min-width since you want css to change starting a certains width and up. If you work desktop first, it's the other way around, you use max-width since you want to work from a certain width and below.

    Feel free to ask me again if you didn't understand what i said. Hope this help you in the futur !

    Keep up the great work !

    Marked as helpful
  • Loris PUECH•180
    @Shiiron

    Hi !

    First of all, congratulations on completing this challenge ! i hope it was fun and will make you do even more challenges in the futur !

    Classes vs Ids vs Selectors I don't think there is a definitive answer to that but as far as i'm concerned, i tend to avoid ids as much as i can because we use Frontend framework to make reusable component. If you keep that idea in mind, we won't have a lot of elements with ids, since we don't want any duplicate

    I think the way you handled it is pretty clean with the use of classes and selector. This avoid having nightmare nesting (especially in scss) and allow you to target multiple elements easily. My general rule is to add a class when i need to specifically target an element, otherwise i use selector as much as possible. It's also important to keep in mind that in frontend framework, with the use of components, using selector can cause conflict because css loaded with 1 component can cause effect on another component if you only target selectors

    Good practice I would give just 2 advices from what i saw, since you used native HTML / CSS :

    • You have an assets folder but you still have svg markup in your html => I think it's better to keep assets separated from the code, therefore i would make .svg file with all the svg you have in the html (for example, the eye icon you have on hover => make an eye.svg file and paste code there). It makes a cleaner and easier code to read
    • Create multiple files => The reset part in your css is good but i would separate it from the style.css file and create a reset.css file. This will be useful if you want to try futur framework where you want as little code as possible so everything has 1 single purpose. But you use comments to separate your css files which is already very good ! (don't be afraid to use even bigger comment block)

    I hope my feedback will be useful to you ! i'm not the best devs around but if i can help, i try my best !

    Have a great day and keep up the good work !

    Marked as helpful
  • Loris PUECH•180
    @Shiiron

    Hi !

    to change the color and to display the icon on hover for the image, the easiest way (for me) is to create an overlay with an opacity of 0 and using :hover to set it up as 1 (you can also add transition for a smooth effect)

    <div class="card-image"> <img src="{yourImage}"> <div class="card-image-overlay center"> <img class="card-view-icon" src="{yourIcon}"> </div> </div>

    the "card-image" div is in position relative and the "card-image-overlay" is on absolute. From there you can customize and style it.

    I hope it helps !

    Marked as helpful