Alf - Spruce #71
Alf - Spruce #71
Conversation
…t the current player, and 'updateSquares' that updates the game state to mark the squares the have been clicked by the respective players
…ked on square to the onClickCallback function in app
…1 instead of col +=1, so it was looping forever git add .! dumb dummy
…e, and stop the clicks on the board if a winner is found
…sage depending on if the game is in progress, or if someone wins
There was a problem hiding this comment.
Great work Alf! You met all the learnings goals for this project, passed most of the tests, and I loved the CSS additions to your project!
The only tests you were failing had to do with a minor string mismatch using uppercase 'X' and 'O' rather than lowercase 'x' and 'o''s like the tests were expecting. No biggie and an easy fix.
In your PR, you'll find comments containing compliments and ways to refactor.
Project Grade: 🟢
| const player1 = 'X'; | ||
| const player2 = 'O'; |
There was a problem hiding this comment.
Annoyingly enough, our tests look for lowercase 'x' and 'o', which was our fault in not updating our tests.
This was the reason why it appeared you failed tests, despite your project working as expected.
| // Then pass it into the squares as a callback | ||
| const [isX, setIsX] = useState(false); | ||
|
|
||
| const currentPlayer = !isX ? player1 : player2; |
| let newSquares = []; | ||
|
|
||
| for (let row = 0; row < 3; row += 1) { | ||
| newSquares.push([]); | ||
| for (let col = 0; col < 3; col += 1) { | ||
| if (id === squares[row][col].id) { | ||
| if (!squares[row][col].value) { | ||
| squares[row][col].value = currentPlayer; | ||
| setIsX(!isX); | ||
| } | ||
| } | ||
| newSquares[row].push(squares[row][col]); | ||
| } | ||
| } | ||
| setSquares(newSquares); | ||
| }; |
There was a problem hiding this comment.
Great job in making a copy of the squares data! Here's a reminder on why we need to make the copy:
To trigger the update stage for this component, React needs to notice a change in state data. We could mutate state directly, however, this doesn't scale well and doesn't follow React best practices.
Instead, we should pass in a copy of our state data to the setState function setSquares() which will have React compare the object references between the copy and the old state data. If the object references are different, React will then trigger the update stage.
There was a problem hiding this comment.
Rather than creating an empty list and pushing values into it with every iteration, we can make a shallow copy with the ... spread operator:
| let newSquares = []; | |
| for (let row = 0; row < 3; row += 1) { | |
| newSquares.push([]); | |
| for (let col = 0; col < 3; col += 1) { | |
| if (id === squares[row][col].id) { | |
| if (!squares[row][col].value) { | |
| squares[row][col].value = currentPlayer; | |
| setIsX(!isX); | |
| } | |
| } | |
| newSquares[row].push(squares[row][col]); | |
| } | |
| } | |
| setSquares(newSquares); | |
| }; | |
| let newSquares = [...squares]; | |
| for (let row = 0; row < 3; row += 1) { | |
| for (let col = 0; col < 3; col += 1) { | |
| if (id === squares[row][col].id && !squares[row][col].value) { | |
| squares[row][col].value = currentPlayer; | |
| setIsX(!isX); | |
| } | |
| } | |
| } | |
| setSquares(newSquares); | |
| }; |
There was a problem hiding this comment.
I've also combined the conditional to reduce how nested this section of code looks.
| }; | ||
|
|
||
| const headerMessage = checkForWinner() ? `The winner is ${checkForWinner()}!!!` : `Current player: ${currentPlayer}`; |
| </header> | ||
| <main> | ||
| <Board squares={squares} /> | ||
| <Board squares={squares} onClickCallback={checkForWinner() ? () => {} : updateSquare} /> |
There was a problem hiding this comment.
Clever use of a ternary operator to change the functionality of the click.
| @@ -1,12 +1,33 @@ | |||
| .square | |||
| { | |||
| border: 4px solid #2c3e50; | |||
| border: 3px solid black; | |||
There was a problem hiding this comment.
LOVE the css hover effects! The overall color palette is super cute and portfolio ready ✨
| if (props.id === 0 || props.id === 5) { | ||
| color = 'blue'; | ||
| } else if (props.id === 1 || props.id === 6) { | ||
| color = 'yellow'; | ||
| } else if (props.id === 2 || props.id === 7) { | ||
| color = 'green'; | ||
| } else if (props.id === 3 || props.id === 8) { | ||
| color = 'pink'; | ||
| } else if (props.id === 4) { | ||
| color = 'orange'; | ||
| } |
There was a problem hiding this comment.
Great use of the || or operator to determine the color of the hover effects!
| /> | ||
| ); | ||
| }); | ||
| }; | ||
|
|
||
| const Board = ({ squares, onClickCallback }) => { | ||
| const squareList = generateSquareComponents(squares, onClickCallback); | ||
| console.log(squareList); |
There was a problem hiding this comment.
Generally, we should remove all console logs and print statements when submitting code for review. And our team should definitely take that advice as well, I believe this was meant to be removed before shipping to y'all.
| console.log(squareList); |
No description provided.