timeTravel done, not classed based#7
timeTravel done, not classed based#7CieloRaymundo wants to merge 1 commit intoThe-Marcy-Lab-School:masterfrom
Conversation
| setBoardState(boardInHistory[step]); | ||
| } | ||
|
|
||
| function goToStart() { |
There was a problem hiding this comment.
👍 - this is a neat feature. Nice work including this!
ipc103
left a comment
ipc103
left a comment
There was a problem hiding this comment.
Hey @CieloRaymundo - 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 stages 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!
|
|
||
| return ( | ||
| <div className="board"> | ||
| <div className="rows"> |
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="row board-row">{row.map((cell, i) => <Square onClick={() => clickHandler(i)} value={cell} }</div>
))}| return ( | ||
| <div> | ||
| <h1>It's your turn player {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 React from 'react'; | ||
| import './style.css'; | ||
|
|
||
| function Squares(props) { |
There was a problem hiding this comment.
One small thing - you can use object destructuring here if you like to avoid needing to call props. multiple times.
function Square({click, value})| import './style.css'; | ||
| import Board from './Board'; | ||
|
|
||
| export const TurnContext = 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 desc = `Go to move #${stage+1}`; | ||
| return ( | ||
| <li key={stage}> | ||
| <button onClick={() => moveToStage(stage)}>{desc}</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 stages array look like here? Once you confirm that, I bet you will be able to debug this :-)
3 participants
No description provided.