Skip to content
OverviewSolutions
26
Comments
30
Loai Esam
@LoaiEsam37

All comments

  • Loai Esam•670
    @LoaiEsam37

    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
  • Loai Esam•670
    @LoaiEsam37

    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
  • Loai Esam•670
    @LoaiEsam37

    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!

  • Loai Esam•670
    @LoaiEsam37

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

  • Loai Esam•670
    @LoaiEsam37

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

  • Loai Esam•670
    @LoaiEsam37

    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 !!!

  • Loai Esam•670
    @LoaiEsam37

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

  • Loai Esam•670
    @LoaiEsam37

    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 !!!

  • Loai Esam•670
    @LoaiEsam37

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

    happy coding !!!

  • Loai Esam•670
    @LoaiEsam37

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

    Marked as helpful
  • Loai Esam•670
    @LoaiEsam37

    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 !!!

  • Loai Esam•670
    @LoaiEsam37

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

    Marked as helpful
  • Loai Esam•670
    @LoaiEsam37

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

  • Loai Esam•670
    @LoaiEsam37

    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/

  • Loai Esam•670
    @LoaiEsam37

    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 😊

  • Loai Esam•670
    @LoaiEsam37

    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
  • Loai Esam•670
    @LoaiEsam37

    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.

  • Loai Esam•670
    @LoaiEsam37

    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