changed files to use, pushed up working game #6
Conversation
| display: table; | ||
| } | ||
|
|
||
| .status { |
There was a problem hiding this comment.
Looks like this status style isn't used in the app. If that's the case, then this can be removed.
| outline: none; | ||
| } | ||
|
|
||
| .kbd-navigation .square:focus { |
There was a problem hiding this comment.
Keyboard navigation styles is classy. I am not familiar with this implementation. How does this work in the app?
| import "./index.css"; | ||
| import * as serviceWorker from './serviceWorker'; | ||
|
|
||
| function Square({ value, onClick }) { |
There was a problem hiding this comment.
There are several components here in index.jsx. In order to organize your code in a more modular way, I'd recommend separating out this file by component (ie could go into a "Square.jsx" file and could go into a "Game.jsx" file, etc). By having logical chunks of code, this app will become more accessible and easier for other developers to reason about.
| function Restart({ onClick }) { | ||
|
|
||
| return ( | ||
| <button className="restart" onClick={onClick}> | ||
| Play again | ||
| </button> | ||
| ); | ||
| } |
There was a problem hiding this comment.
The creation and use of this <Restart/> component is groovy and allows for a an intuitive user experience. 👍
| {renderSquare(0)} | ||
| {renderSquare(1)} | ||
| {renderSquare(2)} | ||
| </div> | ||
| <div className="board-row"> | ||
| {renderSquare(3)} | ||
| {renderSquare(4)} | ||
| {renderSquare(5)} | ||
| </div> | ||
| <div className="board-row"> | ||
| {renderSquare(6)} | ||
| {renderSquare(7)} | ||
| {renderSquare(8)} |
There was a problem hiding this comment.
Separating out the logic of rendering a square helped reduce code repetition and makes this part of the code easier to reason about. 👍 To take this a step further, this could refactored by mapping through the squares. That would mean, however, that the styles for these rows would need updating.
| if (squares[i] != null || winner != null) { | ||
| return; | ||
| } | ||
| const nextSquares = squares.slice(); | ||
| nextSquares[i] = (isXNext ? "X" : "O"); | ||
| setSquares(nextSquares); | ||
|
|
||
| setIsXNext(!isXNext); // toggle turns |
There was a problem hiding this comment.
Consider extracting this logic into it's on click handler function.
| [0, 4, 8], | ||
| [2, 4, 6] | ||
| ]; | ||
| // go over all possibly winning lines and check if they consist of only X's/only O's |
There was a problem hiding this comment.
The inline code comments for this function are helpful for other developers to get a quick grasp of how these utility functions are used. 👍
| function Game() { | ||
| const [ squares, setSquares ] = useState(Array(9).fill(null)); | ||
| const [ isXNext, setIsXNext ] = useState(true); | ||
| const nextSymbol = isXNext ? "X" : "O"; |
There was a problem hiding this comment.
It was wise to place this ternary statement in a variable. 👍 . Since nextSymbol is defined, it can be used down on lines 52 and 66.
| ); | ||
| } | ||
|
|
||
| ReactDOM.render(<Square />, document.getElementById("root")); |
There was a problem hiding this comment.
Since the <Game/> component renders the instances of the <Square/>, this component does not require its own render.
ReactDOM.render() docs states If the React element was previously rendered into container, this will perform an update on it and only mutate the DOM as necessary to reflect the latest React element.
This means that ReactDOM.render() is updated by the ReactDOM.render() on line 126. If this line remained without line 126, then the app would render only one instance of the <Square/> component.
No description provided.