From bbcae0d1740e1d9e1f1a2995e73af2bd97924861 Mon Sep 17 00:00:00 2001 From: Marc Thomas Date: Fri, 14 Jan 2022 13:49:42 +0000 Subject: [PATCH 1/6] chore: navigation ia improvements --- src/components/Navigation/README.md | 122 ++++++++++++++- src/components/Navigation/_variables.scss | 2 + .../Navigation/components/Item/Item.tsx | 12 +- .../components/Item/tests/Item.test.tsx | 44 ++++++ .../Navigation/components/Section/Section.tsx | 41 ++++- .../components/Section/tests/Section.test.tsx | 142 ++++++++++++++++++ 6 files changed, 350 insertions(+), 13 deletions(-) diff --git a/src/components/Navigation/README.md b/src/components/Navigation/README.md index 24929600a78..d535e6a1eff 100644 --- a/src/components/Navigation/README.md +++ b/src/components/Navigation/README.md @@ -220,10 +220,128 @@ Use to present a secondary action, related to a section and to title the section selected: true, subNavigationItems: [ { - url: '/admin/products', + url: '/admin/products/collections', disabled: false, selected: true, - label: 'All products', + label: 'Collections', + }, + { + url: '/admin/products/inventory', + disabled: false, + label: 'Inventory', + }, + ], + }, + ]} + /> + +``` + +### Navigation with multiple secondary navigations + +Use to present a secondary action, related to a section and to title the section. + +```jsx + + + +``` + +### Navigation with an active root item with secondary navigation items + +Use to present a secondary action, related to a section and to title the section. + +```jsx + + { if (!isNavigationCollapsed && expanded) { - setExpanded(false); + onToggleExpandedState?.(); } - }, [expanded, isNavigationCollapsed]); + }, [expanded, isNavigationCollapsed, onToggleExpandedState]); const handleKeyUp = useCallback( (event) => { @@ -254,6 +257,7 @@ export function Item({ disabled && styles['Item-disabled'], selected && subNavigationItems.length === 0 && styles['Item-selected'], showExpanded && styles.subNavigationActive, + childIsActive && styles['Item-child-active'], keyFocused && styles.keyFocused, ); @@ -347,7 +351,7 @@ export function Item({ isNavigationCollapsed ) { event.preventDefault(); - setExpanded(!expanded); + onToggleExpandedState?.(); } else if (onNavigationDismiss) { onNavigationDismiss(); if (onClick && onClick !== onNavigationDismiss) { diff --git a/src/components/Navigation/components/Item/tests/Item.test.tsx b/src/components/Navigation/components/Item/tests/Item.test.tsx index 076030b8f46..648de24b2f6 100644 --- a/src/components/Navigation/components/Item/tests/Item.test.tsx +++ b/src/components/Navigation/components/Item/tests/Item.test.tsx @@ -3,6 +3,8 @@ import {PlusMinor, ExternalMinor} from '@shopify/polaris-icons'; import {matchMedia} from '@shopify/jest-dom-mocks'; import {mountWithApp} from 'tests/utilities'; +import {PolarisTestProvider} from '../../../../PolarisTestProvider'; +import type {MediaQueryContext} from '../../../../../utilities/media-query'; import {Badge} from '../../../../Badge'; import {Icon} from '../../../../Icon'; import {Indicator} from '../../../../Indicator'; @@ -649,6 +651,30 @@ describe('', () => { expect(spy).toHaveBeenCalledTimes(1); }); }); + + describe('onToggleExpandedState', () => { + it('fires the onToggleExpandedState handler when clicked', () => { + const onToggleExpandedState = jest.fn(); + const item = mountWithNavigationAndPolarisTestProvider( + , + {location: '/admin/orders'}, + {isNavigationCollapsed: true}, + ); + item?.find('a')?.trigger('onClick', { + preventDefault: jest.fn(), + currentTarget: { + getAttribute: () => 'baz', + }, + }); + expect(onToggleExpandedState).toHaveBeenCalledTimes(1); + }); + }); }); describe('keyFocused', () => { @@ -725,3 +751,21 @@ function mountWithNavigationProvider( , ); } + +function mountWithNavigationAndPolarisTestProvider( + node: React.ReactElement, + navigationContext: React.ContextType = { + location: '', + }, + mediaQueryContext: React.ContextType = { + isNavigationCollapsed: false, + }, +) { + return mountWithApp( + + + {node} + + , + ); +} diff --git a/src/components/Navigation/components/Section/Section.tsx b/src/components/Navigation/components/Section/Section.tsx index 41ef87e1c6b..f4f494ed87b 100644 --- a/src/components/Navigation/components/Section/Section.tsx +++ b/src/components/Navigation/components/Section/Section.tsx @@ -1,13 +1,13 @@ -import React, {useEffect, useRef} from 'react'; +import React, {useEffect, useRef, useState} from 'react'; import {HorizontalDotsMinor} from '@shopify/polaris-icons'; import {classNames} from '../../../../utilities/css'; -import {navigationBarCollapsed} from '../../../../utilities/breakpoints'; +import {useMediaQuery} from '../../../../utilities/media-query'; import {useUniqueId} from '../../../../utilities/unique-id'; import {useToggle} from '../../../../utilities/use-toggle'; import {Collapsible} from '../../../Collapsible'; import {Icon, IconProps} from '../../../Icon'; -import {Item, ItemProps} from '../Item'; +import {Item, ItemProps, SubNavigationItem} from '../Item'; import styles from '../../Navigation.scss'; export interface SectionProps { @@ -43,6 +43,8 @@ export function Section({ setFalse: setExpandedFalse, } = useToggle(false); const animationFrame = useRef(null); + const {isNavigationCollapsed} = useMediaQuery(); + const [expandedIndex, setExpandedIndex] = useState(); const handleClick = ( onClick: ItemProps['onClick'], @@ -57,7 +59,7 @@ export function Section({ cancelAnimationFrame(animationFrame.current); } - if (!hasSubNavItems || !navigationBarCollapsed().matches) { + if (!hasSubNavItems || !isNavigationCollapsed) { animationFrame.current = requestAnimationFrame(setExpandedFalse); } }; @@ -93,18 +95,43 @@ export function Section({ ); - const itemsMarkup = items.map((item) => { - const {onClick, label, subNavigationItems, ...rest} = item; + const itemsMarkup = items.map((item, index) => { + const {onClick, label, url, disabled, subNavigationItems, ...rest} = item; const hasSubNavItems = subNavigationItems != null && subNavigationItems.length > 0; + const itemAsSubNavigationItem: SubNavigationItem = { + onClick, + label, + url: url as string, + disabled, + }; + const addedSubNavigationItems = + isNavigationCollapsed && hasSubNavItems + ? [ + itemAsSubNavigationItem, + ...(subNavigationItems as SubNavigationItem[]), + ] + : subNavigationItems; + + const handleToggleExpandedState = () => { + if (expandedIndex === index) { + setExpandedIndex(-1); + } else { + setExpandedIndex(index); + } + }; return ( ); }); diff --git a/src/components/Navigation/components/Section/tests/Section.test.tsx b/src/components/Navigation/components/Section/tests/Section.test.tsx index bda4f24106f..fb859170ff7 100644 --- a/src/components/Navigation/components/Section/tests/Section.test.tsx +++ b/src/components/Navigation/components/Section/tests/Section.test.tsx @@ -2,6 +2,8 @@ import React from 'react'; import {matchMedia, animationFrame} from '@shopify/jest-dom-mocks'; import {mountWithApp} from 'tests/utilities'; +import {PolarisTestProvider} from '../../../../PolarisTestProvider'; +import type {MediaQueryContext} from '../../../../../utilities/media-query'; import {Collapsible} from '../../../../Collapsible'; import {NavigationContext} from '../../../context'; import {Item} from '../../Item'; @@ -270,6 +272,128 @@ describe('', () => { expect(onClickSpy).toHaveBeenCalledTimes(1); }); + + it('duplicates the root item when on a mobile viewport', () => { + matchMedia.setMedia(() => ({matches: true})); + const withSubNav = mountWithNavigationAndPolarisTestProvider( +
, + { + ...context, + }, + { + isNavigationCollapsed: true, + }, + ); + expect(withSubNav).toContainReactComponent(Item, { + label: 'other label', + subNavigationItems: [ + { + label: 'other label', + url: '/other', + }, + {label: 'sub label', url: '/other/sub'}, + ], + }); + }); + + it('acts as an accordion when on a mobile viewport', () => { + matchMedia.setMedia(() => ({matches: true})); + const withSubNav = mountWithNavigationAndPolarisTestProvider( +
, + { + ...context, + }, + { + isNavigationCollapsed: true, + }, + ); + const firstItem = withSubNav.find(Item, {label: 'label a'}); + const secondItem = withSubNav.find(Item, {label: 'label b'}); + const thirdItem = withSubNav.find(Item, {label: 'label c'}); + + firstItem?.trigger('onToggleExpandedState'); + expect(withSubNav.find(Item, {label: 'label a'})).toHaveReactProps({ + expanded: true, + }); + + secondItem?.trigger('onToggleExpandedState'); + expect(withSubNav.find(Item, {label: 'label a'})).toHaveReactProps({ + expanded: false, + }); + expect(withSubNav.find(Item, {label: 'label b'})).toHaveReactProps({ + expanded: true, + }); + + thirdItem?.trigger('onToggleExpandedState'); + expect(withSubNav.find(Item, {label: 'label b'})).toHaveReactProps({ + expanded: false, + }); + expect(withSubNav.find(Item, {label: 'label c'})).toHaveReactProps({ + expanded: true, + }); + }); }); function mountWithNavigationProvider( @@ -283,4 +407,22 @@ function mountWithNavigationProvider( ); } +function mountWithNavigationAndPolarisTestProvider( + node: React.ReactElement, + navigationContext: React.ContextType = { + location: '', + }, + mediaQueryContext: React.ContextType = { + isNavigationCollapsed: false, + }, +) { + return mountWithApp( + + + {node} + + , + ); +} + function noop() {} From 1afc7d547aee4511f8017c6f60070a86998db15b Mon Sep 17 00:00:00 2001 From: Marc Thomas Date: Wed, 19 Jan 2022 14:43:52 +0000 Subject: [PATCH 2/6] chore: update readme --- UNRELEASED.md | 1 + 1 file changed, 1 insertion(+) diff --git a/UNRELEASED.md b/UNRELEASED.md index 1436da53ca6..f5356c939e4 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -9,6 +9,7 @@ Use [the changelog guidelines](/documentation/Versioning%20and%20changelog.md) t ### Enhancements - Tightened up the Navigation component UI density. ([#4874](https://github.com/Shopify/polaris-react/pull/4874)) +- Updated the Navigation IA ([#4902](https://github.com/Shopify/polaris-react/pull/4902)) ### Bug fixes From e58c389f8326b1adb8fe2bb568193d09b03ca2d1 Mon Sep 17 00:00:00 2001 From: Marc Thomas Date: Mon, 24 Jan 2022 11:32:50 +0000 Subject: [PATCH 3/6] chore: put duplication behind a prop --- src/components/Navigation/README.md | 20 ++++++++++--------- .../Navigation/components/Item/Item.tsx | 4 +++- .../Navigation/components/Section/Section.tsx | 4 +++- .../components/Section/tests/Section.test.tsx | 1 + 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/components/Navigation/README.md b/src/components/Navigation/README.md index d535e6a1eff..36c64a8fc61 100644 --- a/src/components/Navigation/README.md +++ b/src/components/Navigation/README.md @@ -95,15 +95,16 @@ A navigation section groups together related navigation items. Navigation sectio ### Section properties -| Prop | Type | Description | -| --------- | ------------------------ | --------------------------------------------------------------------------------------------- | -| items | [Item[]](#type-item) | A collection of navigation items to be rendered inside the section | -| icon | IconProps['source'] | An icon to be displayed next to the section title | -| title | string | A string property providing a title for the navigation section | -| fill | boolean | A boolean property indicating whether the section should take up all vertical space available | -| rollup | [Rollup[]](#type-rollup) | An object determining the collapsing behavior of the navigation section | -| action | [Action[]](#type-action) | Renders an icon-only action as a supplementary action next to the section title | -| separator | boolean | A boolean property indicating whether the section should have a visual separator | +| Prop | Type | Description | +| ----------------- | ------------------------ | -------------------------------------------------------------------------------------------------------- | +| items | [Item[]](#type-item) | A collection of navigation items to be rendered inside the section | +| icon | IconProps['source'] | An icon to be displayed next to the section title | +| title | string | A string property providing a title for the navigation section | +| fill | boolean | A boolean property indicating whether the section should take up all vertical space available | +| rollup | [Rollup[]](#type-rollup) | An object determining the collapsing behavior of the navigation section | +| action | [Action[]](#type-action) | Renders an icon-only action as a supplementary action next to the section title | +| separator | boolean | A boolean property indicating whether the section should have a visual separator | +| duplicateRootItem | boolean | A boolean property to duplicate the root level item as the first sub navigation item on mobile viewports | @@ -319,6 +320,7 @@ Use to present a secondary action, related to a section and to title the section ```jsx ', () => { matchMedia.setMedia(() => ({matches: true})); const withSubNav = mountWithNavigationAndPolarisTestProvider(
Date: Tue, 25 Jan 2022 10:56:25 +0000 Subject: [PATCH 4/6] chore: update release notes --- UNRELEASED.md | 1 + 1 file changed, 1 insertion(+) diff --git a/UNRELEASED.md b/UNRELEASED.md index f5356c939e4..7329d7063b6 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -10,6 +10,7 @@ Use [the changelog guidelines](/documentation/Versioning%20and%20changelog.md) t - Tightened up the Navigation component UI density. ([#4874](https://github.com/Shopify/polaris-react/pull/4874)) - Updated the Navigation IA ([#4902](https://github.com/Shopify/polaris-react/pull/4902)) +- Added new `duplicateRootItem` prop to a Navigation Section to support new mobile Navigation IA ([#4902](https://github.com/Shopify/polaris-react/pull/4902)) ### Bug fixes From 165705638b1c84f6c4bd9ba23e046fcf823cb17b Mon Sep 17 00:00:00 2001 From: Marc Thomas Date: Tue, 25 Jan 2022 17:51:08 +0000 Subject: [PATCH 5/6] chore: remove prop due to handling duplicates downstream --- UNRELEASED.md | 1 + src/components/Navigation/README.md | 19 ++++----- .../Navigation/components/Section/Section.tsx | 23 ++--------- .../components/Section/tests/Section.test.tsx | 41 ------------------- 4 files changed, 13 insertions(+), 71 deletions(-) diff --git a/UNRELEASED.md b/UNRELEASED.md index 7329d7063b6..e5fe9a819f9 100644 --- a/UNRELEASED.md +++ b/UNRELEASED.md @@ -11,6 +11,7 @@ Use [the changelog guidelines](/documentation/Versioning%20and%20changelog.md) t - Tightened up the Navigation component UI density. ([#4874](https://github.com/Shopify/polaris-react/pull/4874)) - Updated the Navigation IA ([#4902](https://github.com/Shopify/polaris-react/pull/4902)) - Added new `duplicateRootItem` prop to a Navigation Section to support new mobile Navigation IA ([#4902](https://github.com/Shopify/polaris-react/pull/4902)) +- Updated mobile behaviour of Navigation to only show one sub-section at a time ([#4902](https://github.com/Shopify/polaris-react/pull/4902)) ### Bug fixes diff --git a/src/components/Navigation/README.md b/src/components/Navigation/README.md index 36c64a8fc61..f936093def2 100644 --- a/src/components/Navigation/README.md +++ b/src/components/Navigation/README.md @@ -95,16 +95,15 @@ A navigation section groups together related navigation items. Navigation sectio ### Section properties -| Prop | Type | Description | -| ----------------- | ------------------------ | -------------------------------------------------------------------------------------------------------- | -| items | [Item[]](#type-item) | A collection of navigation items to be rendered inside the section | -| icon | IconProps['source'] | An icon to be displayed next to the section title | -| title | string | A string property providing a title for the navigation section | -| fill | boolean | A boolean property indicating whether the section should take up all vertical space available | -| rollup | [Rollup[]](#type-rollup) | An object determining the collapsing behavior of the navigation section | -| action | [Action[]](#type-action) | Renders an icon-only action as a supplementary action next to the section title | -| separator | boolean | A boolean property indicating whether the section should have a visual separator | -| duplicateRootItem | boolean | A boolean property to duplicate the root level item as the first sub navigation item on mobile viewports | +| Prop | Type | Description | +| --------- | ------------------------ | --------------------------------------------------------------------------------------------- | +| items | [Item[]](#type-item) | A collection of navigation items to be rendered inside the section | +| icon | IconProps['source'] | An icon to be displayed next to the section title | +| title | string | A string property providing a title for the navigation section | +| fill | boolean | A boolean property indicating whether the section should take up all vertical space available | +| rollup | [Rollup[]](#type-rollup) | An object determining the collapsing behavior of the navigation section | +| action | [Action[]](#type-action) | Renders an icon-only action as a supplementary action next to the section title | +| separator | boolean | A boolean property indicating whether the section should have a visual separator | diff --git a/src/components/Navigation/components/Section/Section.tsx b/src/components/Navigation/components/Section/Section.tsx index 9e04ef8a748..8b8b20c6816 100644 --- a/src/components/Navigation/components/Section/Section.tsx +++ b/src/components/Navigation/components/Section/Section.tsx @@ -7,7 +7,7 @@ import {useUniqueId} from '../../../../utilities/unique-id'; import {useToggle} from '../../../../utilities/use-toggle'; import {Collapsible} from '../../../Collapsible'; import {Icon, IconProps} from '../../../Icon'; -import {Item, ItemProps, SubNavigationItem} from '../Item'; +import {Item, ItemProps} from '../Item'; import styles from '../../Navigation.scss'; export interface SectionProps { @@ -27,7 +27,6 @@ export interface SectionProps { onClick(): void; }; separator?: boolean; - duplicateRootItem?: boolean; } export function Section({ @@ -37,7 +36,6 @@ export function Section({ items, rollup, separator, - duplicateRootItem, }: SectionProps) { const { value: expanded, @@ -98,22 +96,9 @@ export function Section({ ); const itemsMarkup = items.map((item, index) => { - const {onClick, label, url, disabled, subNavigationItems, ...rest} = item; + const {onClick, label, subNavigationItems, ...rest} = item; const hasSubNavItems = subNavigationItems != null && subNavigationItems.length > 0; - const itemAsSubNavigationItem: SubNavigationItem = { - onClick, - label, - url: url as string, - disabled, - }; - const addedSubNavigationItems = - isNavigationCollapsed && hasSubNavItems && duplicateRootItem - ? [ - itemAsSubNavigationItem, - ...(subNavigationItems as SubNavigationItem[]), - ] - : subNavigationItems; const handleToggleExpandedState = () => { if (expandedIndex === index) { @@ -128,9 +113,7 @@ export function Section({ key={label} {...rest} label={label} - url={url} - disabled={disabled} - subNavigationItems={addedSubNavigationItems} + subNavigationItems={subNavigationItems} onClick={handleClick(onClick, hasSubNavItems)} onToggleExpandedState={handleToggleExpandedState} expanded={expandedIndex === index} diff --git a/src/components/Navigation/components/Section/tests/Section.test.tsx b/src/components/Navigation/components/Section/tests/Section.test.tsx index ae055038d87..7d63638eb63 100644 --- a/src/components/Navigation/components/Section/tests/Section.test.tsx +++ b/src/components/Navigation/components/Section/tests/Section.test.tsx @@ -273,47 +273,6 @@ describe('', () => { expect(onClickSpy).toHaveBeenCalledTimes(1); }); - it('duplicates the root item when on a mobile viewport', () => { - matchMedia.setMedia(() => ({matches: true})); - const withSubNav = mountWithNavigationAndPolarisTestProvider( -
, - { - ...context, - }, - { - isNavigationCollapsed: true, - }, - ); - expect(withSubNav).toContainReactComponent(Item, { - label: 'other label', - subNavigationItems: [ - { - label: 'other label', - url: '/other', - }, - {label: 'sub label', url: '/other/sub'}, - ], - }); - }); - it('acts as an accordion when on a mobile viewport', () => { matchMedia.setMedia(() => ({matches: true})); const withSubNav = mountWithNavigationAndPolarisTestProvider( From 04fbf1799b7c234cbda98908a04a1aec95915f92 Mon Sep 17 00:00:00 2001 From: Marc Thomas Date: Wed, 26 Jan 2022 13:32:17 +0000 Subject: [PATCH 6/6] chore: updates from merge --- src/components/Navigation/README.md | 58 ++++------------------------- 1 file changed, 8 insertions(+), 50 deletions(-) diff --git a/src/components/Navigation/README.md b/src/components/Navigation/README.md index f936093def2..03e0e99ce8c 100644 --- a/src/components/Navigation/README.md +++ b/src/components/Navigation/README.md @@ -194,7 +194,7 @@ Use to present a navigation menu in the [frame](https://polaris.shopify.com/comp ``` -### Navigation with an active secondary navigation item +### Navigation with multiple secondary navigations Use to present a secondary action, related to a section and to title the section. @@ -212,49 +212,6 @@ Use to present a secondary action, related to a section and to title the section label: 'Orders', icon: OrdersMinor, badge: '15', - }, - { - url: '/admin/products', - label: 'Products', - icon: ProductsMinor, - selected: true, - subNavigationItems: [ - { - url: '/admin/products/collections', - disabled: false, - selected: true, - label: 'Collections', - }, - { - url: '/admin/products/inventory', - disabled: false, - label: 'Inventory', - }, - ], - }, - ]} - /> - -``` - -### Navigation with multiple secondary navigations - -Use to present a secondary action, related to a section and to title the section. - -```jsx - -