Skip to content
OverviewSolutions
47
Comments
24
ROCKY BARUA
@Drougnov

All comments

  • Christian Novo Sacramento•20
    @cnsacramentoSubmitted almost 2 years ago
    1. Is it semantic correct?
    2. Should I use always class names instead of high level tags?
    3. Should I import font in HTML or CSS?
    4. What could I improve in my CSS?

    QR Code component

    1
    ROCKY BARUA•1,090
    @DrougnovPosted almost 2 years ago

    Hello @cnsacramento, great job. The code looks neat and clean. Here are some things you can improve:

    • Change the h2 to h1 as the page should contain a level-one heading.
    • The box shadow is not working on the .card, cause you've only provided the color. You also need to provide the h-offset and v-offset value. You can learn more about box shadow from here
    1
  • i_d_s_l•270
    @ilvdrsknSubmitted almost 2 years ago

    hello everyone! I have a problem with a modal window that pops up when you click on the menu button: for some reason it does not cover the entire window, but only the head. How can this problem be solved?

    FLEX||GRID

    1
    ROCKY BARUA•1,090
    @DrougnovPosted almost 2 years ago

    Hello @ilvdrskn, great job on completing this challenge.

    As the .modal-menu is an absolute element and its parent .head doesn't have a defined height, it is only taking the head's initial height. To fix this, simply remove the height: 100% from the .modal-menu, and it will get the desired full height.

    Also, try to fix the accessibility warnings. If you need further help on this, feel free to ask or check out my solution. Have a good day :)

    Marked as helpful
    1
  • ROCKY BARUA•1,090
    @DrougnovPosted about 2 years ago

    Hi @SpacemanOG, the design is looking great. Good job.

    Use a pseudo-element(::after,::before) to create an overlay on that image container. Like this:

     .heroImage::after {
  position: absolute;
  content: "";
  width: 100%;
  height: 100%;
  background: hsla(277, 64%, 40%, 0.6);
  top: 0;
  left: 0;
}

    And don't forget to add position: relative to the heroImage.

    You can remove the 'source media' for the 300px width as the img will be shown by default on lower that 1200px width screen.

    Hope this helps. Have a good day :)

    Marked as helpful
    0
  • Cenk•90
    @cenkdermanSubmitted about 2 years ago

    Frankly, I had some difficulty in responsive design and it took a lot of time. Why do you think I wasted so much time? Please review my code, I would appreciate if you indicate the places you think are missing or unnecessary.

    Product preview card component solution using flexbox

    1
    ROCKY BARUA•1,090
    @DrougnovPosted about 2 years ago

    Hello @cenkderman, great job.

    • The design is looking perfect on the desktop viewport. But the text-content is exceeding the container in mobile view. Avoid using height if not absolutely necessary.

    • The google font link is wrong. In the HTML change your font link to this:

    <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link href="https://fonts.googleapis.com/css2?family=Fraunces:opsz,wght@9..144,700&family=Montserrat:wght@500;700&display=swap" rel="stylesheet">```.

Or, just simply add this line at the top of your CSS :

```@import url('https://fonts.googleapis.com/css2?family=Fraunces:opsz,[email protected],700&family=Montserrat:wght@500;700&display=swap');```
    • Use a default font in the body tag. Ex: Montserrat and use the other one on the needed element. In this way, you can save some lines.
    • Although you have beautifully added the images with display none and media queries, the semantic way would be using the picture tag. Like this:

    <picture> <source media="(min-width: 650px)" srcset="./images/image-product-desktop.jpg"> <img src="./images/image-product-mobile.jpg" alt="Gabrielle CHANEL's perfume bottle"> </picture>

    Don't forget the alt text 😉.

    You can check my solution for further information. If you have further questions, feel free to ask. Have a good day.

    (edit) Having trouble formatting the comment. I hope you can still understand what I'm trying to say

    Marked as helpful
    0
  • ROCKY BARUA•1,090
    @DrougnovPosted about 2 years ago

    Hi @caio-rosa, good job on the challenge.

    To achieve the purple filter on the image, you can use pseudo-code(after or before) on the container. At first use position relative to the container.:

    .container {
  position: relative;
  z-index: 0;
  overflow: hidden;
}

    And add after to the container like this:

    .container::after {
  position: absolute;
  content: "";
  width: 100%;
  height: 100%;
  background: hsla(277, 64%, 40%, 0.6);
  top: 0;
  left: 0;
}

    this should do the trick.

    You might wanna work on the accessibility and responsiveness(mobile devices). For further help feel free to comment or view my solution

    Anyway, keep up the good work and have a good day : )

    Marked as helpful
    0
  • ROCKY BARUA•1,090
    @DrougnovPosted about 2 years ago

    Hey @caiomiyaji, good work. The card looks awesome.

    You can fix the accessibility issue by changing the h2 to h1. It is the best practice to have a level 1 header for a well-designed page.

    Here is a blog about the details if you wanna know more.

    Marked as helpful
    1
  • ROCKY BARUA•1,090
    @DrougnovPosted over 2 years ago

    Hello @pharaohmak, good job on the design. Still, you can make a lot of improvements to your codes. Here are some suggestions for you:

    • Your HTML isn't semantic. To fix that you can use some tags like header main section footer instead of div.
    • The card should be in the center of the page. Add justify-items: center on the container to move it to the center.
    • The background image is not aligned well. To fix it use 
       hsl(225, 100%, 94%);
background-size: contain;
background-position-y: -15%; ```

    on the body.

    • You have only downloaded font-weight 200. That's why the heading isn't bold. Re-download the font or use this @import url("https://fonts.googleapis.com/css2?family=Red+Hat+Display:wght@500;700;900&display=swap"); in your CSS.

    Have a good day my friend :)

    Marked as helpful
    0
  • ROCKY BARUA•1,090
    @DrougnovPosted almost 3 years ago

    Hello @RikkaaDev, great design. As you asked, yes, it is actually a bad practice. We should only put the logo and nav (if needed) inside the header. The actual content should be wrapped around the main tag. If you want to explore more here is a blog about when to use header

    The design looks great but the image looks a little bit small on mobile devices. Use width:100% with media queries to give it full width.

    Again, great job. Have a good day my friend :)

    Marked as helpful
    0
  • ROCKY BARUA•1,090
    @DrougnovPosted almost 3 years ago

    Hello @devenyam, great job. The design looks pretty good.

    However, you can improve on the responsiveness part(especially on laptop devices).

    • Use max-width: 320px(any size) instead of width. It will look more flexible. You'll not even need a media query then.

    • There are some accessibility issues on the Html part. Use main, footer, section etc. instead of div. And use at least one h1 tag on the page. You can change the h2 to h1.

    Overall, great design. Keep it up. Have a good day my friend :)

    Marked as helpful
    0
  • ROCKY BARUA•1,090
    @DrougnovPosted almost 3 years ago

    Hello @NicolasGula, you did a marvelous job on the design. It's perfect.

    However, you need to improve your Html code's accessiblity. Change/Add these things:

    • Use main, footer, section, article etc. instead of div.
    • Use responsive unit like rem, em instead of px
    • You need atleast one heading in your page. You can change the <p class="title"> to h1 class="title"

    Overall, the design looks great. Keep it up. Have a good day, my friend :)

    Marked as helpful
    1
  • ROCKY BARUA•1,090
    @DrougnovPosted almost 3 years ago

    Hello @cyberspatial, great job. You can add the patterns by using pseudo-classes(:: before,:: after) to the body. Try this:

      body{
  position:relative
  }

  body::before {
  background: url(./images/bg-pattern-top.svg) no-repeat bottom right;
  top: 0;
  left: 0;
}

