Need help on random quote machine

Need help on random quote machine
0

#1

Hi, I’ve been working on this project for a number of days but I can’t seem to figure out why is it failing the test #9: “My quote machine should fetch the new quote’s author when the #new-quote button is clicked and display it in the #author element.”

Currently: https://codepen.io/JayYong/pen/bxKRBq

I can’t seem to figure out why the following are not working as well…

  1. Twitter intent (Can’t get the quote & author)

  2. Image not rendering on refresh (I’m trying to load a new random image together with the quote every time the “Random Quote” button is pressed)
    If I were to hardcode in componentDidMount: document.getElementById("quote-box").style.background = "url('https://picsum.photos/1000/450/?random')"; It would load a new image upon loading the page only.
    I have a method called updateBackground which supposedly grab a new image and update the background, but if I were to remove the componentDidMount line, it would also change the background ONCE at most upon clicking the random quote button, so I’m not where went wrong. Any help would be really appreciated! Thanks !


#2

When I run the tests with your project, none of them fail. I am using latest version of Chrome.

First of all, you should not be trying to manipulate the DOM directly like you are with the background styling of the quote-box element. Instead, you should should create a property in QuoteApps’ state to hold a style object like below:

    this.state = {
      quote: '',
      author: '',
      randBg: {background: "url('https://picsum.photos/1000/450/?random')"}
    };

Then, pass this property via props to the QuoteBox component. In my example below, I used a prop named boxStyle.

<QuoteBox getNewQuote={this.getNewQuote} boxStyle={this.state.randBg} />

Then, in QuoteBox’s render method, you would reference the style like:

  render() {
    return (
      <div id="quote-box" style={this.props.boxStyle}>

Now, how do you update the background when a new quote is generated. You will do that inside your fetchQuote method’s setState call. The API you using cache the results of the call, so you need to modify the url slightly to “trick” the API into giving you another a new background image. I did this by randomly assigning a number to a variable named randNum and then modifying the url within setState (see below).

const randNum = Math.random();
this.setState({
  quote: quote[0].content,
  author: quote[0].title,
  randBg: {background: "url('https://picsum.photos/1000/450/?random&"+ randNum +"')"}
});

Two other things I noticed about your project which you should change are:

  1. You are making two identical calls using fetch, when you only need one. If you just call fetchQuote method inside the componentDidMount method, you eliminate this duplicate code.

  2. Just like you should not have been updating the DOM (changing the style of an element directly), you should also not even have that updateQuote method at all. Instead, advantage the fact you are updating the state properties quote and author and pass them to the QuoteBox component via props (like I suggested above with the boxStyle prop). You will also need to pass the getNewQuote as a prop to QuoteBox. Instead of the following :

class QuoteBox extends React.Component {
  render() {
    return (
      <div id="quote-box" style={this.props.boxStyle}>
        <span id="text">
          <p>
            Constant repetition carries conviction. 
          </p>
        </span>
        <span id="author"> Rober Collier</span>
        <div id="buttonsGroup">
          <button id="new-quote" onClick={this.props.getNewQuote}>Random Quote</button>
          <a href={`https://twitter.com/intent/tweet/?text=${this.props.quote} - ${this.props.author}`} target="_blank" id='tweet-quote'>
            <button id="tweet">Tweet Quote</button>
          </a>
        </div>
      </div>
    );
  }
}

you would use:

class QuoteBox extends React.Component {
  render() {
    return (
      <div id="quote-box" style={this.props.boxStyle}>
        <span id="text">{this.props.quote}</span>   
        <span id="author">{this.props.author}</span>
        <div id="buttonsGroup">
          <button id="new-quote" onClick={this.props.getNewQuote}>Random Quote</button>
          <a 
            href={`https://twitter.com/intent/tweet/?text=${this.props.quote} - ${this.props.author}`} 
            target="_blank" id='tweet-quote'
          >
            <button id="tweet">Tweet Quote</button>
          </a>
        </div>
      </div>
    );
  }
}

The only catch with this method is, the API you are using returns the quotes in html format, which is a problem for React, because it escapes everything to avoid XSS issues. There is a way to deal with this, but I will let you research it for yourself. Hint: dangerouslySetInnerHTML


#3

Hi, thanks so much for the help and feedback. I’ve read up abit on XSS, seems like it’s a security vulnerability or some sort and reactJS escapes it to prevent cross site scripting. It seems like reactJS is relatively xss safe when used correctly. But what I do not understand is why is it a problem since the api I’m using returns is in html, after escaping the characters it should be fine isn’t it?

Some other things I’m still having problem is the twitter intent is still undefined when I click on the tweet button for both the quote & author.
As well as the image would “jump” back to initial image loaded for a split second before loading the new image. Any idea why is this so?


#4

It may not be an issue with the API you are using, but whenever you display something from a data source which you do not control (user input or 3rd party), you do run the risk of someone throwing in some JavaScript code which could do malicious things (read cookies or localstorage).

Not sure what you mean by this question. How can I replicate this?


#5

Hi, you can replicate the problem by clicking on the “Random Quote” button. The initial picture would appear for just a split second before it updates to the new one for every time the button is clicked, not sure if this is just on my end though.
So to prevent XSS when calling an api, are there any actions required on my end?


#6

With my super long first reply, there was a lot to read through, you seemed to have missed me implying that you no should be using updateBackground or updateQuote in your solution, because they are manipulating the DOM outside of React.

To fix, take the following actions:

#1) Remove the last then on the fetch.

.then(this.updateQuote())

#2) Remove updateQuote method from the QuoteApp component.

  // Update the current quote with new quote
  updateQuote() {
    document.getElementById("text").innerHTML = this.state.quote;
    document.getElementById("author").innerHTML = this.state.author;
  }

#3) Remove the updateBackground method from the QuoteApp component.

  // Update background into a random background
  updateBackground() {
     document.getElementById("quote-box").style.background = "url('https://picsum.photos/1000/450/?random')";
  }

#4) Remove the call to this.updateBackground inside the getNewQuote method.

  // Update the quote and background by calling the two methods
  getNewQuote() {
    this.fetchQuote();
    this.updateBackground();
  }

#5) In your QuoteApp render method (seen below), you are missing some parts. In addition to the getNewQuote and boxStyle props, you should be passing a quote and the author prop to QuoteBox. You reference props for these two, but you need to pass a value in or they will not render in the QuoteBox component.

  render() {
    return (
      <div>
        <QuoteBox getNewQuote={this.getNewQuote} boxStyle={this.state.randBg} />
      </div>
    );
  }

The best way to protect yourself against a 3rd party XSS issue when using a third party’s API, is never trust they will not purposely/accidentally send you malicious JavaScript code via the API. In a previous reply, I hinted about using dangerouslySetInnerHTML, but using this takes away the protection.

In this particular API, I doubt it would be a problem to use dangerouslySetInnerHTML. However, the only other way to100% protect yourself is to not use dangerouslySetInnerHTML and allow the html text to come in and your code just needs to re-encode the characters (certain punctuation like single quotes, double quotes, etc.) and then replace the p element tags.


#7

Thanks so much for the help randell! I’ve managed to fix those issues and learn something new during it. I will try to keep at it in the next project. Have a good one.