Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pine: Kris & Lia #60

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Pine: Kris & Lia #60

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
94077b6
Changes to Board
lgaetano Dec 15, 2021
a216ec3
yarn.lock
KJabbusch Dec 15, 2021
2330ea4
Merge branch 'main' of https://github.com/lgaetano/react-tic-tac-toe
KJabbusch Dec 16, 2021
7f53e1e
1d array
KJabbusch Dec 16, 2021
f734a1f
doSomething works
KJabbusch Dec 20, 2021
09eb1f7
wave 1-2 passing
KJabbusch Dec 20, 2021
2e0398b
almost all tests passing...
KJabbusch Dec 21, 2021
69a4eb4
all tests passing!
KJabbusch Dec 21, 2021
982992d
Deactivate squares works and winner-doesnt-update-bug fixed.
lgaetano Dec 22, 2021
4e4fce1
Co-authored-by: Kristel Jabbusch <[email protected]>
lgaetano Dec 22, 2021
95376ce
Debugs checkForWinner functionality
lgaetano Jan 18, 2022
3a923c3
Cleans up comments
lgaetano Jan 19, 2022
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
90 changes: 80 additions & 10 deletions src/App.js
View file
Open in desktop
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';

const generateSquares = () => {
const squares = [];
Expand All @@ -17,6 +17,7 @@ const generateSquares = () => {
squares[row].push({
id: currentId,
value: '',
disabled: false,
});
currentId += 1;
}
Expand All @@ -26,40 +27,109 @@ const generateSquares = () => {
};

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 [ticDict, setTicDict] = useState({});
const [turn, setTurn] = useState(PLAYER_1);
const [winner, setWinner] = useState(null);
// 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 toggleTurn = () => {
let newTurn = '';
if (turn === PLAYER_1) {
newTurn = PLAYER_2;
} else {
newTurn = PLAYER_1;
}
setTurn(newTurn);
};

const onClickCallback = (squareID) => {
const newSquares = [...squares];
newSquares.forEach((row) => {
row.forEach((square) => {
if (square.id === squareID && square.value === '') {
square.value = turn;
ticDict[squareID] = turn;
Copy link

Choose a reason for hiding this comment

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

This variable isn't being used correctly as a state variable. A state variable should be treated as if it is const at all times, and changes should be made through the corresponding set state function. To change a state var, make a copy, make the changes to the copy, and then call the set state function and pass in the changed copy. This can cause some issues if you want to use the new version right away, but you can bypass this by using the copied version (ie, pass the copied version to checkForWinner).

Copy link
Author

Choose a reason for hiding this comment

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

Hi Jasmine, thank you for the feedback. Can you clarify which variable isn't being used correctly as a state variable here? Do you mean that we shouldn't be writing directly to ticDict? We should be writing to copy of ticDict and then using a set function to update it?

}
});
});
setSquares(newSquares);
toggleTurn();

let someoneWon = checkForWinner();
if (someoneWon) {
setWinner(someoneWon);
squares.forEach((row) => {
row.map((square) => {
if (square.value == '') {
square.disabled = true;
Copy link

Choose a reason for hiding this comment

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

Neat addition!

}
});
});
}
};

const checkForWinner = () => {
// Complete in Wave 3
// You will need to:
// 1. Go accross each row to see if
// 1. Go across 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.
/* we need to update dict each click and then check

012, 345, 678 horizontal winners
036, 147, 258 vertical winners
048, 246 diagonal winners */

let winner = null;
Copy link

Choose a reason for hiding this comment

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

There is an interesting bug in this function! The code fails the test that checks the bottom row of x's. The test setup goes as follows: 6 - x, 1 - o, 8 - x, 2 - o, 7 - x, which results in 2 o's in the top row and 3 x's in the bottom row. The if-else sequence to find the winner fails at this point, because line 94 is true! For values 3, 4 and 5, ticDict is undefined, so they are equal! As a result, winner is set to undefined.

Copy link
Author

Choose a reason for hiding this comment

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

Oh my goodness. Thank you for figuring this one out--we were stumped!

Copy link

Choose a reason for hiding this comment

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

Haha, cool bug!!! Thanks for pointing it out :-)

