SASS for flexible CSS, Media queries for responsive Design

Community feedback

• 

  バレンタイン 😈•64,190

  @VCaramesPosted almost 2 years ago

  Hey there! 👋 Here are some suggestions to help improve your code:

  • not only improve your HTML code but to identify the main content of you page, you will want to wrap your entire component inside the main element.

  More Info:📚

  MDN Main Element

  • The “icons/illustrations” in this component serve no other purpose than to be decorative; They add no value. There alt tag should be left blank and have an aria-hidden=“true” to hide them from assistive technology.

  More Info:📚

  https://www.w3.org/WAI/tutorials/images/

  • The proper way to build the "rating buttons" in this challenge is to create a form and inside of it, there should be fiveinput radios and each input should have a label attached to it to make the buttons accessible. Finally wrap all the inputs and labels inside a fieldset to prevent users from making more than one selection.

  More Info:📚

  MDN <input type="radio">

  MDN <fieldset>: The Field Set element

  • The attribution should be wrapped in a footer element.

  If you have any questions or need further clarification, feel free to reach out to me.

  Happy Coding!🎄🎁

  Marked as helpful

  1

  ---

  

  Dytoma•550

  @DytomaPosted almost 2 years ago

  @vcarames Thank you a lot for your tips. I will apply that to my next project.

  1
• 

  Elaine•11,400

  @elaineleungPosted almost 2 years ago

  Hi Dytoma, this looks like a great start, and I think your code works fine. I do suggest giving your classes and variables more meaning names, where instead of

  const rating = document.querySelectorAll(".nbr");
  

  ... you can try something like this:

  const ratingBtns = document.querySelectorAll(".rating");
  // note that Btns stand for buttons since there is more than 1 button here!
  

  For the for loop, I suggest using classList instead of className so that you can easily add or remove a class instead of changing the className attribute directly. I also don't think selected needs querySelectorAll since there's only one button being selected. You can try this:

  for(let i = 0; i < rating.length; i++) {
      rating[i].addEventListener("click", function() {
          let selected = document.querySelector(".selected");
          selected.classList.remove("selected")
          this.classList.add("selected");
      });
  }
  

  Lastly, instead of having a default score of 1, I would just leave it empty and let the user choose. Sometimes users might accidentally click on the submit before they select the score, and you wouldn't want them to accidentally send a score of 1! Hope this helps you out a bit 🙂

  1

  ---

  

  Dytoma•550

  @DytomaPosted almost 2 years ago

  @elaineleung Really appreciate your feedback, I've taken note and I'll apply that to my project and my next projects. Thank you very much.

  1
• 

  Adriano•38,080

  @AdrianoEscarabotePosted almost 2 years ago

  Hello Dytoma Batogouma, how are you? I truly loved your project's outcome, however I have some advice that I hope you'll find useful:

  Para fazer com que o botão de submti funcione apenas quando o usuario selecionar algum numero, podemos fazer isso;

  for(let i = 0; i < rating.length; i++) {
      rating[i].addEventListener("click", function() {
          let selected = document.querySelectorAll(".selected");
      selected[0].className = selected[0].className.replace("selected", "");
      this.className += " selected";
      submit.addEventListener("click", function() {
          let ratinNbr = document.querySelector(".selected");
          let finalRating = document.querySelector(".rating");
          let feedback = document.querySelector(".success");
          let form = document.querySelector(".active");
          form.classList.add("unactive");
          finalRating.innerHTML = ratinNbr.innerHTML;
          feedback.className = feedback.className.replace("unactive", "");
          })
      });
  }
  

  The remainder is excellent.

  I hope it's useful. 👍

  1

  ---

  

  Dytoma•550

  @DytomaPosted almost 2 years ago

  @AdrianoEscarabote Thanks Adriano, I'm gonna apply that.

  0