First time EVER using javascript did alot of youtubing

Community feedback

• 

  y4rb0w•260

  @YavanhaPosted about 2 years ago

  Hello,

  The issue here is that you're declaring the same function five times. So it's the last function that remain that id used. Either use one function to handle the click or change the name of your functions.

  I would go with the first way :

  something like that:

  div class="numbers">
        <div id="click1" class="one stars" onclick="changeColor(event)">1</div>
        <div id="click2" class="two stars" onclick="changeColor(event)">2</div>
        <div id="click3" class="three stars" onclick="changeColor(event)">3</div>
        <div id="click4" class="four stars" onclick="changeColor(event)">4</div>
        <div id="click5" class="five stars" onclick="changeColor(event)">5</div>
      </div>
  

  let currentNumber = undefined;
  
  
  function changeColor(event) {
           if(currentNumbers) {
                  currentNumber.classLists.remove("clicked");
           }
           event.target.classList.add("clicked");
           event.stopPropagation();
           currentNumber = event.target;
    }
  

  so here you add the clicked class to the element that triggerd the click (event.target) then you stop the propagation of the event (no other element will catch the click ) and finally you keep a soft copy of the element that is currently clicked to reset it later;

  hopefully it helps

  Marked as helpful

  0
• 

  T

  Mike Hayden•1,005

  @mickygingerPosted about 2 years ago

  Hey Mauger,

  This looks great! I think @Yavanha has left some great feedback above, but just to reiterate the most salient point: when you name a function you will overwrite an existing function of the same name:

  function hello() {
    console.log('hello');
  }
  
  function hello() {
     console.log('goodbye');
  }
  
  hello(); // this will log "goodbye" because the second function has overwritten the first
  

  There's loads of different approaches to this challenge but I think the way I would do it is to assign the textContent property of the button (ie the actual text inside the button), to a variable on click. Then you can display that in the popup.

  If you think about each HTML element as an object that you can interrogate (or get properties from), then you can write a function that asks the button that was clicked what its text content is and then append that to the popup. Something like this:

  <div class="numbers">
    <button>1</button>
    <button>2</button>
    <button>3</button>
    <button>4</button>
    <button>5</button>
  </div>
  <section class="popup">
    <div class="result">You selected ? out of 5</div>
  </section>
  

  const buttons = document.querySelectorAll('button'); // get ALL the buttons in one go
  const popup = document.querySelector('.popup');
  const result = document.querySelector('.result');
  
  function handleClick() {
    const selected = this.textContent // get the text content of the button that was clicked
    this.classList.add('clicked'); // `this` refers to the button that was clicked
    result.textContent = `You selected ${selected} out of 5` // update the text of the popup with the selected amount
    popup.classList.add('open-popup');
  }
  
  buttons.forEach(function(button) {
    button.addEventListener('click', handleClick); // assign the click handler to each button
  }
  

  I think this approach also means you can simplify your css a little. Here's a little more info on this in JavaScript. It's kinda nuanced and weird but very powerful! Don't expect to get it straight away, but simply put it refers to the thing that called the function. So for click events the button that was clicked, for scroll events the window that was scrolled, for keyup events, the key that was depressed...

  Oh, also you should use button and not div for the buttons. Firstly for semantic reasons but also for accessibility.

  Hopefully that helps. Let me know if you have any questions :)

  1

  ---

  

  y4rb0w•260

  @YavanhaPosted about 2 years ago

  @mickyginger Well explained and thank you for the references :)

  0
• 

  y4rb0w•260

  @YavanhaPosted about 2 years ago

  For the second part with the the Thank you card, you need to inject it dynamically through javascript when tu the "form" is submitted.

  I invite you you check out my solution :y4rb0w solution

  It's not the best one, but still I think you can find some answers :)

  If you need more in depth understading, feel free to reply :)

  0
• 

  Mauger•210

  @mauger1998Posted about 2 years ago

  thankyou I have now fixed my problem, I still cannot get the message on the pop up to tell you how many stars you have selected though and the pop up is still coming up in a different place to where the card was depending on the screen size

  0