3
\$\begingroup\$

I am currently learning React and tried building the UI of a basic minesweeper game as a first project. My goal was to write simple, re-usable components. I will go over each of them and do my best to explain the rationale behind them and, after that, explain how they are all tied together in the main App component. Please let me know if anything I have written can be done in a more efficient or idiomatic way.

As said before I have built the UI using React, the game logic is separate from the the React code. The whole project can be seen on github and the game can be actually played on my neocities website. One last precision: for readability reasons, the following is jsx code.

The Picker component

The picker component is used to create a group of labeled radio-buttons. It comes with a function checkedRadio(groupName) which returns the html value attribute of the chosen option. Note that I render a collection of elements without using key, here's why:

  • each radio-button has an id attribute to make it work with the html label, I feel like adding a unique key would be redundant
  • the list of radio-button shouldn't change over time, which means no re-render, and thus no need for a key
/* Picker component:
 * Return a group of radio buttons.
 * Expected props:
 *      - name: the html name for the group of radio buttons
 *      - descriptions: a sequence of strings that each describes an option in the
 *          group
 *      - defaultPick: the index of the description for the default option picked
 *      - onChange: a function called when the radio button checked changes, this
 *          function can take as an argument an object of the form
 *          { description, indexOfDescription }
 *      - radioClass: the class name for every option of the Picker
 * Additional props will be attached to the wrapping div.
 */
const Picker = function({name, descriptions, defaultPick, onChange, radioClass, ...rest}) {
    return (
        <div {...rest}>
            {descriptions.map((description, i) => {
                return (
                    <span className={radioClass}>
                    <input type='radio'
                               name={name}
                               value={description}
                               id={`${ name }-${ description }`}
                               defaultChecked={i===defaultPick}
                               onChange={() => onChange({description, i})} />
                    <label htmlFor={`${ name }-${ description }`}>{description}</label>
                    </span>);
            })}
        </div>
    );
}

/* Return the value of the radio button checked in the group of radio buttons
 * that is named groupName.
 * Will throw an error if no such group is found.
 */
const checkedRadio = function(groupName) {
    const grp = document.querySelectorAll(`input[name=${groupName}`);
    for (const node of grp)
        if (node.checked)
            return node.value;
    throw new Error(`no radio group of name ${ groupName }`);
}

The Matrix component

This component is used to render a rectangular 2D array of data. It uses a Row component to render lines of cells. How each cell is render is up to the user, and a cellComponent should be provided for both of these components to work as intended. Here as well, I am not using any key, it is up to the user to give a key (or not) to their cells. Given that the user can make their cells have keys, I figured that Row elements don't need any key of their own.

/* Matrix component:
 * Return a table-like element.
 * Expected props:
 *      - array: the 2D array that must be translated into a pseudo-table
 *      - rowClass: the class name for every row in the Matrix
 *      - onClick, onContextMenu: functions to be called when a cell of the
 *          Matrix is respectively left-clicked or right-clicked
 *      - cellComponent: a component to be instanciated for every cell of the
 *          given 2D array. Such a component should accept props of the form
 *          { cell, x, y, onClick, onContextMenu }
 * Additional props will be attached to the wrapping div.
 */
const Matrix = function({array, rowClass, onClick, onContextMenu, cellComponent, ...rest}) {
    return (
        <div {...rest}>
            {array.map((row, y) => {
                return (
                    <Row
                      className={rowClass}
                      row={row}
                      y={y}
                      onClick={onClick}
                      onContextMenu={onContextMenu}
                      cellComponent={cellComponent} />
                );
            })}
        </div>
    );
}

/* Row component:
 * Return a flat sequence of cells. This component is meant to be instanciated
 * by the Matrix component.
 * Expected props:
 *      - row: the flat (1D) array that must be translated into a Row
 *      - y: the number of the Row in the Matrix
 *      - onClick, onContextMenu: functions to be called when a cell of the
 *          Row is respectively left-clicked or right-clicked
 *      - cellComponent: a component to be instanciated for every cell of the
 *          given 2D array. Such a component should accept props of the form
 *          { cell, x, y, onClick, onContextMenu }
 * Additional props will be attached to the wrapping div.
 */
const Row = function({row, y, onClick, onContextMenu, cellComponent, ...rest}) {
    return (
        <div {...rest}>
            {row.map((cell, x) => cellComponent({x, y, onClick, onContextMenu, cell}))}
        </div>
    );
}

My own cellComponent: the mineCell component

