Skip to content
OverviewSolutions
5
Comments
3
Maciej Duda
@mcdud

All comments

  • Joshua•25
    @minimalsmSubmitted almost 3 years ago

    Completed my first frontend mentor challenge, yay 🎉

    Questions

    • How could I improve the naming of my selectors? I've spent too long in StyledComponents land... I think they're okay but definitely could be better.
    • I feel like the stylesheet could be better organised, any tips here?
    • I originally used a H2 for the 'Join our community' heading. Realised the HTML checker would shout at me if I didn't have it as an H1 but it feels kind of strange having it as an H1 considering this would likely be a component within a page, rather than a page itself?

    Mobile first single price grid component (HTML, CSS)

    2
    Maciej Duda•100
    @mcdudPosted almost 3 years ago

    Hi, @minimalsm!

    Great job!

    • look into BEM methodology for naming, it will help you better structure your HTML and CSS
    • you can use H2 for the component, and add screen reader only H1. Here is the comment from @darryncodes which helped me with the same problem.

    Good luck!

    Marked as helpful
    2
  • Maciej Duda•100
    @mcdudPosted almost 3 years ago

    Hi, @Fortwinhamz!

    Nice one!

    After reviewing your code:

    • it's easier to design mobile-first in my opinion.
    • use font-family provided in the design. You can easily google how to add it from google fonts.
    • for better file structure put your images in images folder
    • you should look into CSS relative units, to make your solution scalable.
    • look into CSS property box-sizing, setting it to border-box will make your life easier.
    • you shouldn't have fixed width and heights on your cards because you can easily overflow them with content. Put only max-width on your container and your cards should have size based on the content that you put in them.
    • to make your code less repetitive you can use multiple classes in one HTML element, cards in this solution have mostly the same styles, only few differences. So you could structure your code like this:

    HTML

    <div class="container">
        <div class="card card-violet daniel">content</div>
        <div class="card card-light-gray jonathan">content</div>
        <div class="card card-white kira">content</div>
        <div class="card card-dark-gray jeanette">content</div>
        <div class="card card-white patrick">content</div>
</div>

    CSS

    .card {
*styles repeating in all of those cards*
}

.card-violet {
*styles only in violet card*
}

.daniel {
*for grid-area purposes*
}

    For naming your classes look into BEM methodology.

    Overall pretty nice job with this one. Good luck!

    Marked as helpful
    1
  • Victor•50
    @zekt1209Submitted almost 3 years ago

    I had to improvise the images color because it originally came in back & white, I added a violet container and reduce the opacity of the image to %50 and I got that result, My question is if im okay on that step.

    stats-preview-card-component-main

    1
    Maciej Duda•100
    @mcdudPosted almost 3 years ago

    Hi, @zekt1209!

    Really nice job with this one!

    • nice, clean code, easy to understand.
    • you could look into CSS relative units, to make your card scalable.
    • your solution with the image looks fine, you could also do this like this:
    .article-img {
  width: 50%;
  background-color: rgb(166, 102, 206);
}
.article-img img {
  border-top-right-radius: 8px;
  border-bottom-right-radius: 8px;
  mix-blend-mode: multiply;
}

    You can also look at my solution and see how you can add overlay on image with ::after pseudoelement.

    Good luck!

    0