Search code examples
reactjsreact-bootstrappopover

Takes two clicks for react bootstrap popover to show up


I've run into an issue while trying to build a page that allows the user to click on a word and get its definition in a bootstrap popover. That is achieved by sending an API request and updating the state with the received data.

The problem is that the popover only appears after the second click on the word. The console.log() in useEffect() shows that every time a new word is clicked an API request is made. For the popover to appear the same word must be clicked twice. It'd be better if it only took one click.

    import React, { useState, useRef, useEffect } from "react";
    import axios from "axios";
    import { Alert, Popover, OverlayTrigger } from "react-bootstrap";
    
    export default function App() {
      const [text, setText] = useState(
        "He looked at her and saw her eyes luminous with pity."
      );
      const [selectedWord, setSelectedWord] = useState("luminous");
      const [apiData, setApiData] = useState([
        {
          word: "",
          phonetics: [{ text: "" }],
          meanings: [{ definitions: [{ definition: "", example: "" }] }]
        }
      ]);
    
      const words = text.split(/ /g);
    
      useEffect(() => {
        var url = "https://api.dictionaryapi.dev/api/v2/entries/en/" + selectedWord;
        axios
          .get(url)
          .then(response => {
            setApiData(response.data)
            console.log("api call")
           })
          .catch(function (error) {
            if (error) {
              console.log("Error", error.message);
            }
          });
      }, [selectedWord]);
    
      function clickCallback(w) {
        var word = w.split(/[.!?,]/g)[0];
        setSelectedWord(word);
      }
    
      const popover = (
        <Popover id="popover-basic">
          <Popover.Body>
            <h1>{apiData[0].word}</h1>
            <h6>{apiData[0].meanings[0].definitions[0].definition}</h6>
          </Popover.Body>
        </Popover>
      );
    
      return (
        <Alert>
          {words.map((w) => (
            <OverlayTrigger
              key={uuid()}
              trigger="click"
              placement="bottom"
              overlay={popover}
            >
              <span onClick={() => clickCallback(w)}> {w}</span>
            </OverlayTrigger>
          ))}
        </Alert>
      );
    }

UPDATE: Changed the apiData initialization and the <Popover.Body> component. That hasn't fixed the problem.

    const [apiData, setApiData] = useState(null)
    <Popover.Body>
            {
              apiData ?
                <div>
                  <h1>{apiData[0].word}</h1>
                  <h6>{apiData[0].meanings[0].definitions[0].definition}</h6>
                </div> :
                <div>Loading...</div>
            }
          </Popover.Body>

