All comments

• 

  Olu Marshal• 20

  @Olumarshal

  Submitted over 2 years ago

  Please kindly take a look at my solution and give some feedbacks, advice or tips. I encountered some difficulties trying to make it responsive on different screen sizes but i'm not sure if i did a great job. please feel free to share your thoughts and opinions as it will help improve my skills. Thanks

  

  QR Code Component Solution using HTML and plain CSS

  1

  50

  

  Jorge• 130

  @JorgeIturrieta

  Posted over 2 years ago

  Hi MarShal you have done a great job! Let me give you some advices.

  • There are many ways to center a element. One of them (easy way) is in your body element:

      display: flex;
      min-height: 100vh;
      justify-content: center;
      align-items: center;
  

  And Remove this code of your .main-cnt

  position: absolute;
      left: 50%;
      top: 50%;
      transform: translate(-50%, -50%);
       height: 475px
  

  • You don't need add height in your card (main-cnt class). This element grow up when you put content inside.

  You can learn more about flex Box here : Concepts of flexbox And if you want, you can learn about flexbox playing a game: Flexboxgame

  Marked as helpful

  0

  This comment has received 1 replyShow
• 

  Adrianna Thomas• 70

  @adrianna-thomas

  Submitted over 2 years ago

  Did I use the best practice for my functions? I selected by element ID, should I have used classes instead or querySelectorAll?

  

  Interactive Rating Component

  1

  20

  

  Jorge• 130

  @JorgeIturrieta

  Posted over 2 years ago

  Hi Adrianna!! Good Job! Offcourse you can selected by element id.

  A little detail about your code :

      button.addEventListener("click", clickRatingBtn);  
   submitRatingBtn.addEventListener("click", ratingSubmit);
  });
  

  In every loop of forEach you are assigning the same event for submitRatingBtn element : submitRatingBtn.addEventListener("click", ratingSubmit);

  So you can move this line and put it outside

      button.addEventListener("click", clickRatingBtn); 
  });
   submitRatingBtn.addEventListener("click", ratingSubmit);
  

  On the other hand the card must be centered.

  You can do that in your main tag instead container class.

       display: flex;
       justify-content: center;
       align-items: center;
       min-height: 100vh;
  

  I hope you are well and keep it up !

  0
• 

  Mahmoud Mohamed Abd Elaziz• 470

  @ehoda9

  Submitted over 2 years ago

  I'm still learning JS, any feedback on js code ?? I focus on javascript .. lazy to finish CSS :D

  

  Javascript

  1

  40

  

  Jorge• 130

  @JorgeIturrieta

  Posted over 2 years ago

  Hi Mahmoud ! Your Javascript code looks good but i think you forgot when an item is active and the user clicks on it, it should close A possible solution to do tath is:

  const tabsLi = document.querySelectorAll(".section");
  
  function handleToggle() {
    let firstChild = this.firstElementChild;
    let lastChild = this.lastElementChild;
  
    // Toggle Action
    if (lastChild.classList.contains("show")) {
      firstChild.classList.remove("active");
      lastChild.classList.remove("show");
    } else {
      lastChild.classList.add("show");
      firstChild.classList.add("active");
    }
  }
  tabsLi.forEach((tab) => tab.addEventListener("click", handleToggle));
  

  This challenge can be done without JavaScript. You can check my solution if you want. FAQ accodion card

  I hope I've helped you and keep it up!

  Marked as helpful

  1