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.

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

Scissors Marisa #55

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

Supermobarbie
Copy link

No description provided.

…ave 2 test to test for 'O' and 'X' instead of 'o' and 'x' as well as moving SAMPLE_BOARD outside of any one function so it could be accessed by all functions
Copy link

@beccaelenzil beccaelenzil left a comment

Choose a reason for hiding this comment

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

Good work on this React project. It is clear that you have met the learning goals around working with state and event handling in nested components. I have pointed out a few small changes that makes all the tests pass (they are picky tests). Looking forward to talking about this project more in our 1:1.

clicked = true;

setNumSquaresClicked(numSquaresClicked + 1);
if (currentPlayer === PLAYER_1) {

Choose a reason for hiding this comment

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

Just note you can use a ternary here: setPlayer( currentPlayer === PLAYER_1 ? PLAYER_2 : PLAYER_1);

@@ -57,12 +105,13 @@ const App = () => {
return (
<div className="App">
<header className="App-header">
<h1>React Tic Tac Toe</h1>
<h2>The winner is ... -- Fill in for wave 3 </h2>
<h1>Marisa's Tic Tac Toe</h1>

Choose a reason for hiding this comment

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

Minor note, the test is looking for the Header to be React Tic Tac Toe

render(<App />);
      35 | 
    > 36 |     const header = screen.getByText('React Tic Tac Toe');
         |                           ^
      37 | 
      38 |     // Assert
      39 |     expect(header).toBeInTheDocument();

<h2>The winner is ... -- Fill in for wave 3 </h2>
<h1>Marisa's Tic Tac Toe</h1>
<h2>It's {currentPlayer}'s turn</h2>
<h2>{`The Winner is ${winner}`}</h2>

Choose a reason for hiding this comment

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

The test is looking for x to be lower case and the exact test Winner is x

App › Prints "Winner is o" when o wins › that a winner will be identified when 3 Os go accross the top-right to bottom-left

    expect(received).toEqual(expected) // deep equality

    Expected: "X"
    Received: "x"

      12 |     
      13 |     buttons = container.querySelectorAll('.grid button');
    > 14 |     expect(buttons[buttonIndex].innerHTML).toEqual(expectedResult);
         |                                            ^
      15 |   }
      16 | 
      17 |   describe('Wave 2: clicking on squares and rendering App', () => {

      at clickButtonAndVerifyResult (src/App.test.js:14:44)
      at Object.<anonymous> (src/App.test.js:353:7)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants