4
\$\begingroup\$

I am new to ReactJS as a fun alternative to the old boring JS and for practicing I tried to make a sort of "tabby cat" app similar to the browser extension one (tabbycats.club) where you get a new random cat image/name/background color for every new tab or page reload. But I feel like my current code is quite messed up. Any feedback/suggestions on how to improve it? Demo here

index.js

import React from "react";
import ReactDOM from "react-dom";
import "./style.css";
import black_cat from "./img/black_cat.png";
import brown1_cat from "./img/brown1_cat.png";
import brown2_cat from "./img/brown2_cat.png";
import gray1_cat from "./img/gray1_cat.png";
import gray2_cat from "./img/gray2_cat.png";
import ginger_cat from "./img/ginger_cat.png";

/* HELPER FUNCTIONS */

function getRGBComponents(color) {
    // Returns red/green/blue as integers inside object
    var r = color.substring(1, 3);
    var g = color.substring(3, 5);
    var b = color.substring(5, 7);
    return {
        R: parseInt(r, 16),
        G: parseInt(g, 16),
        B: parseInt(b, 16)
    };
}

function getIdealColor(bgColor) {
    // Returns white/black color based on given background color
    var nThreshold = 105;
    var components = getRGBComponents(bgColor);
    var bgDelta = components.R * 0.299 + components.G * 0.587 + components.B * 0.114;
    return 255 - bgDelta < nThreshold ? "#000000" : "#ffffff";
}

// Previous vars to prevent immediate duplicates
var lastColor = "";
var lastCatImg = "";
var lastCatName = "";

/* APP */

class TabbyCatApp extends React.Component {
    constructor() {
        super();
        this.state = {
            backgroundColor: ""
        };
    }

    getRandomColor() {
        var color =
            "#" +
            Math.random()
                .toString(16)
                .substr(-6);
        // Prevent duplicate
        while (color === lastColor) {
            color =
                "#" +
                Math.random()
                    .toString(16)
                    .substr(-6);
        }
        // Store var and return it
        lastColor = color;
        return lastColor;
    }

    changeColor() {
        this.setState({
            backgroundColor: this.getRandomColor()
        });
    }

    getIdealColor(backgroundColor) {
        return getIdealColor(backgroundColor);
    }

    getRandomCatName() {
        var names = [
            "Oscar",
            "Max",
            "Tiger",
            "Bingo",
            "Coco",
            "Simba",
            "Sammy",
            "Smokey",
            "Ginger",
            "Felix",
            "Garfield",
            "Alfie",
            "Charlie"
        ];
        var randomNum = Math.floor(Math.random() * names.length);
        // Prevent duplicate
        while (names[randomNum] === lastCatName) {
            randomNum = Math.floor(Math.random() * names.length);
        }
        // Store var and return it
        lastCatName = names[randomNum];
        return lastCatName;
    }

    getRandomCatImage() {
        var images = [
            black_cat,
            brown1_cat,
            gray1_cat,
            ginger_cat,
            brown2_cat,
            gray2_cat
        ];
        var randomNum = Math.floor(Math.random() * images.length);
        // Prevent duplicate
        while (images[randomNum] === lastCatImg) {
            randomNum = Math.floor(Math.random() * images.length);
        }
        // Store var and return it
        lastCatImg = images[randomNum];
        return lastCatImg;
    }

    getRandomCat(backgroundColor) {
        let fontColor = {
            color: this.getIdealColor(backgroundColor)
        };
        return [
            <img alt="cat_img" className="cat_img" src={this.getRandomCatImage()} />,
            <h2 className="cat_name" style={fontColor}>
                {this.getRandomCatName()}
            </h2>
        ];
    }

    render() {
        // Generate random background color
        let backgroundColor = this.getRandomColor();
        let initialBackgroundColor = {
            backgroundColor: backgroundColor
        };
        // Generate random cat
        let cat = this.getRandomCat(backgroundColor);
        // Return background + cat
        return (
            <div
                className="background"
                style={initialBackgroundColor}
                onClick={this.changeColor.bind(this)}
            >
                <div className="cat_container">{cat}</div>
            </div>
        );
    }
}

ReactDOM.render(<TabbyCatApp />, document.getElementById("root"));
\$\endgroup\$
1
\$\begingroup\$

There are some conceptual misunderstanding, from the code that I see, so let me try to elaborate them:

Pre-requisite: The react formula

I have seen react philosophy described by one of their core team members as:

v = f(d)

or UI/view is a function of data.

Data in react can be stored in state or prop, the function is render, the result or view is the resulting dom.

Your render method is giving separate output for same state and props

Many people initially think that whenever render method is called, the dom is manipulated or that react calls render whenever it is ready to repaint the dom. I answered this in details on stackoverflow.

If you look at your code, it is breaking the react formula. The same function is returning different view for the same data. This may not seem important for your example now, but will cause problems any time you have a hierarchy of components.

From the official react docs:

The render() function should be pure, meaning that it does not modify component state, it returns the same result each time it’s invoked, and it does not directly interact with the browser.

I will leave out why react wants it to be that way, but leave a comment if you want me to elaborate on that.

Is your data complete & at one place?

You are using variables at different places, state to store your data. It can work, but again think of the formula. What data does your function require to produce the view that you want to see?

If I take your code and replace function calls with variables that they produce:

<div
  className="background"
  style={someBgColor}
  onClick={this.changeColor.bind(this)}
>
  <div className="cat_container">
    <img alt="cat_img" className="cat_img" src={someCatImage} />
    <h2 className="cat_name" style={someFontColor}>
      {someCatName}
    </h2>
  </div>
</div>

I see you need bgColor, fontColor, catImage, catName. These should stay in your state.

I will stop here to keep the answer brief, I will edit to add more points a bit later.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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