Question 204784 in Html Question posted in
Some basic documentation can be found here.

Html – button text color not changing when I place the script inside another function

I want to change the text color of a button when I click on it using a function.
so I made a function for it which does not work.

code:

function correct(x) {
  document.getElementById("x").style.color = "green";
}

function wrong(y) {
  document.getElementById("y").style.color = "red";
}

function answer() {
  if (document.getElementById("p").innerHTML == "question 1") {
    correct(bt1);
    wrong(bt2);
  } else if (document.getElementById("p").innerHTML == "question 2") {
    correct(bt2);
    wrong(bt1);
  }
}
<p id="p">question 1</p>

<button id="bt1" onclick="answer()"> option 1</button>
<button id="bt2" onclick="answer()"> option 2</button>
Tags:

6

Answers


  1. 0

    You have your quotes backwards: the arguments to correct and wrong should be quoted; the use of their parameters in those functions should not.

    Login or Signup to reply.
  2. 0

    I have added comments to your code directly. It seems that you may have put string markers at the wrong place

       <script>
    function correct(x) {
      // you are looking for a string "x" instead of the id represented by x
      document.getElementById("x").style.color = "green";
    }
    
    function wrong(y) {
      // same problem
      document.getElementById("y").style.color = "red";
    }
    
    function answer() {
      if (document.getElementById("p").innerHTML == "question 1") {
        // correct("bt1");
        correct(bt1);
        // wrong("bt2");
        wrong(bt2);
      } else if (document.getElementById("p").innerHTML == "question 2") {
        // correct("bt2");
        correct(bt2);
        // wrong("bt1");
        wrong(bt1);
      }
    }
    </script>
    Login or Signup to reply.
  3. 0

    You don’t need to call getElementById() to get the buttons to change. answer() passes the buttons directly, use the parameters.

    function correct(x) {
  x.style.color = "green";
}

function wrong(y) {
  y.style.color = "red";
}

function answer() {
  if (document.getElementById("p").innerHTML == "question 1") {
    correct(bt1);
    wrong(bt2);
  } else if (document.getElementById("p").innerHTML == "question 2") {
    correct(bt2);
    wrong(bt1);
  }
}
    <p id="p">question 1</p>

<button id="bt1" onclick="answer()"> option 1</button>
<button id="bt2" onclick="answer()"> option 2</button>

    Note that use bt1 and bt2 as arguments takes advantage of the fact that element IDs automatically become global variables. This is not generally considered good style.

    Login or Signup to reply.
  4. 0

    Since you are passing the button into the function but not using it, just change it to the following:

    function correct(element) {
  element.style.color = "green";
}

function wrong(element) {
  element.style.color = "red";
}

function answer() {
  if (document.getElementById("p").textContent === "question 1") {
    correct(bt1);
    wrong(bt2);
  } else if (document.getElementById("p").textContent === "question 2") {
    correct(bt2);
    wrong(bt1);
  }
}
    <p id="p">question 1</p>

<button id="bt1" onclick="answer()"> option 1</button>
<button id="bt2" onclick="answer()"> option 2</button>

    A better approach would be to use classes instead of IDs.

    function correct(element) {
  element.style.color = "green";
}

function wrong(element) {
  element.style.color = "red";
}

function answer(event) {
  const question = event.target.closest('.question');
  const prompt = question.querySelector('.prompt');
  if (prompt.textContent === "Question 1") {
    correct(question.querySelector('.btn:nth-of-type(1)'));
    wrong(question.querySelector('.btn:nth-of-type(2)'));
  } else if (prompt.textContent === "Question 2") {
    wrong(question.querySelector('.btn:nth-of-type(1)'));
    correct(question.querySelector('.btn:nth-of-type(2)'));
  }
}
    <div class="question">
  <p class="prompt">Question 1</p>
  <button class="btn" onclick="answer(event)">Option 1</button>
  <button class="btn" onclick="answer(event)">Option 2</button>
</div>

<div class="question">
  <p class="prompt">Question 2</p>
  <button class="btn" onclick="answer(event)">Option 1</button>
  <button class="btn" onclick="answer(event)">Option 2</button>
</div>
    Login or Signup to reply.
  5. 0

    Firstly, your correct() and wrong() functions are trying to get elements with ID "x" and "y" respectively, but what i think you wanted is obtain the id from the function parameters.
    Secondly, you’re sending to the correct() and wrong() functions, the IDs of the buttons as bt1 and bt2, but these are undefined, you should send them as strings "bt1", "bt2".

    <p id="p">question 1</p>

<button id="bt1" onclick="answer()"> option 1</button>
<button id="bt2" onclick="answer()"> option 2</button>

<script>
function correct(id) {
  document.getElementById(id).style.color = "green";
}

function wrong(id) {
  document.getElementById(id).style.color = "red";
}

function answer() {
  if (document.getElementById("p").innerHTML == "question 1") {
    correct("bt1");
    wrong("bt2");
  } else if (document.getElementById("p").innerHTML == "question 2") {
    correct("bt2");
    wrong("bt1");
  }
}
</script>
    Login or Signup to reply.
  6. 0

    Others have already pointed out that your variables and the use of quotes is what is causing your problem, but beyond that, you really don’t need and shouldn’t be setting up dedicated functions just to change the color. Instead, you should set up CSS classes that define the possible "looks" you want and then simply add or remove the classes as needed.

    Additionally, buttons aren’t really a good choice for answers to questions. This is what checkboxes and radio buttons are for. But, you can hide the checkboxes or radio buttons and use label elements to wrap them and show the labels, styled as buttons. This will give you the mutual exclusivity (in the case of radio buttons) that you desire.

    // Set up your events in JavaScript, not HTML
// Create one event handler that will be triggered by a click
// anywhere within the "question" div. That click will bubble up
// and be handled there.

document.addEventListener("click", function(event){
  // Check to see if the click originated at an element we care to handle
  if(event.target.classList.contains("response")){
    // Clear any previous classes
    event.target.className = "";
    // Check to see which answer was chosen
    if(event.target.textContent === "3.14"){
      event.target.classList.add("green");
    } else {
      event.target.classList.add("red");   
    }
  }
});
    /* Hide the radio buttons (but the labels for the buttons will still be visible  */
input[type="radio"] { display:none; }

/* Make the labels look like buttons */
label { border:1px solid #808080; border-radius:3px; padding:5px; background:#e0e0e0; }
label:active { background:#a0a0a0; }

/* Colors */
.red { background:red; }
.green { background:green; }
    <div class="question">
  <p>Which number below represents Pi?</p>
  <label class="response"><input type="radio" name="question1">4.13</button></label>
  <label  class="response"><input type="radio" name="question1">3.14</button></label>
</div>
    Login or Signup to reply.
Please signup or login to give your own answer.

Back To Top
Search