DOCSP-8872: Fix toctree behavior (#158)

mongodb · Feb 18, 2020 · 82beb9e · 82beb9e
1 parent 77c9cd4
commit 82beb9e
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 52 deletions.
diff --git a/src/components/TOCNode.js b/src/components/TOCNode.js
@@ -1,4 +1,4 @@
-import React, { useContext } from 'react';
+import React, { useContext, useState } from 'react';
 import PropTypes from 'prop-types';
 import Link from './Link';
 import { formatText } from '../utils/format-text';
@@ -19,67 +19,73 @@ const TOCNode = ({ node, level = BASE_NODE_LEVEL }) => {
   const target = slug || url;
   const hasChildren = !!children.length;
   const isExternalLink = !!url;
-  const { activeSection, toggleDrawer } = useContext(TOCContext);
+  const { activeSection, setActiveSection } = useContext(TOCContext);
   const isActive = isActiveTocNode(activeSection, slug, children);
   const anchorTagClassNames = `reference ${isActive ? 'current' : ''} ${isExternalLink ? 'external' : 'internal'}`;
   const isSelected = isSelectedTocNode(activeSection, slug);
   const toctreeSectionClasses = `toctree-l${level} ${isActive ? 'current' : ''} ${isSelected ? 'selected-item' : ''}`;
+  const isDrawer = !!(options && options.drawer);
+
+  const [isOpen, setIsOpen] = useState(isActive);
+
+  // Show caret if not on first level of TOC
+  const caretIcon =
+    level !== BASE_NODE_LEVEL ? (
+      <span
+        className={hasChildren ? 'expand-icon docs-expand-arrow' : 'expand-icon'}
+        style={{ WebkitTransform: isOpen ? 'rotate(135deg)' : null, transform: isOpen ? 'rotate(135deg)' : null }}
+      />
+    ) : null;
 
   const NodeLink = () => {
     // If title is a plaintext string, render as-is. Otherwise, iterate over the text nodes to properly format titles.
     const formattedTitle = formatText(title);
-    if (level === BASE_NODE_LEVEL) {
-      const isDrawer = !!(options && options.drawer);
-      if (isDrawer && children.length > 0) {
-        const _toggleDrawerOnEnter = e => {
-          if (e.key === 'Enter') {
-            toggleDrawer(slug);
-          }
-        };
-        // TODO: Ideally, this value should be a button, but to keep consistent with CSS render as anchor
-        return (
-          <a // eslint-disable-line jsx-a11y/anchor-is-valid
-            onClick={() => toggleDrawer(slug)}
-            onKeyDown={_toggleDrawerOnEnter}
-            className={anchorTagClassNames}
-            aria-expanded={hasChildren ? isActive : undefined}
-            role="button"
-            tabIndex="0"
-          >
-            {formattedTitle}
-          </a>
-        );
-      }
+
+    if (isDrawer && children.length > 0) {
+      const _toggleDrawerOnEnter = e => {
+        if (e.key === 'Enter') {
+          setIsOpen(!isOpen);
+        }
+      };
+      // TODO: Ideally, this value should be a button, but to keep consistent with CSS render as anchor
       return (
-        <Link
-          to={target}
-          aria-expanded={hasChildren ? isActive : undefined}
+        <Link // eslint-disable-line jsx-a11y/anchor-is-valid
+          onClick={e => {
+            e.preventDefault();
+            setIsOpen(!isOpen);
+          }}
+          onKeyDown={_toggleDrawerOnEnter}
           className={anchorTagClassNames}
-          onClick={() => toggleDrawer(slug)}
+          aria-expanded={hasChildren ? isActive : undefined}
+          role="button"
+          tabIndex="0"
+          href={target}
         >
+          {caretIcon}
           {formattedTitle}
         </Link>
       );
     }
-
-    // In this case, we have a node which should be rendered with the 'expand-icon'
     return (
-      <Link to={target} className={anchorTagClassNames} aria-expanded={hasChildren ? isActive : undefined}>
-        <span className={hasChildren ? 'expand-icon docs-expand-arrow' : 'expand-icon'} />
+      <Link
+        to={target}
+        aria-expanded={hasChildren ? isActive : undefined}
+        className={anchorTagClassNames}
+        onClick={() => setActiveSection(slug)}
+      >
+        {caretIcon}
         {formattedTitle}
       </Link>
     );
   };
   return (
     <li className={toctreeSectionClasses}>
       <NodeLink />
-      {isActive ? (
+      {isOpen ? (
         <ul>
           {children.map(c => {
             const key = c.slug || c.url;
-            return (
-              <TOCNode activeSection={activeSection} node={c} level={level + 1} toggleDrawer={toggleDrawer} key={key} />
-            );
+            return <TOCNode node={c} level={level + 1} key={key} />;
           })}
         </ul>
       ) : null}

diff --git a/src/components/TableOfContents.js b/src/components/TableOfContents.js
@@ -14,16 +14,9 @@ const TableOfContents = ({ toctreeData: { children } }) => {
     currentPage = window.location.pathname;
   }
   const [activeSection, setActiveSection] = useState(currentPage);
-  const toggleDrawer = newSlug => {
-    if (activeSection === newSlug) {
-      setActiveSection(null);
-    } else {
-      setActiveSection(newSlug);
-    }
-  };
 
   return (
-    <TOCContext.Provider value={{ activeSection, toggleDrawer }}>
+    <TOCContext.Provider value={{ activeSection, setActiveSection }}>
       <ul className="current">
         {children.map(c => {
           const key = c.slug || c.url;

diff --git a/src/components/Target.js b/src/components/Target.js
@@ -49,7 +49,7 @@ const Target = ({ nodeData: { children, name, target } }) => {
 
 Target.propTypes = {
   nodeData: PropTypes.shape({
-    children: PropTypes.arrayOf(PropTypes.node).isRequired,
+    children: PropTypes.arrayOf(PropTypes.object).isRequired,
     name: PropTypes.string.isRequired,
     target: PropTypes.string.isRequired,
   }).isRequired,

diff --git a/src/components/toc-context.js b/src/components/toc-context.js
@@ -2,5 +2,5 @@ import React from 'react';
 
 export const TOCContext = React.createContext({
   activeSection: undefined,
-  toggleDrawer: () => {},
+  setActiveSection: () => {},
 });
diff --git a/tests/unit/TableOfContents.test.js b/tests/unit/TableOfContents.test.js
@@ -85,20 +85,20 @@ describe('Table of Contents testing', () => {
         const drawer = testComponent
           .find('.toctree-l1')
           .last()
-          .find('.reference');
-        // Drawers should not redirect to a page, so href should be undefined
-        expect(drawer.prop('href')).toBe(undefined);
+          .find('.reference')
+          .first();
         drawer.simulate('click');
         const numUseCasesNodes = mockTocData.children[3].children.length;
         expect(testComponent.find('.toctree-l2')).toHaveLength(numUseCasesNodes);
         expect(window.location.pathname).toBe('/');
         const otherDrawer = testComponent
           .find('.toctree-l1')
           .at(2)
-          .find('.reference');
+          .find('.reference')
+          .first();
         otherDrawer.simulate('click');
         const numPlatformNodes = mockTocData.children[2].children.length;
-        expect(testComponent.find('.toctree-l2')).toHaveLength(numPlatformNodes);
+        expect(testComponent.find('.toctree-l2')).toHaveLength(numUseCasesNodes + numPlatformNodes);
       });
     });
   });

diff --git a/tests/unit/__snapshots__/TableOfContents.test.js.snap b/tests/unit/__snapshots__/TableOfContents.test.js.snap
@@ -5,7 +5,7 @@ exports[`Table of Contents testing Table of Contents unit tests renders correctl
   value={
     Object {
       "activeSection": "/",
-      "toggleDrawer": [Function],
+      "setActiveSection": [Function],
     }
   }
 >