beaucarnes / fcc-project-tutorials

freeCodeCamp video project tutorials.
296 stars 416 forks source link

Incorrect property names being passed to Box component from Grid component #20

Open JulianNF opened 5 years ago

JulianNF commented 5 years ago

Hiya,

Thanks for the lovely tutorial @beaucarnes . I followed along with your tutorial yesterday evening, typing as you talked, in order to review React and noticed what I believe to be two wee typos/errors:

Issue 1:

Box needs props.id to add an id to the given box element that it is rendering ...

class Box extends React.Component {
    selectBox = () => {
        this.props.selectBox(this.props.row, this.props.col)
    };

    render() {
        return(
            <div
                className = {this.props.boxClass}
                id = {this.props.id}
                onClick = {this.selectBox}
            >
            </div>
        );
    };
};

... however, the Grid component is passing the property boxId rather than the property id...

class Grid extends React.Component {
    render() {
        const width = this.props.cols * 15;
        let rowsArr = [];
        let boxClass = "";
        for (let i = 0; i < this.props.rows; i++) {
            for (let j = 0; j < this.props.cols; j ++) {
                let boxId = i + "_" + j;

                boxClass = this.props.gridFull[i][j] ? "box on" : "box off";
                rowsArr.push(
                    <Box
                        boxClass = {boxClass}
                        key = {boxId}
                        boxId = {boxId}
                        row = {i}
                        col = {j}
                        selectBox = {this.props.selectBox}
                    />
                );
            };
        };

        return(
            <div className="grid" style={{width: width}}>
                {rowsArr}
            </div>
        );
    };
};

Issue 2:

Similarly, Grid passes the property key to Box, but the latter doesn't use/need it. In fact it seems that there are no other instances of key in the whole JS file.

Suggestions

Change the following line of Box component's render method:

id = {this.props.id}

to the following, which should keep things clearer and more consistent (e.g., as compared to boxClass):

id = {this.props.boxId}

and remove the following line from Grid:

key={boxId}

Thanks again Beau!

JulianNF commented 5 years ago

Oh! Please ignore the second issue (re: keys property) that I reported above. I've just learned the following:

Warning: Each child in a list should have a unique "key" prop.

Learning, learning, learning! 😸

beaucarnes commented 5 years ago

You're right about that first issue. Good catch! Are you interested in making a pull request to fix the issue?