body::after {
  background: url(./images/bg-pattern-bottom.svg) no-repeat top left;
  top: 100%;
  left: 100%;
}

    It's probably the most efficient way. Also here are something you might wanna consider to change/add:

    • The card looks much wider than the actual design. So add max-width to the card to fix that.
    • You need to start with level one heading(h1). That means you cannot start with lower(h2,h3, etc). Don't use the section or article tag if there is no child heading. Use div instead.

    Overall your design looks great. Have a great day :)

    Marked as helpful
    2
  • Malick Roungou•470
    @mroungouSubmitted almost 3 years ago

    Hi, this is my second challenge on Front-end Mentor! Overall I think my implementation is close to the design. I wasn't able to add the overlay to the button nor "change" one you hovered on them. I'd appreciate a few pointers as to how I could implement that.

    Also, though my code works, I'm sure it is not the most efficient/best way to go about things. I would also appreciate a few comments as to how I could make it better. Thanks ;)

    Responsive order summary card

    1
    ROCKY BARUA•1,090
    @DrougnovPosted almost 3 years ago

    Hello @mroungou, Great job on the implementation.

    Although, here are some things you can improve.

    • Your hover effect works fine both on the change link and the button but you need to change the color too. So add color: hsl(245, 75%, 62%); and background-color: hsl(245, 75%, 62%); respectively in the hover rules.
    • You need to add a background pattern as the design required. Use background: url(./images/pattern-background-desktop.svg) no-repeat, hsl(225, 100%, 94%);
    • Use max-width instead of width and rem instead of px for responsiveness. Then you wouldn't even need media queries for the card.
    • To fix accessibility issues, use header main footer etc. instead of div. And make sure to use at least one heading on a page.
    • Change the h3 to h1 as you need at least one level one heading on a page.

    If you need more information or help, feel free to ask or check out my solution. Have a good day my friend :)

    Marked as helpful
    1