From 7cde0fd01e629efbcb2b573b7c7c3e3e59e7b2cb Mon Sep 17 00:00:00 2001 From: Yasa Akbulut Date: Wed, 19 Feb 2020 14:17:57 +0100 Subject: [PATCH 1/2] Fix nested NavItems Currently, a NavItem having more than one child will throw an error. This is due to the `children` property of `NavItem` being passed as the first argument to `React.cloneElement`, which expects a single element. The existing tests are only verifying the case where the `NavItem` has only one child. This commit maps over `children` and calls `React.cloneElement` on each of them. --- frontend/packages/shared/components/layout/NavItem.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/packages/shared/components/layout/NavItem.js b/frontend/packages/shared/components/layout/NavItem.js index b6d47ab9f6..92656e57c2 100644 --- a/frontend/packages/shared/components/layout/NavItem.js +++ b/frontend/packages/shared/components/layout/NavItem.js @@ -100,7 +100,7 @@ class NavItem extends Component { // Automatically expand nested nav if one of the child elements is active const isChildActive = React.Children.toArray(children).some(c => NavItem.isActive(c.props.href, c.props.exact)); const isExpanded = expanded || isChildActive; - const selfAwareChildren = children && React.cloneElement(children, { child: true }); + const selfAwareChildren = children && React.Children.map(children, c => React.cloneElement(c, { child: true })); let iconElement; From 8215318c4fd693cf35aa9abaa6fecf440cfdf3e4 Mon Sep 17 00:00:00 2001 From: Yasa Akbulut Date: Wed, 19 Feb 2020 14:20:10 +0100 Subject: [PATCH 2/2] Update nested NavItem tests --- .../shared/components/layout/NavItem.test.js | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/frontend/packages/shared/components/layout/NavItem.test.js b/frontend/packages/shared/components/layout/NavItem.test.js index 997b765a12..9c892e2812 100644 --- a/frontend/packages/shared/components/layout/NavItem.test.js +++ b/frontend/packages/shared/components/layout/NavItem.test.js @@ -15,11 +15,6 @@ const minProps = { href: '/mocked', }; -const minChildProps = { - title: 'A child title', - href: '/child', -}; - describe('', () => { it('renders without exploding', () => { const rendered = render(wrapInThemedTestApp()); @@ -59,7 +54,8 @@ describe('', () => { const { getByText, getByTestId } = render( wrapInThemedTestApp( - + + , ), ); @@ -71,13 +67,15 @@ describe('', () => { const { getByText, getByTestId, queryByText } = render( wrapInThemedTestApp( - + + , ), ); expect(queryByText('A child link')).not.toBeInTheDocument(); fireEvent.click(getByTestId('expand-toggle')); - expect(getByText('A child title')).toBeInTheDocument(); + expect(getByText('Child 1 title')).toBeInTheDocument(); + expect(getByText('Child 2 title')).toBeInTheDocument(); }); it('shows nested navigation automatically if the child is active', () => { @@ -85,11 +83,12 @@ describe('', () => { const { getByText } = render( wrapInThemedTestApp( - + + , ), ); expect(getByText('A link title')).toBeInTheDocument(); - expect(getByText('A child title')).toBeInTheDocument(); + expect(getByText('Child 1 title')).toBeInTheDocument(); }); });