Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Navigation Component: Search Control in Menu Titles #25315

Merged
merged 30 commits into from
Oct 23, 2020
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
2649516
Move the menu title in its own component
Copons Sep 14, 2020
0159b72
Introduce basic items filtering
Copons Sep 14, 2020
460f844
Update the style
Copons Sep 14, 2020
7fde9a1
Sort rebase issues
Copons Sep 17, 2020
a73ed9b
Add a _isVisible property to the navigation tree items
Copons Sep 17, 2020
52f4bbf
Fix improper merge
Copons Sep 22, 2020
502a799
Update menu title styles
Copons Sep 22, 2020
bb60064
Restructure and fix animation
Copons Sep 22, 2020
ddf3165
Remove search field decoration
Copons Sep 23, 2020
6acdb76
Make the search optionally controlled from the outside
Copons Sep 23, 2020
cc82f17
Move the search into its own component to useRef
Copons Sep 23, 2020
d5aaf2f
Simplify the menu components interfaces
Copons Sep 23, 2020
f94dbf7
Add a visually hidden label
Copons Sep 24, 2020
3a7e083
Update search icon colors
Copons Sep 25, 2020
9d8c43b
Close search on ESC
Copons Sep 25, 2020
96be998
Focus search button on search close
Copons Sep 25, 2020
cf5e7d1
Make items navigable via up/down arrows
Copons Sep 25, 2020
b46eb78
Clarify use of title by search
Copons Sep 25, 2020
c4c947e
Update the story
Copons Sep 25, 2020
34a0ce5
Add aria labels
Copons Sep 27, 2020
1e1daf3
Simplify CSS
Copons Sep 27, 2020
b51edcd
Use space function instead of hardcoded values
Copons Sep 27, 2020
f40f9ac
Announce the number of search results
Copons Sep 27, 2020
8b90be6
Add group roles and labels
Copons Oct 7, 2020
5a63a2e
Add more aria-labelledby
Copons Oct 7, 2020
642332d
Test without listitem role
Copons Oct 12, 2020
25f33f9
Add an internal unique ID to NavigationItem instead of using the opti…
Copons Oct 14, 2020
792f368
Add a group context to handle empty groups
Copons Oct 20, 2020
15a9fe6
Remove tooltips (but keep labels)
Copons Oct 20, 2020
4dc2c3b
Move the search focus delay into a constant
Copons Oct 21, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions packages/components/src/navigation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,13 @@ A callback to handle clicking on the back button. If this prop is provided then

Optional className for the `NavigationMenu` component.

### hasSearch

- Type: `boolean`
- Required: No

Enable the search feature on the menu title.

### `menu`

- Type: `string`
Expand All @@ -109,19 +116,33 @@ Optional className for the `NavigationMenu` component.

The unique identifier of the menu. The root menu can omit this, and it will default to "root"; all other menus need to specify it.

### onSearch

- Type: `function`
- Required: No

When `hasSearch` is active, this function handles the search input's `onChange` event, making it controlled from the outside. It requires setting the `search` prop as well.

### `parentMenu`

- Type: `string`
- Required: No

The parent menu slug; used by nested menus to indicate their parent menu.

### search

- Type: `string`
- Required: No

When `hasSearch` is active and `onSearch` is provided, this controls the value of the search input. Required when the `onSearch` prop is provided.

### `title`

- Type: `string`
- Required: No

The menu title.
The menu title. It's also the field used by the menu search function.

## Navigation Group Props

Expand Down Expand Up @@ -169,7 +190,7 @@ If provided, renders `a` instead of `button`.
### `item`

- Type: `string`
- Required: Yes
- Required: No

The unique identifier of the item.

Expand Down
9 changes: 9 additions & 0 deletions packages/components/src/navigation/group/context.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/**
* WordPress dependencies
*/
import { createContext, useContext } from '@wordpress/element';

export const NavigationGroupContext = createContext( { group: undefined } );