if (ticDict[0] == ticDict[1] && ticDict[1] == ticDict[2]) {
winner = ticDict[0];
} else if (ticDict[3] == ticDict[4] && ticDict[4] == ticDict[5]) {
winner = ticDict[3];
} else if (ticDict[6] == ticDict[7] && ticDict[6] == ticDict[8]) {
winner = ticDict[6];
} else if (ticDict[0] == ticDict[3] && ticDict[3] == ticDict[6]) {
winner = ticDict[0];
} else if (ticDict[1] == ticDict[4] && ticDict[4] == ticDict[7]) {
winner = ticDict[1];
} else if (ticDict[2] == ticDict[5] && ticDict[5] == ticDict[8]) {
winner = ticDict[2];
} else if (ticDict[0] == ticDict[4] && ticDict[4] == ticDict[8]) {
winner = ticDict[0];
} else if (ticDict[2] == ticDict[4] && ticDict[4] == ticDict[6]) {
Copy link

Choose a reason for hiding this comment

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

This is a long sequence of hardcoded checks - I recommend thinking about how you could shorten this with loops.

winner = ticDict[2];
} else if (Object.keys(ticDict).length === 9) {
winner = 'tie';
Copy link

Choose a reason for hiding this comment

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

Nice!

}
return winner;
};

const resetGame = () => {
// Complete in Wave 4
setTicDict({});
setSquares(generateSquares());
setTurn(PLAYER_1);
setWinner(null);
};