Solution

  • The Problem

    Here's what I think is happening:

    1. Component renders
    2. Start fetching definition for "luminous".
    3. The definition of "luminous" has finished being fetched. It calls setApiData(data).
    4. Component rerenders
    5. If you click "luminous", the popper is shown immediately, this is because the data for the popper is ready to use and setSelectedWord("luminous") does nothing.
    6. If you click another word, such as "pity", the popper attempts to show, but setSelectedWord("pity") causes the component to start rerendering.
    7. Component rerenders
    8. Start fetching definition for "pity".
    9. The definition of "pity" has finished being fetched. It calls setApiData(data).
    10. Component rerenders
    11. If you click "pity", the popper is shown immediately, this is because the data for the popper is ready to use and setSelectedWord("pity") does nothing.

    Selecting another word will repeat this process over and over.

    To fix this, you need to first make use of the show property to show the popover after rendering it out if it matches the selected word. But what if the word appears multiple times? If you did this for the word "her", it would show the popover in multiple places. So instead of comparing against each word, you'd have to assign each word a unique ID and compare against that.

    Fixing the Component

    To assign words an ID that won't change between renders, we need to assign them IDs at the top of your component and store them in an array. To make this "simpler", we can abstract that logic into a re-useable function outside of your component:

    // Use this function snippet in demos only, use a more robust package
    // https://gist.github.com/jed/982883 [DWTFYWTPL]
    const uuid = function b(a){return a?(a^Math.random()*16>>a/4).toString(16):([1e7]+-1e3+-4e3+-8e3+-1e11).replace(/[018]/g,b)}
    
    // Splits the text argument into words, removes excess formatting characters and assigns each word a UUID.
    // Returns an array with the shape: { [index: number]: { word: string, original: string, uuid: string }, text: string }
    function identifyWords(text) {
      // split input text into words with unique Ids
      const words = text
        .split(/ +/)
        .map(word => {
          const cleanedWord = word
            .replace(/^["]+/, "")     // remove leading punctuation
            .replace(/[.,!?"]+$/, "") // remove trailing punctuation
          
          return { word: cleanedWord, original: word, uuid: uuid() }
        });
      
      // attach the source text to the array of words
      // we can use this to prevent unnecessary rerenders
      words.text = text;
      
      // return the array-object
      return words;
    }
    

    Within the component, we need to setup the state variables to hold the words array. By passing a callback to useState, React will only execute it on the first render and skip calling it on rerenders.

    // set up state array of words that have their own UUIDs
    // note: we don't want to call _setWords directly
    const [words, _setWords] = useState(() => identifyWords("He looked at her and saw her eyes luminous with pity."));
    

    Now that we have words and _setWords, we can pull out the text value from it:

    // extract text from words array for convenience
    // probably not needed
    const text = words.text;
    

    Next, we can create our own setText callback. This could be simpler, but I wanted to make sure we support React's mutating update syntax (setText(oldValue => newValue)):

    // mimic a setText callback that actually updates words as needed
    const setText = (newTextOrCallback) => {
      if (typeof newTextOrCallback === "function") {
        // React mutating callback mode
        _setWords((words) => {
          const newText = newTextOrCallback(words.text);
          return newText === words.text
            ? words // unchanged
            : identifyWords(newText); // new value
        });
      } else {
        // New value mode
        return newTextOrCallback === words.text
          ? words // unchanged
          : identifyWords(newTextOrCallback); // new value
      }
    }
    

    Next, we need to set up the currently selected word. Once the definition is available, this word's popover will be shown.

    const [selectedWordObj, setSelectedWordObj] = useState(() => words.find(({word}) => word === "luminous"));
    

    If you don't want to show a word by default, use:

    const [selectedWordObj, setSelectedWordObj] = useState(); // nothing selected by default
    

    To fix the API call, we need to make use of the "use async effect" pattern (there are libraries out there to simplify this):

    const [apiData, setApiData] = useState({ status: "loading" });
    
    useEffect(() => {
      if (!selectedWordObj) return; // do nothing.
    
      // TODO: check cache here
    
      // clear out the previous definition
      setApiData({ status: "loading" });
      
      let unsubscribed = false;
      axios
        .get(`https://api.dictionaryapi.dev/api/v2/entries/en/${selectedWordObj.word}`)
        .then(response => {
          if (unsubscribed) return; // do nothing. out of date response
          
          const body = response.data;
          
          // unwrap relevant bits
          setApiData({
            status: "completed",
            word: body.word,
            definition: body.meanings[0].definitions[0].definition
          });
        })
        .catch(error => {
          if (unsubscribed) return; // do nothing. out of date response
          
          console.error("Failed to get definition: ", error);
          
          setApiData({
            status: "error",
            word: selectedWordObj.word,
            error
          });
        });
        
      return () => unsubscribed = true;
    }, [selectedWord]);
    

    The above code block makes sure to prevent calling the setApiData methods when they aren't needed any more. It also uses a status property to track it's progress so you can render the result properly.

    Now to define a popover that shows a loading message:

    const loadingPopover = (
      <Popover id="popover-basic">
        <Popover.Body>
          <span>Loading...</span>
        </Popover.Body>
      </Popover>
    );
    

    We can mix that loading popover with apiData to get a popover to show the definition. If we're still loading the definition, use the loading one. If we've had an error, show the error. If it completed properly, render out the defintion. To make this easier, we can put this logic in a function outside of your component like so:

    
    function getPopover(apiData, loadingPopover) {
      switch (apiData.status) {
        case "loading":
          return loadingPopover;
        case "error":
          return (
            <Popover id="popover-basic">
              <Popover.Body>
                <h1>{apiData.word}</h1>
                <h6>Couldn't find definition for {apiData.word}: {apiData.error.message}</h6>
              </Popover.Body>
            </Popover>
          );
        case "completed":
          return (
            <Popover id="popover-basic">
              <Popover.Body>
                <h1>{apiData.word}</h1>
                <h6>{apiData.definition}</h6>
              </Popover.Body>
            </Popover>
          );
      }
    }
    

    We call this funtion in the component using:

    const selectedWordPopover = getPopover(apiData, loadingPopover);
    

    Finally, we render out the words. Because we are rendering out an array, we need to use a key property that we'll set to each word's Id. We also need to select the word that was clicked - even if there were more than one of the same words, we only want just the clicked one. For that we'll check its Id too. If we click on a particular word, we need to sure that the one we clicked on is selected. We also need to render out the original word with its punctuation. This is all done in this block:

    return (
      <Alert>
        {words.map((wordObj) => {
          const isSelectedWord = selectedWordObj && selectedWordObj.uuid = wordObj.uuid;
          return (
            <OverlayTrigger
              key={wordObj.uuid}
              show={isSelectedWord}
              trigger="click"
              placement="bottom"
              overlay={isSelectedWord ? selectedWordPopover : loadingPopover}
            >
              <span onClick={() => setSelectedWordObj(wordObj)}> {wordObj.original}</span>
            </OverlayTrigger>
          )})}
      </Alert>
    );
    

    Complete Code

    Bringing all that together gives:

    import React, { useState, useRef, useEffect } from "react";
    import axios from "axios";
    import { Alert, Popover, OverlayTrigger } from "react-bootstrap";
    
    // Use this function snippet in demos only, use a more robust package
    // https://gist.github.com/jed/982883 [DWTFYWTPL]
    const uuid = function b(a){return a?(a^Math.random()*16>>a/4).toString(16):([1e7]+-1e3+-4e3+-8e3+-1e11).replace(/[018]/g,b)}
    
    // Splits the text argument into words, removes excess formatting characters and assigns each word a UUID.
    // Returns an array with the shape: { [index: number]: { word: string, original: string, uuid: string }, text: string }
    function identifyWords(text) {
      // split input text into words with unique Ids
      const words = text
        .split(/ +/)
        .map(word => {
          const cleanedWord = word
            .replace(/^["]+/, "")     // remove leading characters
            .replace(/[.,!?"]+$/, "") // remove trailing characters
          
          return { word: cleanedWord, original: word, uuid: uuid() }
        });
      
      // attach the source text to the array of words
      words.text = text;
      
      // return the array
      return words;
    }
    
    function getPopover(apiData, loadingPopover) {
      switch (apiData.status) {
        case "loading":
          return loadingPopover;
        case "error":
          return (
            <Popover id="popover-basic">
              <Popover.Body>
                <h1>{apiData.word}</h1>
                <h6>Couldn't find definition for {apiData.word}: {apiData.error.message}</h6>
              </Popover.Body>
            </Popover>
          );
        case "completed":
          return (
            <Popover id="popover-basic">
              <Popover.Body>
                <h1>{apiData.word}</h1>
                <h6>{apiData.definition}</h6>
              </Popover.Body>
            </Popover>
          );
      }
    }
    
    export default function App() {
      // set up state array of words that have their own UUIDs
      // note: don't call _setWords directly
      const [words, _setWords] = useState(() => identifyWords("He looked at her and saw her eyes luminous with pity."));
      
      // extract text from words array for convenience
      const text = words.text;
      
      // mimic a setText callback that actually updates words as needed
      const setText = (newTextOrCallback) => {
        if (typeof newTextOrCallback === "function") {
          // React mutating callback mode
          _setWords((words) => {
            const newText = newTextOrCallback(words.text);
            return newText === words.text
              ? words // unchanged
              : identifyWords(newText); // new value
          });
        } else {
          // New value mode
          return newTextOrCallback === words.text
            ? words // unchanged
            : identifyWords(newTextOrCallback); // new value
        }
      }
    
      const [selectedWordObj, setSelectedWordObj] = useState(() => words.find(({word}) => word === "luminous"));
      
      const [apiData, setApiData] = useState({ status: "loading" });
    
      useEffect(() => {
        if (!selectedWordObj) return; // do nothing.
    
        // TODO: check cache here
    
        // clear out the previous definition
        setApiData({ status: "loading" });
        
        let unsubscribed = false;
        axios
          .get(`https://api.dictionaryapi.dev/api/v2/entries/en/${selectedWordObj.word}`)
          .then(response => {
            if (unsubscribed) return; // do nothing. out of date response
            
            const body = response.data;
            
            // unwrap relevant bits
            setApiData({
              status: "completed",
              word: body.word,
              definition: body.meanings[0].definitions[0].definition
            });
           })
          .catch(error => {
            if (unsubscribed) return; // do nothing. out of date response
            
            console.error("Failed to get definition: ", error);
            
            setApiData({
              status: "error",
              word: selectedWordObj.word,
              error
            });
          });
          
        return () => unsubscribed = true;
      }, [selectedWord]);
    
      function clickCallback(w) {
        var word = w.split(/[.!?,]/g)[0];
        setSelectedWord(word);
      }
      
      const loadingPopover = (
        <Popover id="popover-basic">
          <Popover.Body>
            <span>Loading...</span>
          </Popover.Body>
        </Popover>
      );
    
      const selectedWordPopover = getPopover(apiData, loadingPopover);
    
      return (
        <Alert>
          {words.map((wordObj) => {
            const isSelectedWord = selectedWordObj && selectedWordObj.uuid = wordObj.uuid;
            return (
              <OverlayTrigger
                key={wordObj.uuid}
                show={isSelectedWord}
                trigger="click"
                placement="bottom"
                overlay={isSelectedWord ? selectedWordPopover : loadingPopover}
              >
                <span onClick={() => setSelectedWordObj(wordObj)}> {wordObj.original}</span>
              </OverlayTrigger>
            )})}
        </Alert>
      );
    }
    

    Note: You can improve this by caching the results from the API call.