This is the cellComponent that is meant to be called by the Matrix component. It is meant to represent cells in a minesweeper game, and therefore relies more on the logic part (which isn't covered here). mines.UNREV, mines.BOMB and mines.FLAG are constants defined in the logic part that are used to differentiate between kinds of cell, here they are used to apply different styles to different cells. Still in the logic part, each cell can be represented be a single character, which is the actual cell argument passed to the mineCell component.

// Map to determine the CSS class of a cell based on its value
const cellClasses = new Map(
    [[mines.UNREV, 'tile-unrevealed'],
    [mines.BOMB, 'tile-bomb'],
    [mines.FLAG, 'tile-flag']]
);

// Cell component for the Minesweeper game
const mineCell = function({x, y, onClick, onContextMenu, cell}) {
    let className = cellClasses.get(cell);
    return (
        <span
          className={className}
          onClick={() => onClick(x, y)}
          onContextMenu={(evt) => { evt.preventDefault(); onContextMenu(x, y) }}>
            {cell}
        </span>
    );
}

Where everything is tied together: the App component

Let's have a look at the render function. It uses a Picker element (with difficulties defined in the logic part as descriptions) to let the user choose the difficulty setting and a Matrix element to render all the cells of the game.

// labels for the difficulty settings
const descriptions = Array.from(mines.difficulties.keys());

// Map to determine the reset button css class depending on the minefield state
const resetClasses = new Map(
    [[mines.WON, 'reset-won'],
    [mines.LOST, 'reset-lost'],
    [mines.PLAYING, 'reset-normal']]
);

App.prototype.render = function() {
    const minefield = this.state.minefield;
    let resetButtonClass = resetClasses.get(minefield.state);
    return (
        <div id='app'>
            <Picker
              id='picker'
              descriptions={descriptions}
              name='difficulty'
              defaultPick={1}
              onChange={() => this.changeDifficulty()}
              radioClass='radio-button'/>
            <button
              id='reset-button'
              className={resetButtonClass}
              onClick={() => this.resetGame()}
              alt='button to reset the game'>
            </button>
            <Matrix
              id='game'
              array={minefield.view}
              rowClass='mines-row'
              onClick={(x, y) => this.mineviewLeftClick(x, y)}
              onContextMenu={(x, y) => this.mineviewRightClick(x, y)}
              cellComponent={mineCell}/>
        </div>
    );
}

Here's how the App component is defined. The chosen difficulty can be retrieved at any moment by finding which option is checked in the difficulty Picker. The state only contains the Minefield instance. (Quick precision about the Minefield constructor defined in the logic part: it requires a position to instantiate any object so that the first cell clicked is not a bomb. For that reason anything displayed before the first click of a game is actually a dummy minefield.)

// Dummy minefield used when the game hasn't started yet
const DUMMY = 'du';
const dummyMinefield = function([width, height, _]) {
    return {
        view: Array(height).fill(Array(width).fill(mines.UNREV)),
        state: DUMMY
    };
}

const App = function(props) {
    React.Component.call(this, props);
    Object.defineProperty(this, 'difficulty', {
        enumerable: true,
        get: function() { return checkedRadio('difficulty'); }
    });
    this.state = {
        minefield: dummyMinefield(mines.difficulties.get(descriptions[1])),
    };
}
App.prototype = Object.create(React.Component.prototype);
App.prototype.constructor = App;

The rest is the different functions: changing difficulty, starting the game over, clicking on a cell, putting a flag on a cell.

// Change the difficulty picked
App.prototype.changeDifficulty = function() {
    this.setState({
        minefield: dummyMinefield(mines.difficulties.get(this.difficulty)),
    });
}

// Reset the on-going game of minesweeper
App.prototype.resetGame = function() {
    this.setState({
        minefield: dummyMinefield(mines.difficulties.get(this.difficulty)),
    });
}

// Handle click events on the Matrix component that represents the minefield

/* Handle left click events
 * Reveal the minefield's cell at coordinates x, y if the minefield is not a dummy.
 * Create a minefield otherwise
 */
App.prototype.mineviewLeftClick = function(x, y) {
    if (this.state.minefield.state===DUMMY) {
        const [width, height, bombs] = mines.difficulties.get(this.difficulty);
        const minefield = new mines.Minefield(width, height, [x, y], bombs);
        this.setState({minefield});
    }
    else {
        const minefield = this.state.minefield.reveal([x, y]);
        this.setState({minefield});
    }
}

/* Handle right click events
 * Flag the minefield's cell at coordinates x, y if the minefield is not a dummy.
 */
App.prototype.mineviewRightClick = function(x, y) {
    if (this.state.minefield.state!==DUMMY) {
        const minefield = this.state.minefield.flag([x, y]);
        this.setState({minefield});
    }
}

That's it, I hope it's not to long or too vague and I am looking forward to hear advice on how to write more idiomatic React code.

\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

Use classes instead of creating a function that manually inheirts from React.Component. This will also allow you to more concisely define properties and methods. For example:

class App extends React.Component {
    state = {
        minefield: dummyMinefield(mines.difficulties.get(descriptions[1])),
    };
    get difficulty() {
        return checkedRadio('difficulty');
    }
    changeDifficulty() {
        this.setState({
            minefield: dummyMinefield(mines.difficulties.get(this.difficulty)),
        });
    }
    // ...

Classes are a lot nicer for organizing code than functions and assigning to the .prototype.

Or, even better:

Consider functional components - React recommends trying functional components and hooks instead of class components in new code. To me, the operation and lifecycle of functional components is more intuitive than for class components. If you try them, you may find that they make maintenance easier.

Avoid native DOM methods when possible - with React, whenever you see the use of a native DOM method like querySelector (or .children or .style, etc.), take a step back and consider if there's another way. Usually, the use of such methods can be avoided by using React's state and render methods instead.

The only part with this issue is with the difficulty picker. Ideally, the checkedRadio function would be removed completely. (If you decide to keep it anyway, at least change the name to something like getCheckedRadioName - the variable doesn't hold the checked radio button, it returns the checked radio name)

Put the difficulty into the app's state instead, and make the Picker a controlled component instead (so that selection changes go into state). Pass down the difficulty value and setter into the picker. Something like this:

const App = () => {
    // these get passed as selectedDescription and setSelectedDescription
    const [difficulty, setDifficulty] = useState('EASY');
const Picker = function({name, descriptions, selectedDescription, setSelectedDescription, radioClass, ...rest}) {
    return (
        <div {...rest}>
            {descriptions.map((description) => {
                return (
                    <span className={radioClass}>
                    <input type='radio'
                               name={name}
                               value={description}
                               id={`${ name }-${ description }`}
                               checked={description === selectedDescription}
                               onChange={() => setSelectedDescription(description)} />
                    <label htmlFor={`${ name }-${ description }`}>{description}</label>
                    </span>);
            })}
        </div>
    );
}

Rest parameters There are a whole lot of props in the component. Rather than allowing for as many additional props as the caller wants, you might consider using a single prop instead of the rest, and give it an informative name. Maybe:

, radioClass, pickerContainerProps = {}}) {
  return (
    <div {...pickerContainerProps}>

Name I don't think the name prop needs to be passed down - it's an implementation detail that only the Picker cares about, after all, and only for the purposes of grouping the radios. Maybe have Picker come up with a name instead?

const inputNameRef = useRef();
if (!inputNameRef.current) {
  inputNameRef.current = makeRandomString();
}
// proceed to use `inputNameRef.current` instead of `name`

You can also avoid the ids and the htmlFor by making the <input> a child of the <label>.

JSDoc The standard way to document functions in JS is with JSDoc or a similar format. In contrast, using something like

/* Picker component:
 * Return a group of radio buttons.
 * Expected props:
 *      - name: the html name for the group of radio buttons

while certainly useful, isn't quite in the expected format for good IDEs to take advantage of it. Article with examples.

Here's a small example of how VSCode could inform the developer of the expected props by putting your comments into JSDoc format:

enter image description here

Consider using plain arrow functions everywhere instead of functions - arrow functions alone are a much more common convention in React. They're a bit more concise, both in the lack of function and their ability to implicitly return.

Spread props and use shorthand properties, if you like - when you only have to list each prop once when rendering rather than twice, that's one less potential for a typo-based problem to manifest. For example, this:

<Row
  className={rowClass}
  row={row}
  y={y}
  onClick={onClick}
  onContextMenu={onContextMenu}
  cellComponent={cellComponent}
/>

could be:

<Row
  className={rowClass}
  {...{
    row,
    y,
    onClick,
    onContextMenu,
    cellComponent
  }}
/>

(you could also use object rest to collect all of these but className into a single object in the parameter list of Matrix)

Use const instead of let - see here. Especially in React, where functional programming is preferred (and sometimes necessary), it's good to indicate to the reader of the code that even the possibility of reassignment is impossible. Both let uses I see can be replaced with const.

Destructure, if you like. Hopefully you'll be using functional components instead of class components, but if not, you can change stuff like this:

const minefield = this.state.minefield;

to

const { minefield } = this.state;

Cell text When there's a visible mine on a cell, the cell text becomes "X". When there's a visible flag on a cell, the cell text becomes "P". These texts, while invisible normally, become visible if the board is highlighted. Same for unvisited cells, which have .s. Consider not rendering those texts at all - only render text if the text is a number.

Face tooltip You have

alt='button to reset the game'

Some people may not be familiar with Minesweeper's UI. Consider adding a tooltip as well, so that someone hovering over the face can see that it's clickable. Maybe add a :hover effect as well.

If you wanted to make things fancy, you could also change the face to the hesitant "O" face when the mouse is pressed down on a cell but not released yet.

Make dragging harder to do accidentally It's very, very easy to accidentally drag the main interface by clicking down anywhere inside the interface and then dragging. This makes it impossible to reconsider one's choice if one clicks down on a cell but hasn't released the mouse yet. Consider allowing the drag action only with the .close-bar at the top.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for your insights! I disagree with you on a few points (namely the use of classes and DOM methods), however, I strongly appreciate your answer overall and your advice will surely help me enhance the quality of my code for future projects. \$\endgroup\$
    – Louisono
    Commented Jan 12, 2021 at 19:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.