Spruce - Ngozi Amaefule #74
Conversation
| @@ -12,7 +12,6 @@ | |||
| "plugin:react/recommended", | |||
| "plugin:jsx-a11y/recommended", | |||
| "plugin:react-hooks/recommended", | |||
| "plugin:jest/recommended", | |||
There was a problem hiding this comment.
I assume jest was removed from VS Code because it was running tests with every change right?
We actually turn off auto run by adding the key-value pair to our settings.json in VS Code:
"jest.autoRun": "off"
| // Wave 2 | ||
| // You will need to create a method to change the square | ||
| // When it is clicked on. | ||
| // Then pass it into the squares as a callback | ||
|
|
||
| const onClickSquare = (id) => { | ||
| console.log(id); |
There was a problem hiding this comment.
When pushing code to be reviewed, we should eliminate any print statements or console logs.
| console.log(id); |
| // Wave 2 | ||
| // You will need to create a method to change the square | ||
| // When it is clicked on. | ||
| // Then pass it into the squares as a callback | ||
|
|
||
| const onClickSquare = (id) => { | ||
| console.log(id); | ||
| const newSquares = [...squares]; |
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.
| const onClickSquare = (id) => { | ||
| console.log(id); | ||
| const newSquares = [...squares]; | ||
| if (checkForWinner() === null) { |
There was a problem hiding this comment.
The ! (not) operator can also be used to check if a value is falsy
| if (checkForWinner() === null) { | |
| if (!checkForWinner()) { |
| if (currentPlayer === Player1) { | ||
| setCurrentPlayer(Player2); | ||
| } else { | ||
| setCurrentPlayer(Player1); | ||
| } |
There was a problem hiding this comment.
We can use a ternary operator to set the currentPlayer:
| if (currentPlayer === Player1) { | |
| setCurrentPlayer(Player2); | |
| } else { | |
| setCurrentPlayer(Player1); | |
| } | |
| setCurrentPlayer(currentPlayer === PLAYER_1 ? PLAYER_2 : PLAYER_1; | |
| const Square = (props) => { | ||
| // For Wave 1 enable this | ||
| // Component to alert a parent | ||
| // For Wave 1 enable this | ||
| // Component to alert a parent | ||
| // component when it's clicked on. | ||
| const clickHandler = (e) => { | ||
| e.preventDefault(); | ||
| props.onClickCallback(props.id); | ||
| }; | ||
|
|
||
| return <button | ||
| className="square" | ||
| > | ||
| {props.value} | ||
| </button> | ||
| } | ||
| return ( | ||
| <button className="square" onClick={clickHandler}> | ||
| {props.value} | ||
| </button> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
We could use object destructuring to reference prop data by their key name instead.
const Square = ( {value, id, onClickCallback, key} ) => {
const clickHandler = (e) => {
e.preventDefault();
onClickCallback(id);
};
return (
<button className="square" onClick={clickHandler}>
{value}
</button>
);
No description provided.