I'm relatively new to react and attempted to create a simple Tic-Tac-Toe game. Would be great if i could get some feedback on this. Again, at this point using only React (no Redux).
main.js - main file which adds our App component to the DOM
import React from 'react';
import ReactDOM from 'react-dom';
import App from './container/App';
document.addEventListener('DOMContentLoaded', function() {
ReactDOM.render(
React.createElement(App),
document.getElementById('mount')
);
});
App.js - root component that contains the Board and Dashboard. One specific question I had here was: when I receive the reset()
event from my Dashboard App, what's the best practice on how I can end up triggering the reset()
of Board
?
import React, { Component } from 'react';
import Board from './Board'
import Dashboard from './Dashboard'
export default class App extends Component {
constructor(props) {
super(props)
this.state = {
winner: ''
}
}
reset() {
console.log('how do i pass reset to Board...?')
}
hasWinner(winner) {
this.setState({
winner: winner
})
}
render() {
return (
<div>
<Board rows={3} cols={3} hasWinner={this.hasWinner.bind(this)} />
<Dashboard winner={this.state.winner} reset={this.reset.bind(this)} />
</div>
)
}
}
Board.js - Board
component that contains Cell
components! Please ignore the checkWinner()
method for the purpose of this review.
import React, { Component } from 'react';
import Cell from './Cell'
import styles from '../css/board.css'
export default class Board extends Component {
constructor(props) {
super(props)
let board = this.createBoard();
this.state = {
board: board,
currentPlayer: 'X'
}
}
createBoard() {
let board = new Array(this.props.rows);
for ( var row = 0 ; row < this.props.rows ; row++ ) {
board[row] = new Array(this.props.cols);
board[row].fill('');
};
return board;
}
checkWinner() {
// TODO: please add correct winner algo!
return this.state.board[0][0];
}
drawBoard() {
let board = [];
for ( let i = 0 ; i < this.props.rows ; i++ ) {
for ( let j = 0 ; j < this.props.cols ; j++ ) {
var id = i + '' + j;
board.push(
<Cell
key={id}
id={id}
play={this.play.bind(this)}
value={this.state.board[i][j]} />
);
}
}
return board;
}
reset() {
let board = this.createBoard();
this.setState({
board: board,
currentPlayer: 'X'
})
}
play(ij) {
let i = ij[0];
let j = ij[1];
let board = this.state.board;
board[i][j] = this.state.currentPlayer;
this.setState({
board: board,
currentPlayer: this.state.currentPlayer==='X'?'O':'X'
})
let winner = this.checkWinner();
if ( winner != '' ) {
this.props.hasWinner(winner);
this.reset();
}
}
getClassName() {
return styles.board
}
render() {
return (
<div className={this.getClassName()}>
{this.drawBoard()}
</div>
)
}
}
Cell.js - Cell
component
import React, { Component } from 'react';
import styles from '../css/cell.css'
export default class Cell extends Component {
constructor(props) {
super(props)
}
getClassName() {
return styles.emptycell
}
play() {
if ( this.props.value === '' )
this.props.play(this.props.id)
}
render() {
return (
<div
onClick={this.play.bind(this)}
className={this.getClassName()}>
{this.props.value}
</div>
)
}
}
Dashboard.js - Component to show winner and contains a reset button.
import React, { Component } from 'react'
export default class Dashboard extends Component {
constructor(props) {
super(props)
}
render() {
const winner = this.props.winner
?<h1>And the winner is: {this.props.winner}</h1>
:<p></p>
return (
<div>
{winner}
<input type="button" value="reset" onClick={this.props.reset} />
</div>
)
}
}
-
\$\begingroup\$ Welcome to Code Review! Great job formatting your question - hopefully you get some helpful feedback! \$\endgroup\$avojak– avojak2017年01月15日 05:26:04 +00:00Commented Jan 15, 2017 at 5:26
-
\$\begingroup\$ ^ Thanks and looking forward to some helpful pointers. \$\endgroup\$noi.m– noi.m2017年01月15日 05:47:10 +00:00Commented Jan 15, 2017 at 5:47
1 Answer 1
You have two options for handling the reset event.
If the
Board
component absolutely must handle its own logic for setting its initial board state, you can add aref
prop to it, which allows you to hold onto a reference to that instance of the component, and call methods on that instance:reset() { this.board.reset(); } ... render() { return ( <div> <Board rows={3} cols={3} hasWinner={this.hasWinner.bind(this)} ref={(board) => { this.board = board; }} /> <Dashboard winner={this.state.winner} reset={this.reset.bind(this)} /> </div> ) }
Your second (and preferred) option is to "lift up" the board state from the
Board
component. This is the approach that is recommended over using aref
in the React documentation. Essentially, you want to move your board state and thecreateBoard
method from theBoard
component to theApp
component.
A note on naming:
Your Board
component's hasWinner
prop sounds like it should be receiving a
boolean value, rather than a callback. A common convention in React is to prefix
callback props with "on," so your prop names could become onWin
, onPlay
, and
onReset
, for example.
Finally, in your Dashboard
component, instead of rendering an empty p
tag when there is no winner, you can take advantage of the fact that React skips rendering values like false
and null
. So you can replace your ternary statement with something simpler:
const winner = this.props.winner && <h1>And the winner is: {this.props.winner}</h1>