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

  • Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    Your solution is truly impressive and legit . Keep going !!! and i have some suggestions to make it more incredible than it is.

    Look i don`t know much about tailwindcss so i will just write css so you can get the idea

    You can add these propeties to <main> tag so that it become responsive to extra large screens :-

    main {
    max-width: 1400px;
    margin: auto;
    }
    

    and then remove the padding from the media query so that the <main> tag can take its full width:-

    @media (min-width: 768px) {
    .md\:p-40 {
    padding: 10rem; /* remove this property */
    }
    }
    

    there are more details to make it responsive for small screen sizes but to do that i will need to read the full structure of your code and this will be very painful šŸ˜…

    hope you found this helpful šŸ˜Š

    Marked as helpful

    1
  • Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    You're a true expert at coding and Your skills are truly impressive and you can make it more incredible if you use this html structure to wrap the elements and center the container

    <body>
    <main>
    <div class="container">
    <div class="wrapper">
    <div class="description">
    ...
    </div>
    
    <div class="ratings">
    ...
    </div>
      
    <div class="reviews_container">
    ...
    </div>
    </div>
    </div>
    </main>
    </body>
    

    give the <div class="container"> tag max-width: 1200px; and padding: 0 15px; and remove the padding and display grid from the main tag and give it these properties :-

    main {
    display: flex;
    justify-content: center;
    align-items: center;
    }
    

    and you can use the <div class="wrapper"> for making some padding. there are some more changes but i don`t have time to say it all šŸ˜… also you can checkout my solution for this chalange if you want to see my source code

    hope you found this helpful !!! šŸ˜Š

    Marked as helpful

    1
  • Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    Wow, your code is truly exceptional! It's so well written and efficient that I think it would be a disservice to try and review it. You've clearly put a lot of time and effort into it, and it shows. Keep up the amazing work!

    1
  • Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    You have made it pixel perfect !!! You're really talented at coding man and I think you are even better than me. Keep going !!!

    0
  • Qumrranā€¢ 370

    @qumrran

    Submitted

    I have a question for the users: I encountered a problem with applying a shadow to the semi-circular elements of individual sections. After applying the shadow underneath, I can see the div's corners in the background color. Unfortunately, the shadow didn't become rounded. Could someone please explain what's going on?

    Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    You're really talented at coding! Your solution is truly impressive. Keep going !!!

    0
  • Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    You're a true expert at coding. Your skills are truly impressive. and if you make your code fully responsive it will be incredible. note that the smallest screen size is 320 px. Keep going !!!

    0
  • Gazdik Tamasā€¢ 40

    @tamasgazdik

    Submitted

    Hi all,

    Couple of questions from my side regarding the solution:

    #1 Semantic HTML and accessibility:

    • Do you think I overdid the accessibility with all the aria-label and aria-labelledby properties on pretty much all of my elements? It kind of felt redundant providing IDs only to be used by other elements for the aria-labelledby attribute.
    • I used article elements for the two main parts of the content, because I thought they'd make sense on their own. Later I used section elements for different the attribute sections. Would you have done it differently?

    #2 CSS

    • For mobile layout I I used Flexbox for the body display style and changed it to Grid for desktop layout. Even though it looks the way I want it, it feels like a lot of extra work. Do you think the same could be achieved with less code?
    • Can you tell me an easier way to create a layout where we have the main content in exactly the middle of the page and some footer at the bottom? I used below solution, but once again it's seems too complicated for a relatively easy problem:
    body {
    display: grid;
    grid-template-rows: auto min-content auto min-content;
    justify-content: center;
    }
    

    then for main content:

    main {
    grid-row: 2 / 3;
    }
    

    and for the footer:

    footer {
    grid-row: 4 / -1;
    }
    

    As always, any feedback on the code or best practices is appreciated, but I'd be really glad if someone could answer the questions above. Thanks for the opportunity, happy coding!

    Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    You're really talented at coding! Your work is impressive. Your solution is so elegant and efficient. Keep going !!!

    1
  • Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    You're really talented at coding! You make coding look easy! Your skills are really top-notch. Your solutions is so elegant and efficient. and if you make it responsive it will be incredible to do that you can change the grid-template property in the media query to be one column only at mobile screen sizes.

    happy coding !!!

    0
  • Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    you can use input tag type range to make the range of the price

    happy coding !!!

    0
  • Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    Wow, you're a coding wizard! Your solution is so elegant and efficient. Keep going !!!

    Marked as helpful

    1
  • Karthikā€¢ 220

    @Bi-Byee

    Submitted

    Hey guys did a new component, hope it's met up with the challenge. Could anyone recommend resources to easily understand responsive design and @media query.

    Bless up, Karthik

    Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    Wow, you're a coding wizard! Your solution is so elegant and efficient. and also if you add these properties to your code it will be better.

    body {
    display: flex
    justify-content: center
    align-items: center
    }
    

    this will make you .wrapper element in the center of the page.

    happy coding !!!

    0
  • UnFikā€¢ 50

    @UnFik

    Submitted

    difficult part with this is centering the component vertically and horizontally but the solution is the parent div just need static height other than %. in this case we should've add static height to body element

    Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    Very Good keep going man. but LOL!!!, you know that you submitted the solution for the wrong chalange, right ?!?

    Marked as helpful

    0
  • Georgeā€¢ 410

    @xsova113

    Submitted

    Most difficult thing about this project for me is the drag and drop reordering.

    I use framer-motion to do reordering which makes it pretty easy but I couldn't figure out how to sync up the order to my backend (postgresql / prisma).

    also reordering list still quite buggy..

    Any tips on that is highly appreciated!

    Fullstack todo-app with nextjs / prisma

    #framer-motion#next#postgres#typescript#tailwind-css

    1

    Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    Did you make your postgres database on vercel for free or do you already have a subscription ?

    0
  • Pierre Fraisseā€¢ 330

    @PierreFrs

    Submitted

    Hi, not a so easy one, I'm trying to wrap my head around the forEach method and trying to automate a maximum and use as little query selectors and hardcoded ids. I finaly made it work but the javascript code seems to be maybe too complex for the task. And the UI is... ok I guess, not that responsive but ok. A review of my JS code especially would be welcome ;) Have a nice day everyone !

    Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    you have to make the card image content changes when the input changes checkout my project to see what i mean: https://frontend-mentor-interactive-card-details-form-blush.vercel.app/

    0
  • Petros Devrikisā€¢ 380

    @Petrosdevri

    Submitted

    This is my solution for the Age Calculator App.

    It was overall easy and enjoyable building this project, however I faced a difficulty regarding functionality:

    • I would basically like to find out how to validate the form based on day and month input. You can type in #day-input every number from 1 to 31 nevertheless there is no validation regarding month restrictions (for example you can type 31 February and it will generate an output). Is there any regex available or do I need to set specific limitations in the JS script?

    I would appreciate your feedback and would like to receive your comments and perspectives regarding the project. Thanks a lot!

    Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    cool but try to learn a framework like react or vuejs it will make your life easier and it will add security to your code (prevent XSS, html injection and many more)

    hope you found this helpful šŸ˜Š

    0
  • Petros Devrikisā€¢ 380

    @Petrosdevri

    Submitted

    This is my solution for the Interactive Rating Component.

    It was overall easy and enjoyable building this project, however I faced a difficulty regarding functionality:

    • I would like to learn how can I disable the button defined in the .submit-btn class when no value is selected. I tried adding the following snippet: if(finalRating.innerHTML === '') { submitBtn.disabled = true; } However, it blocked my button even after I had chosen a rating value between the available ones. With the current code you can see that rating values display perfectly in the .thank-you state.

    I would appreciate your feedback and would like to receive your comments and perspectives regarding the project. Thanks a lot!

    Interactive Rating Component

    #bootstrap#sass/scss

    1

    Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    i have a suggestion but i do not think it is the best but i use it in my project and i think it`s good to think out the box, you can add the button event listener inside the rating buttons event listener so that it start listening for the button when the rating buttons get clicked like this:

    const ratingState = document.getElementById("rating--state");
    const thankyouState = document.getElementById("thank__you--state");
    const submitButton = document.getElementById("submit__rating");
    const ratingSpan = document.getElementById("rating");
    const ratingButtons = document
      .getElementsByClassName("card__btns")[0]
      .getElementsByTagName("button");
    
    Object.values(ratingButtons).forEach((e) => {
      e.addEventListener("click", () => {
        ratingSpan.innerHTML = e.innerHTML;
        submitButton.addEventListener("click", () => {
          ratingState.classList.add("hidden");
          thankyouState.classList.remove("hidden");
        });
      });
    });
    

    and this is my sourcecode

    happy coding!!!

    Marked as helpful

    0
  • Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    add role main to your <div class="card"> tag like this <div class="card" role="main"> or you can just do it like this <main class="card"> and the idea here is to make it more easier for screen readers to know where is the main content and you can also use something like <nav> , <footer> and <sidebar> tags for the same reason.

    0
  • Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    add role main to your <div id="mainframe"> tag like this <div id="mainframe" role="main"> or you can just do it like this <main id="mainframe"> and the idea here is to make it more easier for screen readers to know where is the main content and you can also use something like <nav> , <footer> and <sidebar> tags for the same reason.

    hope you find this helpful

    0
  • Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    add role main to your <div class="wrapper"> tag like this <div class="wrapper" role="main"> or you can just do it like this <main class="container"> and the idea here is to make it more easier for screen readers to know where is the main content and you can also use something like <nav> , <footer> and <sidebar> tags for the same reason. also delete the <div class="attribution"> tag there is not need for it

    hope you find this helpful

    Marked as helpful

    0
  • yamenmaaniā€¢ 30

    @yamenmaani

    Submitted

    tried to finish this page in 1:30 hours

    Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    to make the perfume word become like the design you can use letter-spacing property in css to increase the space between letters in the word also this is an extra tip you can use line-height property to increase or decrease the spacing between lines

    happy coding

    0
  • yamenmaaniā€¢ 30

    @yamenmaani

    Submitted

    tried to finish this page in 1:30 hours

    Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    add role main to your <div class="container"> tag like this <div class="container" role="main"> or you can just do it like this <main class="container"> and the idea here is to make it more easier for screen readers to know where is the main content and you can also use something like <nav> , <footer> and <sidebar> tags for the same reason. also here:

          <p>perfume</p>
          <h1>Gabrielle Essence Eau De Parfum</h1>
    

    you have to make it like this

          <h1>perfume</h1>
          <p>Gabrielle Essence Eau De Parfum</p>
    

    because the page have to start with h1 and then you can change the font-size from css

    hope you find this helpful

    0
  • AdrianoAfkā€¢ 30

    @AdrianoAfk

    Submitted

    I had more difficulty with responsiveness, I couldn't make the text fit according to the screen. Any comments or tips for me to improve I would be very grateful

    Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    Hi adrianoAFK,

    (in line 21) inside index.html replace the <b> tag with <h1> tag because every page should contain level-one heading, also you can change the background-color of the page with this color #d5e1ef

    hope you find this helpful

    0
  • Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    Hi Takeo,

    add role main to your <div class="container"> tag like this <div class="container" role="main"> or you can just do it like this <main class="container"> and the idea here is to make it more easier for screen readers to know where is the main content and you can also use something like <nav> , <footer> and <sidebar> tags for the same reason.

    hope you find this helpful

    Marked as helpful

    0
  • Loai Esamā€¢ 670

    @LoaiEsam37

    Posted

    you have to put your <div class="qr__wrap"> tag inside a <main> tag so that it becomes easier for screen readers to know where is the main content and you can also use something like <nav> , <footer> and <sidebar> tags for the same reason.

    Marked as helpful

    0