Wrote my Quote Machine in React please give me feedback

Hello,

I wrote my Quote Machine in React and in VSCode text editor/IDE. A little bit outside the box so I could push my envelope even further.

Quote Machine

here is the GitHub repo with the code:

Code Repository

Please to give me suggestions and feedback.

Thanks,
Tim

1 Like

I guess it’s not the type of feedback you are looking for, but CSS is a bit broken on the buttons. Scaling the buttons with vh makes them not fit into the width of the card on my screen (I have my task bar on the right side of the screen, so I have a bit more height on a 1920x1080 display).
You could play around with display: flex to keep the quote card to the bottom of the screen if that was your intention.
I have no coments on react.

This is exactly the type of feedback I am looking for.

I’ve attached a screen shot to this thread and i see what you mean! I hadn’t considered full screen.

well, I had switched over some of the styling to vw instead of vh, but not all of it. And I put in some media queries.

I am going to give you some feedback regarding your JavaScript/React code.

I believe you can simplify your this.state initialization to be:

    this.state = {
      quotes: [],
      currQuote: { },
      clrScheme: {
        bkg: 'elem-clr1',
        txt: 'txt-clr1'
      }
    };

Also, in your componentDidMount, you could make things a bit more concise if you wanted:

  async componentDidMount() {
    try {
      const url = 'https://raw.githubusercontent.com/timcombs/marx-headmon/master/quotes.json'
      const { quotes } = await (await fetch(url)).json();
      this.setState({
        quotes,
        currQuote: this.getQuote(quotes)
      });
    }catch(err){
      console.log(err);
    }
  }

You could use some destructuring in your handleClick method, but honestly I would see if you can make your window.open’s a bit more readable (new lines plus indentation).

  handleClick(e) {
    const { quotes, currQuote: { quote, author } } = this.state;
    if (e.target.name === 'newquote') {
      this.setState(() => ({
        currQuote: this.getQuote(quotes)
      }));
      this.changeColors();
    }else if (e.target.name === 'twitter') {
      window.open('https://twitter.com/intent/tweet?hashtags=quotes&text=' + encodeURIComponent(`${quote} -${author}`), '_blank');
    }else if (e.target.name === 'tumblr') {
    window.open('https://www.tumblr.com/widgets/share/tool?posttype=quote&tags=quotes,freecodecamp&caption=' + encodeURIComponent(author) + '&content=' + encodeURIComponent(quote)+'&canonicalUrl=https%3A%2F%2Fwww.tumblr.com%2Fbuttons&shareSource=tumblr_share_button', '_blank');
    }
  }

Now we get to the big one. Your changeColors method is not very React-like. With the exception of the body element, you really should never be referencing the DOM directly with things like querySelectorAll or updating the DOM with things like button.classList.add or replace. Instead changeColors should be updating state properties for the various button components and the Quote component. You would update the state.clrScheme property in such a way so that your buttons and divs are changed. You would pass the state property via props to the various components and dynamically build the applicable class string to use for className of the components.

1 Like

Randall,

Thanks for the incredible amount of detail. This is exactly the feedback I am looking for! This is my first React project so I am still trying to figure out the paradigm.

I was wondering about accessing the DOM, I felt it was too expensive of an operation, and I’m glad to hear that I was right and that the React paradigm is a solution to this.

I guess I have more to learn, which means I have more to do. Nice explanation of how to use destructuring as well.

Thanks for the help!
Tim

Thanks again for the help with the quote machine. I was able to easily change the colors on the components by using the this.state.clrScheme AND thanks for the nudge toward destructuring, now that makes a lot more sense to me.