Spruce - Ngozi Amaefule

[ngoziamaefule]

No description provided.

[audreyandoy]

Great work Ngozi! You met all the learnings goals and passed the tests for this project.

I added ways to refactor and other approaches in your PR.

Keep up the great work!

Project Grade: 🟢

[audreyandoy]

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"

[audreyandoy]

When pushing code to be reviewed, we should eliminate any print statements or console logs.

Suggested change

console.log(id);

[audreyandoy]

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.

[audreyandoy]

The ! (not) operator can also be used to check if a value is falsy

Suggested change

	if (checkForWinner() === null) {
	if (!checkForWinner()) {

[audreyandoy]

We can use a ternary operator to set the currentPlayer:

Suggested change

	if (currentPlayer === Player1) {
	setCurrentPlayer(Player2);
	} else {
	setCurrentPlayer(Player1);
	}
	setCurrentPlayer(currentPlayer === PLAYER_1 ? PLAYER_2 : PLAYER_1;

[audreyandoy]

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>
  );