From 82beb9e132e56d40afee9cd661cca12fa8fe0962 Mon Sep 17 00:00:00 2001 From: Sophie Stadler Date: Tue, 18 Feb 2020 16:52:55 -0500 Subject: [PATCH] DOCSP-8872: Fix toctree behavior (#158) --- src/components/TOCNode.js | 78 ++++++++++--------- src/components/TableOfContents.js | 9 +-- src/components/Target.js | 2 +- src/components/toc-context.js | 2 +- tests/unit/TableOfContents.test.js | 10 +-- .../TableOfContents.test.js.snap | 2 +- 6 files changed, 51 insertions(+), 52 deletions(-) diff --git a/src/components/TOCNode.js b/src/components/TOCNode.js index d0d0c7c1b..459427cd2 100644 --- 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,53 +19,61 @@ 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 ? ( + + ) : 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 ( - toggleDrawer(slug)} - onKeyDown={_toggleDrawerOnEnter} - className={anchorTagClassNames} - aria-expanded={hasChildren ? isActive : undefined} - role="button" - tabIndex="0" - > - {formattedTitle} - - ); - } + + 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 ( - { + e.preventDefault(); + setIsOpen(!isOpen); + }} + onKeyDown={_toggleDrawerOnEnter} className={anchorTagClassNames} - onClick={() => toggleDrawer(slug)} + aria-expanded={hasChildren ? isActive : undefined} + role="button" + tabIndex="0" + href={target} > + {caretIcon} {formattedTitle} ); } - - // In this case, we have a node which should be rendered with the 'expand-icon' return ( - - + setActiveSection(slug)} + > + {caretIcon} {formattedTitle} ); @@ -73,13 +81,11 @@ const TOCNode = ({ node, level = BASE_NODE_LEVEL }) => { return (
  • - {isActive ? ( + {isOpen ? (
      {children.map(c => { const key = c.slug || c.url; - return ( - - ); + return ; })}
    ) : null} diff --git a/src/components/TableOfContents.js b/src/components/TableOfContents.js index c222d65ad..faee98da4 100644 --- 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 ( - +
      {children.map(c => { const key = c.slug || c.url; diff --git a/src/components/Target.js b/src/components/Target.js index a5844a49d..f6bc73a89 100644 --- 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 index 2345790cf..4fa5149ba 100644 --- 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 index d903fde20..d42320d50 100644 --- a/tests/unit/TableOfContents.test.js +++ b/tests/unit/TableOfContents.test.js @@ -85,9 +85,8 @@ 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); @@ -95,10 +94,11 @@ describe('Table of Contents testing', () => { 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 index 6e61767a9..89ef65627 100644 --- 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], } } >