refactor

rizovs · rizovs · commit 8446d6725932 · 2018-11-03T18:26:43.000+01:00
diff --git a/src/App.js b/src/App.js
@@ -17,7 +17,7 @@ function App() {
             </Link>
             <span>
               Panels scroll into view if not fully visible when toggled.
-              Using <pre>useImperativeMethods</pre>, <pre>useRef</pre> and <pre>useMemo</pre>.
+              Using <pre>useImperativeMethods</pre>, <pre>useRef</pre>.
             </span>
           </div>
           <div className="item">
diff --git a/src/components/accordion/Accordion.js b/src/components/accordion/Accordion.js
@@ -1,6 +1,5 @@
-import React, { useRef, createRef, useMemo, useImperativeMethods, useState, useEffect } from 'react';
+import React, { useRef, createRef, forwardRef, useImperativeMethods, useState, useEffect } from 'react';
 import scrollIntoView from 'scroll-into-view-if-needed';
-import FoobarIpsum from 'foobar-ipsum';
 
 function useAccordion(panelsCount) {
   const [currentIndex, setCurrentIndex] = useState();
@@ -15,14 +14,14 @@ function useAccordion(panelsCount) {
       refs[i] = createRef();
     }
     setRefs(refs);
-  }, []);
+  }, []); // run once
 
   useEffect(() => {
     // Scroll current accordion panel into view
     if (currentIndex !== undefined) {
       refs[currentIndex].current.scrollIntoView();
     }
-  }, [currentIndex]);
+  }, [currentIndex]); // Run every time current index changes
 
   function setCurrent(newIndex) {
     setCurrentIndex(currentIndex === newIndex ? undefined : newIndex);
@@ -31,42 +30,32 @@ function useAccordion(panelsCount) {
   return [currentIndex, setCurrent, refs];
 }
 
-const AccordionPanel = React.forwardRef((props, ref) => {
-  const containerRef = useRef();
-
-  // We don't want to generate random text every time this component renders
-  const randomText = useMemo(() => generateRandomText(), []);
-
-  useImperativeMethods(ref, () => ({
-    scrollIntoView: () => {
-      scrollIntoView(containerRef.current, { block: 'nearest', scrollMode: 'if-needed' });
-    }
-  }));
-
-  return <div onClick={props.onClick} ref={containerRef}>
-    <div className="accordion-label">{props.label}</div>
-    {props.isOpen &&
-    <div>{randomText}</div>}
-  </div>;
-});
+// We don't want to re-render this component every time it receives new props
+// We also pass second parameter to this function where we implement our own comparison
+const AccordionPanel = React.memo(
+  forwardRef((props, ref) => {
+    const containerRef = useRef();
+
+    useImperativeMethods(ref, () => ({
+      scrollIntoView: () => {
+        scrollIntoView(containerRef.current, { block: 'nearest', scrollMode: 'if-needed' });
+      }
+    }));
+
+    return <div onClick={props.onClick} ref={containerRef}>
+      <div className="accordion-label">{props.label}</div>
+      {props.isOpen &&
+      <div>{props.content}</div>}
+    </div>;
+  }),
+  // practically shouldComponentUpdate, but reversed
+  (prevProps, nextProps) => prevProps.isOpen === nextProps.isOpen
+);
 
 function Accordion(props) {
   return <div>{props.children}</div>;
 }
 
-function generateRandomNumber(max) {
-  return Math.floor(Math.random() * Math.floor(max));
-}
-
-function generateRandomText() {
-  return new FoobarIpsum({
-    size: {
-      sentence: generateRandomNumber(100),
-      paragraph: generateRandomNumber(10)
-    }
-  }).paragraph();
-}
-
 export {
   useAccordion,
   Accordion,
diff --git a/src/components/accordion/index.js b/src/components/accordion/index.js
@@ -1,6 +1,7 @@
 import React from 'react';
 
 import { Accordion, AccordionPanel, useAccordion } from './Accordion';
+import FoobarIpsum from "foobar-ipsum";
 
 function AccordionScreen() {
   const panels = [...Array(100).keys()].map(e => `Panel number ${e}`);
@@ -11,14 +12,27 @@ function AccordionScreen() {
     {panels.map((panel, index) => (
       <AccordionPanel
         ref={refs && refs[index]}
-        index={index}
         key={index}
         label={panel}
+        content={generateRandomText()}
         isOpen={currentIndex === index}
         onClick={() => setCurrent(index)}
       />
     ))}
   </Accordion>;
 }
 
+function generateRandomNumber(max) {
+  return Math.floor(Math.random() * Math.floor(max));
+}
+
+function generateRandomText() {
+  return new FoobarIpsum({
+    size: {
+      sentence: generateRandomNumber(100),
+      paragraph: generateRandomNumber(10)
+    }
+  }).paragraph();
+}
+
 export default AccordionScreen;
