Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
JulianNF opened this issue Jun 27, 2019 · 2 comments

Comments

@JulianNF
Copy link

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
Copy link
Author

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants