Skip to content
OverviewSolutions
5
Comments
5
josip-h
@josip-h

All comments

  • Daniel Zeljko•50
    @danielzeljko
    What are you most proud of, and what would you do differently next time?

    There's nothing I'm particularly proud of as it's such a simple component.

    What challenges did you encounter, and how did you overcome them?

    I ran into issues where setting a fixed height on project-card would cause the checkout button to overlap with the bottom margin. So, instead of having 32px (desktop padding) all around the project-card__body the actual bottom padding would be something like 15pxs.

    What specific areas of your project would you like help with?

    How do we get the project card to have a fixed height without breaking the padding around the project card?

    Responsive Product Preview Card

    #bem
    1
    josip-h•90
    @josip-h

    Hi Daniel Zeljko,

    Congratulations on completing the project. I appreciate how clean and modular your CSS code is. I noticed that you used the <picture> element to load different image resources for desktop and mobile versions based on the screen size. Coincidentally, I was introduced to this method just yesterday through feedback on my solution.

    In the solution screenshot of the desktop version of the component, the picture and text containers equally divide the card component, but this isn't the case in your solution. To fix this, I suggest adding the box-sizing property with the value border-box to all elements, and then setting the widths of the picture and text components to 50% for the desktop version of the component. This should resolve the issue.

  • Dylan de Bruijn•3,220
    @DylandeBruijn
    What are you most proud of, and what would you do differently next time?

    I'm proud getting the solution close to the design specs. Next time I would inspect the design in more detail so I have a better plan for my HTML and CSS structure. It feels a bit bloated at the moment and can probably use a clean up.

    What challenges did you encounter, and how did you overcome them?

    I ran into problems getting the padding right between mobile and desktop. I wrapped the content in a separate div and wrote some media queries but I'm wondering if there is a more elegant solution.

    What specific areas of your project would you like help with?

    All general feedback is welcome. If anyone has some tips and tricks for styling lists and tables let me know!

    Recipe Page

    #bem#vite#sass/scss
    2
    josip-h•90
    @josip-h

    Hi Dylan!

    We meet again. This time, I was assigned to review your solution by the algorithm. Your solution looks good overall, but upon closer inspection, I found a few small differences in padding and gap dimensions compared to the ones specified in the Figma file.

    1. In the desktop version of the page, the gap between the image and the content should be 40px, but yours is 24px.
    2. In the mobile version, the content padding should be 40px on the top and bottom and 32px on the sides, whereas yours is 24px on all sides.
    3. Another small difference is in the list items: their markers (bullets or numbers) should be a bit more indented. In the Figma file, they are 8px from the left border of their parent container.

    I can't comment much on your CSS code since I don't know Sass, but I can say that it's much shorter and cleaner than my "vanilla" CSS code. I'll make sure to learn Sass once I solidify my knowledge of "vanilla" CSS.

    Marked as helpful
  • josip-h•90
    @josip-h

    Hi Marwin!

    Beautiful solution. I'm somewhat new to web development and recently solved the same challenge. Both solutions look similar but yours is much more refined. Just those transitions between changing the background color of links do so much to make it feel more professional. Thanks for sharing. I'll be sure to follow up and check your other work.

  • josip-h•90
    @josip-h

    Hi Jude!

    Congratulations on solving the challenge. I like how clean and easy to understand your code is. It's close to being pixel-perfect. I think that there are two ways in which your code differs from the one specified by the style guide.

    1. You used the font-weight property of 900 for the .card-content__category class and the .card-content__title class. In the style guide document, I see only two font weights specified, 500 and 800.
    2. Also in the style guide, there is this sentence: "Ensure content is responsive and meets WCAG requirements by testing the full range of screen sizes from 320px to larger screens.". On screen sizes that are less than 352px wide, your card gets a bit hidden.

    To fix that I would do the following:

    • In the .card class:

      • change the height property to .min-height with the value of 501px.
      • add 10px margin-left and 20px margin-right

    • Changes in the .card-illustration class:

      • change the width property to a max-width property of 327px

    I tested your solution with these changes and it seems to me that then everything is within style-guide specifications.

    Also, I would like to say how nice your solution for exactly separating the card elements by 24px by using the space-evenly value for the justify-content property. I didn't see that, and I specified margin values for each element in my solution... By reading through your solution I found many ways in which I could improve on my solution.

    Thanks for sharing.

    Marked as helpful
  • josip-h•90
    @josip-h

    Hi Nathaniel, congratulations on completing the challenge! When looking at your solution submission I have noticed a BEM hashtag. I recently searched for some way of organizing/naming CSS to use especially on a bit larger projects than this one and learned about BEM. As I understood by reading through getbem website and by watching a few YouTube videos on the topic, block and its elements should be separated by double underscores. So in your HTML code

    <div class="card"> <div class=“card-content"> <div class=“card-img"> <img src="images/image-qr-code.png" alt="qr-code"> </div> <div class=“card-text"> <h2 class=“card-heading">Improve your front-end skills by building projects</h2> <p class=“card-desc">Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p> </div> </div> </div> If element with .card class is a block then every tag that has no standalone meaning and is semantically tied to it is its element. As per element definition on getbem.

    In your example standalone elements of card block are: image, heading and paragraph elements. By BEM convention class names of elements should be created by formula: “.block-name__element-name” <=> block name plus two underscores plus element name.

    For example in your case, heading element should have a class of “.card__card-heading”, but for sake of not repeating yourself class name can be shortened to “.card__heading”. Paragraph element should have a class of “.card__card-desc” that is “.card__desc” for short.

    Divs with classes “.card-content”. “.card-img” and “.card-text” are containers or wrappers used for grouping/styling purposes and are treated the same way as elements no matter how deeply nested they are. You can read about that it this article.

    So as I understand it your piece of code written using BEM convention should look like this:

    <div class="card"> <div class=“card__content-wrapper“> <div class=“card__img-wrapper”> <img class=“card__qr-code-img” src="images/image-qr-code.png" alt="qr-code"> </div> <div class=“card__text-wrapper“> <h2 class=“card__heading">Improve your front-end skills by building projects</h2> <p class=“card__desc">Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p> </div> </div> </div> Not long ago I started learning CSS for the n-th time! This time after learning about BEM CSS files that I was writing had structure and it no longer seemed that code was as random or all over the place. This helped me a lot in terms of motivation for further study.

    Also, there are a few things that initially caused a problem in my Accessibility report.

    First thing is that I din’t use h1 tag in the document. Changing card title from h2 tag to h1 solved it for me. Another thing that caused an accessibility error was that I didn’t wrap my card component in any semantic tag. I fixed that problem by wrapping card component in main tag and acknowledgments in footer tag. Now that I think about it probably card and attribution components should both be inside the main tag.

    Does your Accessibility report show any errors or warnings since I see that you have similar situation as I did?

    I’d appreciate if you checked my solution and shared any suggestions. I hope this helps in any way shape or form and that I was clear enough since English is not my first language.

    Best regards, Josip.