Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

All comments

  • @cnsacramento

    Submitted

    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?
    ROCKY BARUA 1,090

    @Drougnov

    Posted

    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

    @ilvdrskn

    Submitted

    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?

    ROCKY BARUA 1,090

    @Drougnov

    Posted

    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

    @Drougnov

    Posted

    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

    @cenkderman

    Submitted

    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.

    ROCKY BARUA 1,090

    @Drougnov

    Posted

    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,[email protected],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

    @Drougnov

    Posted

    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

    @Drougnov

    Posted

    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

    @Drougnov

    Posted

    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

    @Drougnov

    Posted

    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

    @Drougnov

    Posted

    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

    @Drougnov

    Posted

    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

    @Drougnov

    Posted

    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
  • P

    @mroungou

    Submitted

    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 ;)

    ROCKY BARUA 1,090

    @Drougnov

    Posted

    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
  • Dom B 190

    @dombrga

    Submitted

    This is a mobile-first approach. Any comments about how I implemented this and ways for improvement is highly appreciated. Also on the error message, if any has a suggestion or what is their own way of doing this, feel free to share as well. Thank you!

    ROCKY BARUA 1,090

    @Drougnov

    Posted

    Hello @dombrga, Great job. I like how you added different error messages for empty and invalid emails. And good job on the responsive part too.

    However here are something you might wanna consider:

    • The Facebook icon doesn't match the size of the other icons. Add some padding to it.

    • To fix accessibility issues, add aria-label='something like this' to those a tags.

    • To fix Html issues, use div instead of section. Cause you need headings while using section or article tags.

    Overall, it's looking great. And it's my favorite solution to this challenge. Have a good day :)

    0
  • @catherineisonline

    Submitted

    Hello, Frontend Mentor community! This is my solution to the Huddle landing page with a single introductory section.

    I have read all the feedback on this project and improved my code. Due to the fact that I published this project very long ago, I am no longer updating it and changing its status to Public Archive on my Github.

    You are still free to download or use the code for reference in your projects but I longer update it or accept any feedback.

    Thank you

    ROCKY BARUA 1,090

    @Drougnov

    Posted

    Hi @catherineisonline, great job with the layout and responsiveness.

    However, here's something you may consider:

    -The background pattern isn't flexible enough. Use background-size: cover to fix it. -The Facebook icon doesn't look good. Add some padding to both sides.

    Overall, great solution. Happy coding :)

    1
  • @Pat-Gekoski

    Submitted

    I couldn't decide if I should let the content dictate the size of the card or if I should hard code it. I decided to hard-code the width and then use padding to account for the height.

    Still a little confused though....I added padding to the main container that the card sits in and even though I hard-coded the width, it got smaller as the viewport shrank....Does the padding force this?

    BEM and SASS

    #bem#sass/scss

    2

    ROCKY BARUA 1,090

    @Drougnov

    Posted

    Hello, @Pat-Gekoski great work.

    Just use max-width:111rem instead of width:111rem. It will make the card shrink as the screen width shrinks.

    Marked as helpful

    0
  • ROCKY BARUA 1,090

    @Drougnov

    Posted

    Hello @ayushv45, Great job. It's looking good.

    However, here are something you can do to make it even better:

    • The pattern doesn't look good in 375px or lower width devices. Use a different background image for mobile devices using media queries. It is provided in the images folder.

    • Use the blue-ish color on the headings as the design requires.

    • Try to avoid div whenever you can. Use main directly on the container class and the footer on the attribution class. It's better for accessibility.

    Good luck with the next solution and Happy coding :)

    Marked as helpful

    1
  • ROCKY BARUA 1,090

    @Drougnov

    Posted

    @dombrga yes sir, it is close enough. You did a marvelous job.

    Everything is perfect except the background pattern. It's not flexible enough for bigger screen. You can fix it by adding background-size: contain.

    Change the h2 to h1 to make your code accessible.

    Plus you need to add a different pattern for mobile devices. But the design is pretty good. Great job. Happy coding :)

    0
  • ROCKY BARUA 1,090

    @Drougnov

    Posted

    Great job @SachinShelke7. It looks wonderful.

    Just lower the opacity of the hover color a little bit.

    1
  • Herson 220

    @Hersonmei

    Submitted

    Hi,

    This is my first challenge that it took me a while to build and get to finish.

    I'm a beginner and I don't have much knowledge about good practices. If you can evaluate my code and give you some initial good practice tips I would be very grateful.

    I also had difficulty in the font-weight of the title that didn't match the model despite having bold. How can I fix this?

    Thank you very much!

    ROCKY BARUA 1,090

    @Drougnov

    Posted

    Hello, @Hersonmei Good job with the layout.

    I think you selected only 'font-weight=300' from the google font. I'm not sure though. You may check that out.

    Marked as helpful

    2
  • ROCKY BARUA 1,090

    @Drougnov

    Posted

    Hello, @SachinShelke7 great job.👏

    You can make it perfect by merging the container in the middle of the screen. And the design doesn't look good on laptop devices. So you might wanna change that.

    But still, you did a great job. Have a good day and happy coding :)

    Marked as helpful

    0
  • arash 280

    @deadazix

    Submitted

    i cant set background like what challenge asked :(

    ROCKY BARUA 1,090

    @Drougnov

    Posted

    Hello, @deadazix good job.

    To set those patterns you can use pseudo-elements like before, after. Use this code:

    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%;
    }
    

    And some other things:

    • To make it accessible try not to use the div tag as much as you can. You can use the main tag to the container.
    • Try to implement the design as accurately as possible. Such as using the font family, font size, etc. as they asked to use.

    Happy coding and have a good day. ♥

    Marked as helpful

    1
  • Tushar Biswas 4,100

    @itush

    Submitted

    I enjoyed the challenge very much.

    I'd really appreciate if you could answer the following:

    1. What did I do wrong?
    2. What did I do right?
    3. How can I improve?

    Thanks in advance :)

    ROCKY BARUA 1,090

    @Drougnov

    Posted

    Hello, @itush. Nicely done. Here are something you may reconsider:

    • To make your HTML accessible don't use unnecessary divs. You can use main, section, article etc. instead of div.

    • Your background pattern doesn't work well on the big screen. You can use background-size: contain to make it fit the width.

    • On the card item the box-shadow color doesn't fit well. Try using black color with low opacity.

    Happy coding and have a nice day.❤

    2
  • ROCKY BARUA 1,090

    @Drougnov

    Posted

    Hello Kamasah👋, great job with the nav and header 👏.

    The main section is also good but you need to add some padding on the left of the main coloumns as the design requires.

    The footer logo isn't visible due to its color. To make it white, you can use "filter: invert(100%) sepia(0%) saturate(0%) hue-rotate(308deg) brightness(108%) contrast(108%)" on the image.

    And about the footer section, the flexbox isn't that flexible cause you have to add width to every row. You can try grid if you want. Here is my solution. It isn't that great but it might help you out a bit.

    Happy coding❤

    0
  • ROCKY BARUA 1,090

    @Drougnov

    Posted

    Thank you so much. Gonna add it now.

    0