Search code examples
javascripthtmlcss

Button doesn't lock when I try to change the value to true


I'm back with an issue with my buttons value not changing from false to true when clicked. When clicked the button's text is suppose to be changed with "X" or "O" depending on the players value. And when the button is clicked for the first time, it is suppose to change the buttons current value from "false" to "true". Locking the button from being clickable.

let player = 0;

function changeX(buttonType){
    let but = buttonType;
    if (buttonType.value !== "true"){
        if (player === 0){
            document.querySelector(but).textContent = "X";
            player = player + 1;
            but.value = "true";
        }
        else if (player === 1){
            document.querySelector(but).textContent = "O";
            player = player - player;
            but.value = "true";
        }
    }
}
body {
    margin: 0;
}

#header {
    background-color: black;
    color: white;
    font-size: 40px;
    height: 50px;
    text-align: center;
}

.container1 {
    margin: 200px auto;
    justify-content: center;
    display: flex;
    flex-wrap: wrap;
    width: 450px;
}

.but1 {
    font-size: 40px;
    height: 150px;
    width: 150px;
    background-color: transparent;
    border-top: none;
    border-left: none;
    border-right: 1px solid black;
    border-bottom: 1px solid black;
}

.but1:hover {
    background-color: rgb(245, 245, 245);
}

.but2 {
    font-size: 40px;
    height: 150px;
    width: 150px;
    background-color: transparent;
    border-top: none;
    border-left: 1px solid black;
    border-right: 1px solid black;
    border-bottom: 1px solid black;
}

.but2:hover {
    background-color: rgb(245, 245, 245);
}

.but3 {
    font-size: 40px;
    height: 150px;
    width: 150px;
    background-color: transparent;
    border-top: none;
    border-left: 1px solid black;
    border-right: none;
    border-bottom: 1px solid black;
}

.but3:hover {
    background-color: rgb(245, 245, 245);
}

.but4 {
    font-size: 40px;
    height: 150px;
    width: 150px;
    background-color: transparent;
    border-top: 1px solid black;
    border-left: none;
    border-right: 1px solid black;
    border-bottom: 1px solid black;
}

.but4:hover {
    background-color: rgb(245, 245, 245);
}

.but5 {
    font-size: 40px;
    height: 150px;
    width: 150px;
    background-color: transparent;
    border-top: 1px solid black;
    border-left: 1px solid black;
    border-right: 1px solid black;
    border-bottom: 1px solid black;
}

.but5:hover {
    background-color: rgb(245, 245, 245);
}

.but6 {
    font-size: 40px;
    height: 150px;
    width: 150px;
    background-color: transparent;
    border-top: 1px solid black;
    border-left: 1px solid black;
    border-right: none;
    border-bottom: 1px solid black;
}

.but6:hover {
    background-color: rgb(245, 245, 245);
}

.but7 {
    font-size: 40px;
    height: 150px;
    width: 150px;
    background-color: transparent;
    border-top: 1px solid black;
    border-left: none;
    border-right: 1px solid black;
    border-bottom: none;
}

.but7:hover {
    background-color: rgb(245, 245, 245);
}

.but8 {
    font-size: 40px;
    height: 150px;
    width: 150px;
    background-color: transparent;
    border-top: 1px solid black;
    border-left: 1px solid black;
    border-right: 1px solid black;
    border-bottom: none;
}

.but8:hover {
    background-color: rgb(245, 245, 245);
}

.but9 {
    font-size: 40px;
    height: 150px;
    width: 150px;
    background-color: transparent;
    border-top: 1px solid black;
    border-left: 1px solid black;
    border-right: none;
    border-bottom: none;
}

.but9:hover {
    background-color: rgb(245, 245, 245);
}
<!DOCTYPE html>
<html lang="en">
    <head>
        <title>Tic Tac Toe Project</title>
        <meta charset="UTF-8">
        <link rel="stylesheet" href="index.css">
    </head>
    <body>
        <section>
            <header id="header">
                Tic Tac Toe
            </header>
        </section>
        <section>
            <div class="container1">
                <button type="button" class="but1" onclick="changeX('.but1')" value="false">1</button>
                <button type="button" class="but2">2</button>
                <button type="button" class="but3">3</button>
                <button type="button" class="but4">4</button>
                <button type="button" class="but5">5</button>
                <button type="button" class="but6">6</button>
                <button type="button" class="but7">7</button>
                <button type="button" class="but8">8</button>
                <button type="button" class="but9">9</button>
            </div>
        </section>
        <script src="board.js"></script>
    </body>
</html>


Solution

  • There's a few issues here:

    • You've only added the onclick attribute to the first button element, so only that one will trigger your JS logic.
    • If you attach your events using JS instead of HTML, you can get a direct reference to the element via the Event object without needing to pass selectors as strings. This can also be done in a loop to attach to all buttons.
    • Use common classes to group elements by behaviour. Using incremental id or class attributes is counter-productive and not good practice.
    • Using a boolean value for the active player (as there's only 2 players) makes the logic more simple as you can simply invert the value.
    • Within your changeX function but will be a string and have no value property. You need to get a reference to the clicked element and update the value of that instead.
    • Your CSS can be simplified by applying the same style to all buttons and then removing the right/bottom borders where required.

    Here's an updated version of your code with those issues addressed:

    let isPlayer1 = true;
    const buttons = document.querySelectorAll('.but');
    const handleButtonClick = e => {
      const button = e.target;
      
      if (button.value !== "true") {
        button.textContent = isPlayer1 ? 'X' : 'O';    
        button.value = "true";
      }
      
      isPlayer1 = !isPlayer1;
    }
    
    buttons.forEach(btn => {
      btn.addEventListener('click', handleButtonClick);
    });
    body {
      margin: 0;
    }
    
    #header {
      background-color: black;
      color: white;
      font-size: 40px;
      height: 50px;
      text-align: center;
    }
    
    .container1 {
      margin: 200px auto;
      justify-content: center;
      display: flex;
      flex-wrap: wrap;
      width: 450px;
    }
    
    .but {
      font-size: 40px;
      height: 150px;
      width: 150px;
      background-color: transparent;
      border-top: none;
      border-left: none;
      border-right: 1px solid black;
      border-bottom: 1px solid black;
    }
    
    .but:hover {
      background-color: rgb(245, 245, 245);
    }
    
    .but:nth-child(3n) {
      border-right: 0;
    }
    
    .but:nth-last-child(-n + 3) {
      border-bottom: 0;
    }
    <section>
      <header id="header">
        Tic Tac Toe
      </header>
    </section>
    <section>
      <div class="container1">
        <button type="button" class="but">1</button>
        <button type="button" class="but">2</button>
        <button type="button" class="but">3</button>
        <button type="button" class="but">4</button>
        <button type="button" class="but">5</button>
        <button type="button" class="but">6</button>
        <button type="button" class="but">7</button>
        <button type="button" class="but">8</button>
        <button type="button" class="but">9</button>
      </div>
    </section>

    Also note that using the value attribute of the button element to denote which boxes have been used is a little un-semantic. I'd suggest using a data attribute for this instead, but will leave that as a exercise for the OP as the logic works as it stands.