export const useNavigationGroupContext = () =>
useContext( NavigationGroupContext );
56 changes: 39 additions & 17 deletions packages/components/src/navigation/group/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,35 +2,57 @@
* External dependencies
*/
import classnames from 'classnames';
import { find, uniqueId } from 'lodash';

/**
* WordPress dependencies
*/
import { useState } from '@wordpress/element';

/**
* Internal dependencies
*/
import { NavigationGroupContext } from './context';
import { GroupTitleUI } from '../styles/navigation-styles';
import { useNavigationMenuContext } from '../menu/context';
import { useNavigationContext } from '../context';

export default function NavigationGroup( { children, className, title } ) {
const { isActive } = useNavigationMenuContext();
const [ groupId ] = useState( uniqueId( 'group-' ) );
const {
navigationTree: { items },
} = useNavigationContext();

const context = { group: groupId };

// Keep the children rendered to make sure inactive items are included in the navigation tree
if ( ! isActive ) {
return children;
// Keep the children rendered to make sure invisible items are included in the navigation tree.
if ( ! find( items, { group: groupId, _isVisible: true } ) ) {
return (
<NavigationGroupContext.Provider value={ context }>
{ children }
</NavigationGroupContext.Provider>
);
}

const groupTitleId = `components-navigation__group-title-${ groupId }`;
const classes = classnames( 'components-navigation__group', className );

return (
<li className={ classes }>
{ title && (
<GroupTitleUI
as="h3"
className="components-navigation__group-title"
variant="caption"
>
{ title }
</GroupTitleUI>
) }
<ul>{ children }</ul>
</li>
<NavigationGroupContext.Provider value={ context }>
<li className={ classes }>
{ title && (
<GroupTitleUI
as="h3"
className="components-navigation__group-title"
id={ groupTitleId }
variant="caption"
>
{ title }
</GroupTitleUI>
) }
<ul aria-labelledby={ groupTitleId } role="group">
{ children }
</ul>
</li>
</NavigationGroupContext.Provider>
);
}
23 changes: 13 additions & 10 deletions packages/components/src/navigation/item/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,21 @@
* External dependencies
*/
import classnames from 'classnames';
import { noop } from 'lodash';
import { noop, uniqueId } from 'lodash';

/**
* WordPress dependencies
*/
import { useState } from '@wordpress/element';
import { Icon, chevronRight } from '@wordpress/icons';

/**
* Internal dependencies
*/
import Button from '../../button';
import { useNavigationContext } from '../context';
import { ItemBadgeUI, ItemTitleUI, ItemUI } from '../styles/navigation-styles';
import { useNavigationTreeItem } from './use-navigation-tree-item';
import { useNavigationMenuContext } from '../menu/context';
import { ItemBadgeUI, ItemTitleUI, ItemUI } from '../styles/navigation-styles';

export default function NavigationItem( props ) {
const {
Expand All @@ -30,14 +30,17 @@ export default function NavigationItem( props ) {
title,
...restProps
} = props;
useNavigationTreeItem( props );
const { activeItem, setActiveMenu } = useNavigationContext();
const { isActive } = useNavigationMenuContext();

// If this item is in an inactive menu, then we skip rendering
// We need to make sure this component gets mounted though
// To make sure inactive items are included in the navigation tree
if ( ! isActive ) {
const [ itemId ] = useState( uniqueId( 'item-' ) );

useNavigationTreeItem( itemId, props );
const {
activeItem,
navigationTree,
setActiveMenu,
} = useNavigationContext();

if ( ! navigationTree.getItem( itemId )?._isVisible ) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,32 @@ import { useEffect } from '@wordpress/element';
* Internal dependencies
*/
import { useNavigationContext } from '../context';
import { useNavigationGroupContext } from '../group/context';
import { useNavigationMenuContext } from '../menu/context';
import { normalizedSearch } from '../utils';

export const useNavigationTreeItem = ( props ) => {
export const useNavigationTreeItem = ( itemId, props ) => {
const {
activeMenu,
navigationTree: { addItem, removeItem },
} = useNavigationContext();
const { menu } = useNavigationMenuContext();
const { group } = useNavigationGroupContext();
const { menu, search } = useNavigationMenuContext();

const key = props.item;
useEffect( () => {
addItem( key, { ...props, menu } );
const isMenuActive = activeMenu === menu;
const isItemVisible =
! search || normalizedSearch( props.title, search );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine for now, but I think we'll need to think of how to search from external dataset whose items are not completely included in navigation menu? For example, we might have Pages navigation with thousands of items, and we might not want to render them all within the menu.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an onSearch prop that can be used to handle this kinds of things, e.g. to perform an actual database search.
(At least that's the idea, but I haven't tested it in a real-life scenario)


addItem( itemId, {
...props,
group,
menu,
_isVisible: isMenuActive && isItemVisible,
} );

return () => {
removeItem( key );
removeItem( itemId );
};
}, [] );
}, [ activeMenu, search ] );
};
2 changes: 1 addition & 1 deletion packages/components/src/navigation/menu/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { createContext, useContext } from '@wordpress/element';

export const NavigationMenuContext = createContext( {
menu: undefined,
isActive: false,
search: '',
} );
export const useNavigationMenuContext = () =>
useContext( NavigationMenuContext );
56 changes: 37 additions & 19 deletions packages/components/src/navigation/menu/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,46 +3,63 @@
*/
import classnames from 'classnames';

/**
* WordPress dependencies
*/
import { useState } from '@wordpress/element';

/**
* Internal dependencies
*/
import { ROOT_MENU } from '../constants';
import { useNavigationContext } from '../context';
import { MenuTitleUI, MenuUI } from '../styles/navigation-styles';
import NavigationBackButton from '../back-button';
import { NavigationMenuContext } from './context';
import { useNavigationContext } from '../context';
import { useNavigationTreeMenu } from './use-navigation-tree-menu';
import NavigationBackButton from '../back-button';
import NavigationMenuTitle from './menu-title';
import { NavigableMenu } from '../../navigable-container';
import { MenuUI } from '../styles/navigation-styles';

export default function NavigationMenu( props ) {
const {
backButtonLabel,
children,
className,
hasSearch,
menu = ROOT_MENU,
onSearch: setControlledSearch,
parentMenu,
search: controlledSearch,
title,
onBackButtonClick,
} = props;
const [ uncontrolledSearch, setUncontrolledSearch ] = useState( '' );
useNavigationTreeMenu( props );
const { activeMenu } = useNavigationContext();
const isActive = activeMenu === menu;

const classes = classnames( 'components-navigation__menu', className );

const context = {
menu,
isActive,
search: uncontrolledSearch,
};

// Keep the children rendered to make sure inactive items are included in the navigation tree
if ( ! isActive ) {
// Keep the children rendered to make sure invisible items are included in the navigation tree
if ( activeMenu !== menu ) {
return (
<NavigationMenuContext.Provider value={ context }>
{ children }
</NavigationMenuContext.Provider>
);
}

const isControlledSearch = !! setControlledSearch;
const search = isControlledSearch ? controlledSearch : uncontrolledSearch;
const onSearch = isControlledSearch
? setControlledSearch
: setUncontrolledSearch;

const menuTitleId = `components-navigation__menu-title-${ menu }`;
const classes = classnames( 'components-navigation__menu', className );

return (
<NavigationMenuContext.Provider value={ context }>
<MenuUI className={ classes }>
Expand All @@ -53,16 +70,17 @@ export default function NavigationMenu( props ) {
onClick={ onBackButtonClick }
/>
) }
{ title && (
<MenuTitleUI
as="h2"
className="components-navigation__menu-title"
variant="subtitle"
>
{ title }
</MenuTitleUI>
) }
<ul>{ children }</ul>

<NavigationMenuTitle
hasSearch={ hasSearch }
onSearch={ onSearch }
search={ search }
title={ title }
/>

<NavigableMenu>
<ul aria-labelledby={ menuTitleId }>{ children }</ul>
</NavigableMenu>
</MenuUI>
</NavigationMenuContext.Provider>
);
Expand Down
Loading