return (
<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
58 changes: 38 additions & 20 deletions src/App.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe('App', () => {
expect(buttons[buttonIndex].innerHTML).toEqual(expectedResult);
}

describe.skip('Wave 2: clicking on squares and rendering App', () => {
describe('Wave 2: clicking on squares and rendering App', () => {

test('App renders with a board of 9 empty buttons', () => {
// Arrange-Act - Render the app
Expand Down Expand Up @@ -85,7 +85,7 @@ describe('App', () => {
});


describe.skip('Wave 3: Winner tests', () => {
describe('Wave 3: Winner tests', () => {
describe('Prints "Winner is x" when x wins', () => {
test('that a winner will be identified when 3 Xs get in a row across the top', () => {
// Arrange
Expand Down Expand Up @@ -310,61 +310,62 @@ describe('App', () => {
expect(winnerScreen).not.toBeNull();
expect(winnerScreen).toBeInTheDocument();
});
test('that a winner will be identified when 3 Os go accross the right column', () => {

test('that a winner will be identified when 3 Os go accross the top-left to bottom-right', () => {
// Arrange
const { container } = render(<App />);

// Act
clickButtonAndVerifyResult(container, 1, 'x');
clickButtonAndVerifyResult(container, 2, 'o');
clickButtonAndVerifyResult(container, 0, 'x');
clickButtonAndVerifyResult(container, 5, 'o');
clickButtonAndVerifyResult(container, 0, 'o');
clickButtonAndVerifyResult(container, 3, 'x');
clickButtonAndVerifyResult(container, 4, 'o');
clickButtonAndVerifyResult(container, 7, 'x');
clickButtonAndVerifyResult(container, 8, 'o');

// Assert
const winnerScreen = screen.queryByText('Winner is o')
expect(winnerScreen).not.toBeNull();
expect(winnerScreen).toBeInTheDocument();
});

test('that a winner will be identified when 3 Os go accross the top-left to bottom-right', () => {
});
test('that a winner will be identified when 3 Os go accross the top-right to bottom-left', () => {
// Arrange
const { container } = render(<App />);

// Act
clickButtonAndVerifyResult(container, 1, 'x');
clickButtonAndVerifyResult(container, 0, 'o');
clickButtonAndVerifyResult(container, 0, 'x');
clickButtonAndVerifyResult(container, 2, 'o');
clickButtonAndVerifyResult(container, 3, 'x');
clickButtonAndVerifyResult(container, 4, 'o');
clickButtonAndVerifyResult(container, 7, 'x');
clickButtonAndVerifyResult(container, 8, 'o');
clickButtonAndVerifyResult(container, 6, 'o');

// Assert
const winnerScreen = screen.queryByText('Winner is o')
expect(winnerScreen).not.toBeNull();
expect(winnerScreen).toBeInTheDocument();
});
test('that a winner will be identified when 3 Os go accross the top-right to bottom-left', () => {
});

test('that a winner will be identified when 3 Os go accross the right column', () => {
// Arrange
const { container } = render(<App />);

// Act
clickButtonAndVerifyResult(container, 0, 'x');
clickButtonAndVerifyResult(container, 1, 'x');
clickButtonAndVerifyResult(container, 2, 'o');
clickButtonAndVerifyResult(container, 3, 'x');
clickButtonAndVerifyResult(container, 4, 'o');
clickButtonAndVerifyResult(container, 0, 'x');
clickButtonAndVerifyResult(container, 5, 'o');
clickButtonAndVerifyResult(container, 7, 'x');
clickButtonAndVerifyResult(container, 6, 'o');
clickButtonAndVerifyResult(container, 8, 'o');

// Assert
const winnerScreen = screen.queryByText('Winner is o')
expect(winnerScreen).not.toBeNull();
expect(winnerScreen).toBeInTheDocument();
});
});
});

describe.skip('Wave 4: reset game button', () => {
describe('Wave 4: reset game button', () => {
test('App has a "Reset Game" button', () => {
// Arrange-Act
render(<App />);
Expand Down Expand Up @@ -396,4 +397,21 @@ describe('App', () => {
expect(oSquare).toBeNull();
});
});
test('that a winner will be identified when 3 Os go accross the right column', () => {
// Arrange
const { container } = render(<App />);
// Act
clickButtonAndVerifyResult(container, 1, 'x');
clickButtonAndVerifyResult(container, 2, 'o');
clickButtonAndVerifyResult(container, 0, 'x');
clickButtonAndVerifyResult(container, 5, 'o');
clickButtonAndVerifyResult(container, 7, 'x');
clickButtonAndVerifyResult(container, 8, 'o');

// Assert

const winnerScreen = screen.queryByText('Winner is o');
expect(winnerScreen).not.toBeNull();
expect(winnerScreen).toBeInTheDocument();
});
});
33 changes: 24 additions & 9 deletions src/components/Board.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,44 @@ import './Board.css';
import Square from './Square';
import PropTypes from 'prop-types';


const generateSquareComponents = (squares, onClickCallback) => {
// Complete this for Wave 1
// squares is a 2D Array, but
// squares is a 2D Array, but
// you need to return a 1D array
// of square components
let allSquares = [];
squares.forEach((row) => {
for (let i = 0; i < 3; i++) {
allSquares.push(row[i]);
}
});

}
const squareComponents = allSquares.map((square) => {
return (
<Square
key={square.id}
value={square.value}
onClickCallback={onClickCallback}
id={square.id}
disabled={square.disabled}
/>
);
});
return <div className="grid">{squareComponents}</div>;
};

const Board = ({ squares, onClickCallback }) => {
const squareList = generateSquareComponents(squares, onClickCallback);
console.log(squareList);
return <div className="grid" >
{squareList}
</div>
}
// console.log(squareList);
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
29 changes: 17 additions & 12 deletions src/components/Board.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import '@testing-library/jest-dom/extend-expect';
import Board from './Board';
import { render, screen, fireEvent} from '@testing-library/react'
import { render, screen, fireEvent } from '@testing-library/react';

// Sample input to the Board component
const SAMPLE_BOARD = [
Expand Down Expand Up @@ -46,15 +46,14 @@ const SAMPLE_BOARD = [
value: 'X',
id: 8,
},
],
],
];

describe('Wave 1: Board', () => {

test('that board will render with the proper number of Xs and Os', () => {
// Act
render(<Board squares={SAMPLE_BOARD} onClickCallback={() => { }} />);
render(<Board squares={SAMPLE_BOARD} onClickCallback={() => {}} />);

// Assert
const xSquares = screen.getAllByText('X');
expect(xSquares.length).toEqual(5);
Expand All @@ -70,12 +69,14 @@ describe('Wave 1: Board', () => {
return {
...square,
value: '',
}
};
});
});

// Act
const { container } = render(<Board squares={emptyBoard} onClickCallback={() => { }} />);
const { container } = render(
<Board squares={emptyBoard} onClickCallback={() => {}} />
);

// Assert
const buttons = container.querySelectorAll('.grid button');
Expand All @@ -84,10 +85,12 @@ describe('Wave 1: Board', () => {
});
describe('Wave 2: Board', () => {
describe('button click callbacks', () => {
test.skip('that the callback is called for the 1st button', () => {
test('that the callback is called for the 1st button', () => {
// Arrange
const callback = jest.fn();
const { container } = render(<Board squares={SAMPLE_BOARD} onClickCallback={callback} />);
const { container } = render(
<Board squares={SAMPLE_BOARD} onClickCallback={callback} />
);
const buttons = container.querySelectorAll('.grid button');

// Act
Expand All @@ -97,10 +100,12 @@ describe('Wave 2: Board', () => {
expect(callback).toHaveBeenCalled();
});

test.skip('that the callback is called for the last button', () => {
test('that the callback is called for the last button', () => {
// Arrange
const callback = jest.fn();
const { container } = render(<Board squares={SAMPLE_BOARD} onClickCallback={callback} />);
const { container } = render(
<Board squares={SAMPLE_BOARD} onClickCallback={callback} />
);
const buttons = container.querySelectorAll('.grid button');

// Act
Expand All @@ -110,4 +115,4 @@ describe('Wave 2: Board', () => {
expect(callback).toHaveBeenCalled();
});
});
});
});
Loading