Conversation
ipc103
left a comment
ipc103
left a comment
There was a problem hiding this comment.
Hey @LaishaCalalpa - really nice work on this one! I left you some comments in line - the biggest thing to take a look at would be what type of data you're storing in the history array. Otherwise, I left a few comments around what other stuff you might be able to include on the context you created. Feel free to respond if you have any other questions, but overall nice job!
| setBoardState(boardMoveFrame[move]); | ||
| } | ||
|
|
||
| function moveToStart() { |
There was a problem hiding this comment.
👍 - this is a neat feature. Nice work including this!
| </ol> | ||
| </div> | ||
| <div className="rows"> | ||
| <Cells click={() => clickHandler(0)} value={boardState[0]} boardArr={boardArr} setBoardState={setBoardState}/> |
There was a problem hiding this comment.
One trick for handling stuff like this is to use the chunk method from lodash to split your array into three groups. You can then map over the result and render a row for each group. https://lodash.com/docs/4.17.15#chunk
{_.chunk(board, 3).map(row => (
<div className="rows">{row.map((cell, i) => <Cells onClick={() => clickHandler(i)} value={cell} }</div>
))}| @@ -0,0 +1,7 @@ | |||
| import React from 'react'; | |||
|
|
|||
| function Cells(props) { | |||
There was a problem hiding this comment.
A couple of small things here:
- I'd name this component
Cellinstead ofCellssince it represents on square on the board. - If you want to, you can use object destructuring to avoid needing to use
props.multiple times.
function Cell({click, value})| return ( | ||
| <div> | ||
| <h1>Player {turn}, its your turn</h1> | ||
| <TurnContext.Provider value={turn}> |
There was a problem hiding this comment.
It's cool that you used a context here, because it demonstrates knowledge of different React hooks. Nice work! One thing to think about is - when you're making a context, it might make sense to group related pieces of data together. For this one, I'd probably include a bunch of the game data on the context so that it is all in one place. This could include:
- The board
- The current player
- The history of the board
- Whether or not we have a winner
- A function for the current player to take their turn
You have all of that stuff in your app, but it's just spread out. Keeping it all in the context could help keep the components a little bit smaller.
| import Board from './Board'; | ||
|
|
||
| export const TurnContext = React.createContext(); | ||
| export const SetTurnContext = React.createContext(); |
There was a problem hiding this comment.
One other note - you can include multiple pieces of state on a single context, so I'd probably group both the turn and the setTurn function onto one TurnContext instead of separating them out.
| const travel = move ? `Go to move ${move}` : 'Go to game start'; | ||
| return ( | ||
| <li key={move}> | ||
| <button onClick={() => moveTravel(move)}>{travel}</button> |
There was a problem hiding this comment.
This seems to be where the bug in the time travel is coming from. What does your history array look like here? Once you confirm that, I bet you will be able to debug this :-)
3 participants
No description provided.