Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
"plugin:react/recommended",
"plugin:jsx-a11y/recommended",
"plugin:react-hooks/recommended",
"plugin:jest/recommended",
"plugin:testing-library/react"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this line does help reduce the warnings in the test files, but notice that doing so leaves a trailing comma (line 14), which isn't permitted in JSON.

],
"parserOptions": {
"ecmaFeatures": {
Expand Down
17 changes: 15 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# React Tic-Tac-Toe

## Please Read First!

The digital campus' version of Tic-Tac-Toe differs from the original in the following ways:
- We will *not* be using `main` branch. Follow step 6 in the **One-Time Project Setup** to change branches.
- Wave 1 has been completed for you; however, it would help you understand the flow of data by reviewing the code written for Wave 1.
- Wave 3's `checkForWinner` function has been created for you; however, read through Wave 3 instructions to figure out how and where to use it.

## Skills Assessed

- Following directions and reading comprehension
Expand Down Expand Up @@ -81,9 +88,15 @@ We can run `yarn install` multiple times safely, but we only need to do this onc

The file `package.json` contains details about our project, the scripts available, and the dependencies needed. We can inspect this file when we are curious about the details of our dependencies.

6. Follow the directions in the "Getting Started" section.
6. We will not being using `main` branch. Make sure you are working from `digital-starter` by running:

```bash
$ git checkout digital-starter
```

7. Follow the directions in the "Getting Started" section.

7. Follow the directions in the "Project Requirements" section.
8. Follow the directions in the "Project Requirements" section.

## Project Development Workflow

Expand Down
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@
},
"eslintConfig": {
"extends": [
"react-app",
"react-app/jest"
"react-app"
]
},
"browserslist": {
Expand Down Expand Up @@ -52,6 +51,7 @@
"eslint-plugin-react-hooks": "^4.3.0",
"eslint-plugin-testing-library": "^5.0.0",
"gh-pages": "^3.1.0",
"prettier": "^2.4.1"
"prettier": "^2.4.1",
"react-test-renderer": "^17.0.2"
}
}
110 changes: 94 additions & 16 deletions src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import './App.css';

import Board from './components/Board';

const PLAYER_1 = 'X';
const PLAYER_2 = 'O';
const PLAYER_1 = 'x';
const PLAYER_2 = 'o';
Comment on lines -6 to +7

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We gave bad guidance here. This should have been

const PLAYER_1 = 'x';
const PLAYER_2 = 'o';

since the tests were all written for lower case x and o. But it looks like you worked it out yourself!


const generateSquares = () => {
const squares = [];
Expand All @@ -29,37 +29,115 @@ const App = () => {
// This starts state off as a 2D array of JS objects with
// empty value and unique ids.
const [squares, setSquares] = useState(generateSquares());
const [winner, setWinner] = useState(null);
const [currentPlayer, setCurrentPlayer] = useState(PLAYER_1);

// 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 checkForWinner = () => {
// Complete in Wave 3
// You will need to:
// 1. Go accross each row to see if
// 3 squares in the same row match
// i.e. same value
// 2. Go down each column to see if
// 3 squares in each column match
// 3. Go across each diagonal to see if
// all three squares have the same value.
let i = 0;

// Check all the rows and columns for a winner
while (i < 3) {
if (
squares[i][0].value === squares[i][1].value &&
squares[i][2].value === squares[i][1].value &&
squares[i][0].value !== ''
) {
return squares[i][0].value;
} else if (
squares[0][i].value === squares[1][i].value &&
squares[2][i].value === squares[1][i].value &&
squares[0][i].value !== ''
) {
return squares[0][i].value;
}
i += 1;
}
// Check Top-Left to bottom-right diagonal
if (
squares[0][0].value === squares[1][1].value &&
squares[2][2].value === squares[1][1].value &&
squares[1][1].value !== ''
) {
return squares[0][0].value;
}

// Check Top-right to bottom-left diagonal
if (
squares[0][2].value === squares[1][1].value &&
squares[2][0].value === squares[1][1].value &&
squares[1][1].value !== ''
) {
return squares[0][2].value;
}

return null;
};

// const onClickCallBack = (id) => {
// const emptyBoard = [...squares];
// for (let row of emptyBoard) {
// for (let square of row) {
// if (square.id === id && square.value === '') {
// square.value = currentPlayer;
// if (winner) {
// !setCurrentPlayer();
// checkForWinner();
// setSquares(emptyBoard);
// } else if (!winner) {
// if (currentPlayer === PLAYER_1) {
// setCurrentPlayer(PLAYER_2);
// } else if (currentPlayer === PLAYER_2) {
// setCurrentPlayer(PLAYER_1);
// }
// }
// }
// }
// }
// };

const onClickCallback = (id) => {
setSquares((squares) => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can use use the callback style of updating the squares here, but we don't actually know whether we want to or not. What if the player clicked on a square that was already played on? What if the game was already over? In that case, we could track whether a play was made, and if not, simply return the original board reference.

let emptyBoard = squares.map((square)=>{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea to copy the existing squares state using map, but notice this only does a shallow copy. So we get a new outer array, but the inner arrays (the rows) and the square data objects are shared between the previous state, and the new state.

This is what allows the checkForWinner call on line 120 in the click handler to appear to work. It references the squares state variable, which shouldn't appear to be updated until the next render. The fact that it is correctly seeing the winning move means that our code here is cross contaminating the previous state rather than only making a new state.

for (let openSquare of square){
if (openSquare.id === id && openSquare.value === ''){
openSquare.value = currentPlayer;
// if (!winner) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this check (and without protecting the openSquare assignment as well) it's possible to continue playing the game even after there's a winner, and the winner can even change! You could add this check as part of the checks on line 107.

Notice that we should have still duplicated the inner array (each row of the board) and made a copy of the square being modified, adding the copied, updated square to the new row.

if (currentPlayer === PLAYER_1) {
setCurrentPlayer(PLAYER_2);
} else if (currentPlayer === PLAYER_2) {
setCurrentPlayer(PLAYER_1);
}
// }
}
}
return square;
});
setWinner(checkForWinner());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As written, checkForWinner should not be called during the click handling. It is written to look at the squares piece of state, but if we are properly calculating a new state and setting it, then the updated squares state wouldn't become observable until the next render. It is working here, since the changes to the board values are cross-contaminating the current React squares without React knowing about it (due to JS storing references to data).

Instead, with how checkForWinner is written, it should be called in the body of the App component function, which would check for a winner on each re-render. Notice this would allow us to avoid introducing a piece of state to track the winner.

return emptyBoard;
});
};

const resetGame = () => {
// Complete in Wave 4
setSquares(generateSquares());
setWinner(null);
setCurrentPlayer(PLAYER_1);
};
Comment on lines 125 to 130

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, resetting the game essentially comes down to setting the state back to its initial values.


return (
<div className="App">
<header className="App-header">
<div className='App'>
<header className='App-header'>
<h1>React Tic Tac Toe</h1>
<h2>The winner is ... -- Fill in for wave 3 </h2>
<button>Reset Game</button>
<h2>Winner is {winner}</h2>
<button onClick={resetGame}>Reset Game</button>
</header>
<main>
<Board squares={squares} />
<Board squares={squares} onClickCallback={onClickCallback}/>
</main>
</div>
);
Expand Down
28 changes: 16 additions & 12 deletions src/components/Board.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,33 @@ import './Board.css';
import Square from './Square';
import PropTypes from 'prop-types';


// This turns the 2D array into a 1D array
const generateSquareComponents = (squares, onClickCallback) => {
// Complete this for Wave 1
// squares is a 2D Array, but
// you need to return a 1D array
// of square components

}
const singleArraySquares = [].concat(...squares);
return singleArraySquares.map((square) => {
return (
<Square
value={square.value}
id={square.id}
onClickCallback={onClickCallback}
key={square.id}
/>
);
});
};

const Board = ({ squares, onClickCallback }) => {
const squareList = generateSquareComponents(squares, onClickCallback);
console.log(squareList);
return <div className="grid" >
{squareList}
</div>
}
return <div className='grid'>{squareList}</div>;
};

Board.propTypes = {
squares: PropTypes.arrayOf(
PropTypes.arrayOf(
PropTypes.shape({
id: PropTypes.number.isRequired,
value: PropTypes.string.isRequired
value: PropTypes.string.isRequired,
})
)
),
Expand Down
13 changes: 6 additions & 7 deletions src/components/Square.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
import React from 'react';
import PropTypes from 'prop-types';

import './Square.css'
import './Square.css';

const Square = (props) => {
// For Wave 1 enable this
// Component to alert a parent
// component when it's clicked on.
const updateSquareLetter = () => {
props.onClickCallback(props.id);
};
Comment on lines +10 to +12

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice custom handler to call our click callback when the button was clicked. Notice that we can't simply use props.onClickCallback directly as the onClick handler, since we need to pass the id of the clicked square to the callback. If we had used it as the onClick directly, then the event details provided by the browser would have been passed in as the first parameter rather than the id.


return <button
className="square"
>
{props.value}
</button>
}
return <button className='square' onClick={updateSquareLetter}>{props.value}</button>;
};

Square.propTypes = {
value: PropTypes.string.isRequired,
Expand Down
Loading