Navigation Component: Search Control in Menu Titles #25315

Copons · 2020-09-14T18:42:10Z

Description

Introduce a new NavigationMenuTitle component that can optionally become a search box, filtering the menu items.

Fixes #25252

Things to do

The search uses the appear animation instead of slide-in. This is because the parent component uses slide-in, and for some reasons whenever I toggle the search box, both animations would be triggered. (Solved: it was caused by the input gaining focus during the animation, which scrolled it into view, messing up the animation.)
~~My intention would be to make the input field to look like the inserter search box, which is heavily customized and requires more involvement than I initially thought.~~ (Not worthy: a normal TextControl will do just fine)
There are a lot of magic numbers in the CSS, I'm aware of it. 🙂
~~Maybe externalize the normalize function, also used by the inserter search.~~ (It has a very low footprint and it's used by several other components already. We might want to create a general utility for it, but this is not the right place.)
Gracefully handle empty NavigationGroup.
Add an onSearch function to handle custom search logic (this might require its own example in the storybook).
~~Explore if there's a simple way to search inside items rendering custom components.~~ (Discussion moved to Navigation Component: What to Search and How #25509)

How has this been tested?

npm run storybook:dev
Head over to Components / Navigation.
The "Home" menu is now searchable.

Screenshots

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR.

github-actions · 2020-09-14T18:51:44Z

Size Change: +2.82 kB (0%)

Total Size: 1.2 MB

Filename	Size	Change
`build/a11y/index.js`	1.14 kB	-1 B
`build/annotations/index.js`	3.54 kB	-1 B
`build/blob/index.js`	667 B	-1 B
`build/block-directory/index.js`	8.6 kB	+1 B
`build/block-editor/index.js`	130 kB	-1 B
`build/block-editor/style-rtl.css`	11 kB	+80 B (0%)
`build/block-editor/style.css`	11 kB	+77 B (0%)
`build/block-library/index.js`	145 kB	+1.27 kB (0%)
`build/block-library/style-rtl.css`	7.75 kB	+58 B (0%)
`build/block-library/style.css`	7.75 kB	+58 B (0%)
`build/blocks/index.js`	47.6 kB	+2 B (0%)
`build/components/index.js`	171 kB	+1.28 kB (0%)
`build/components/style-rtl.css`	15.4 kB	-6 B (0%)
`build/components/style.css`	15.4 kB	-3 B (0%)
`build/core-data/index.js`	12.1 kB	-1 B
`build/data/index.js`	8.63 kB	+4 B (0%)
`build/date/index.js`	31.9 kB	-1 B
`build/edit-navigation/index.js`	10.8 kB	+1 B
`build/edit-post/index.js`	306 kB	-1 B
`build/edit-site/index.js`	21.7 kB	+36 B (0%)
`build/edit-site/style-rtl.css`	3.79 kB	-14 B (0%)
`build/edit-site/style.css`	3.79 kB	-16 B (0%)
`build/edit-widgets/index.js`	26.6 kB	+1 B
`build/editor/index.js`	42.6 kB	-4 B (0%)
`build/element/index.js`	4.45 kB	+2 B (0%)
`build/redux-routine/index.js`	2.85 kB	+4 B (0%)

ℹ️ View Unchanged

Filename	Size	Change
`build/api-fetch/index.js`	3.35 kB	0 B
`build/autop/index.js`	2.72 kB	0 B
`build/block-directory/style-rtl.css`	943 B	0 B
`build/block-directory/style.css`	942 B	0 B
`build/block-library/editor-rtl.css`	8.93 kB	0 B
`build/block-library/editor.css`	8.93 kB	0 B
`build/block-library/theme-rtl.css`	741 B	0 B
`build/block-library/theme.css`	741 B	0 B
`build/block-serialization-default-parser/index.js`	1.77 kB	0 B
`build/block-serialization-spec-parser/index.js`	3.1 kB	0 B
`build/compose/index.js`	9.63 kB	0 B
`build/data-controls/index.js`	684 B	0 B
`build/deprecated/index.js`	772 B	0 B
`build/dom-ready/index.js`	569 B	0 B
`build/dom/index.js`	4.43 kB	0 B
`build/edit-navigation/style-rtl.css`	881 B	0 B
`build/edit-navigation/style.css`	885 B	0 B
`build/edit-post/style-rtl.css`	6.37 kB	0 B
`build/edit-post/style.css`	6.35 kB	0 B
`build/edit-widgets/style-rtl.css`	3.09 kB	0 B
`build/edit-widgets/style.css`	3.09 kB	0 B
`build/editor/editor-styles-rtl.css`	480 B	0 B
`build/editor/editor-styles.css`	482 B	0 B
`build/editor/style-rtl.css`	3.85 kB	0 B
`build/editor/style.css`	3.84 kB	0 B
`build/escape-html/index.js`	733 B	0 B
`build/format-library/index.js`	7.49 kB	0 B
`build/format-library/style-rtl.css`	547 B	0 B
`build/format-library/style.css`	548 B	0 B
`build/hooks/index.js`	1.74 kB	0 B
`build/html-entities/index.js`	622 B	0 B
`build/i18n/index.js`	3.54 kB	0 B
`build/is-shallow-equal/index.js`	709 B	0 B
`build/keyboard-shortcuts/index.js`	2.39 kB	0 B
`build/keycodes/index.js`	1.85 kB	0 B
`build/list-reusable-blocks/index.js`	3.02 kB	0 B
`build/list-reusable-blocks/style-rtl.css`	476 B	0 B
`build/list-reusable-blocks/style.css`	476 B	0 B
`build/media-utils/index.js`	5.12 kB	0 B
`build/notices/index.js`	1.69 kB	0 B
`build/nux/index.js`	3.27 kB	0 B
`build/nux/style-rtl.css`	671 B	0 B
`build/nux/style.css`	668 B	0 B
`build/plugins/index.js`	2.45 kB	0 B
`build/primitives/index.js`	1.35 kB	0 B
`build/priority-queue/index.js`	789 B	0 B
`build/reusable-blocks/index.js`	3.06 kB	0 B
`build/rich-text/index.js`	13 kB	0 B
`build/server-side-render/index.js`	2.61 kB	0 B
`build/shortcode/index.js`	1.7 kB	0 B
`build/token-list/index.js`	1.24 kB	0 B
`build/url/index.js`	4.06 kB	0 B
`build/viewport/index.js`	1.75 kB	0 B
`build/warning/index.js`	1.13 kB	0 B
`build/wordcount/index.js`	1.23 kB	0 B

_{compressed-size-action}

Copons · 2020-09-17T18:56:48Z

@david-szabo97 Let me loop you in on what I'm proposing here, which contains some changes on the navigation tree structure.

tl;dr: we have the tree, let's use it! 😄

Item `_isVisible` instead of menu `isActive`

With the search, items can be hidden because of 2 reasons: 1) the belong to an inactive menu; 2) they are filtered out by the search.
This means that NavigationMenuContext's isActive wasn't enough anymore, and so I've removed it altogether.

In its stead, I've added a new _isVisible property to the navigation tree's items.
Now the nav tree items update whenever activeMenu or search (the search field state) change.

This property (accessible for example with navigationTree.getItem( item )?._isVisible) includes by default the active menu check, and adds the search filtering as well.

Implementation

NavigationItem uses it directly to determine if the item should be rendered or not.

NavigationMenu replaces the isActive context with a direct activeMenu !== menu check.

NavigationGroup is more complicated.
We don't have it in the tree, and since it's almost entirely a presentational wrapper, I'd rather keep it out unless really unavoidable.
Loop with Children.forEach: if all its children are marked as not _isVisible in the tree, then the group is empty. It doesn't need to render anything, but it can just pass-through children (which are all "invisible" items, so they eventually won't be rendered at all).

Note: apparently there's no way to break from a Children.forEach. It's a bummer, but probably not a big deal? We could turn it into an array with Children.toArray and use a normal loop, but I guess that would negate the perf gain anyway. 🤷

david-szabo97 · 2020-09-18T09:29:32Z

Looks good to me! Any reason to use _ prefix? It's a computed property but I don't think it should be a marked as private (if that was the intention).

apparently there's no way to break from a Children.forEach.

Too bad, probably it's still cheaper to loop through than toArray.

david-szabo97 · 2020-09-18T12:54:59Z

Explore if there's a simple way to search inside items rendering custom components.

This looks promising: https://github.com/fernandopasik/react-children-utilities/blob/master/docs/only-text.md
Maybe we should replicate this to achieve searching in custom components.

Copons · 2020-09-20T11:41:28Z

Looks good to me! Any reason to use _ prefix? It's a computed property but I don't think it should be a marked as private (if that was the intention).

It was both to distinguish it from the components props, and to avoid clashing with a hypothetical isVisible prop.

Explore if there's a simple way to search inside items rendering custom components.

This looks promising: https://github.com/fernandopasik/react-children-utilities/blob/master/docs/only-text.md
Maybe we should replicate this to achieve searching in custom components.

The implementation recurses through all children, which could be expensive with a lot of complicated custom items.
Another way would be to get the Node.textContent() of the rendered item, but keeping the ref is expensive as well. 😓

Just throwing out a random idea: what if we had a keywords prop, similar in function to the blocks' keywords?
(Probably very overkill though)

david-szabo97 · 2020-09-21T10:49:28Z

Just throwing out a random idea: what if we had a keywords prop, similar in function to the blocks' keywords?

Well, this is the least expensive way to do it. I'm not sure if it's developer friendly though. It's a good starting point and we might need this for something else as well. (NavigationItem's label might not be the best describing text for searching in every case)

Copons · 2020-09-21T13:43:22Z

Just throwing out a random idea: what if we had a keywords prop, similar in function to the blocks' keywords?

Well, this is the least expensive way to do it. I'm not sure if it's developer friendly though. It's a good starting point and we might need this for something else as well. (NavigationItem's label might not be the best describing text for searching in every case)

Yup.
I think it's worth discussing it separately, as it's a decision we would eventually need to stick to indefinitely.
I'll open an issue for that (✅ #25509), so that we could wrap this up with just the title search. 🙂

shaunandrews · 2020-09-24T18:17:34Z

I think the icon should be white, not blue.

--

I noticed the focus outline gets cutoff for the search button.

--

As I'm searching, I expected to be able to use the up/down arrow keys to navigate the search results.

--

I'm able to use the tab key to move focus to the x button to close the search, but when it closes my keyboard's focus disappears; I'd expect it to return to the search button after closing the input.

--

I expect to be able to hit the esc button to close the search.

…onal item prop

mattwiebe · 2020-10-20T18:45:20Z

I was a bit confused because I didn't read the testing directions properly and was trying to figure out why the search wasn't showing up in the Site Editor sidebar.

But now that I tested in Storybook, it looks and works well for me. I wasn't sure if it would search only within the subsection (Controlled Search) but the placeholder adapts nicely to that ("Search in Controlled Search"), which is a nice touch.

But coming back to my initial confusion, are we intending to use the search on the Site Editor sidebar?

packages/components/src/navigation/menu/menu-title.js

david-szabo97 · 2020-10-21T07:25:16Z

packages/components/src/navigation/utils.js

+ */
+import { deburr } from 'lodash';
+
+// @see packages/block-editor/src/components/inserter/search-items.js


Would it worth opening an issue to extract this as a util function? So we can reuse it in both places.

Yes definitely!
My intention was to open a follow up right after merging this, and this is a nice reminder, since at this point I'm sure I'd have forgot about it. 😄

david-szabo97 · 2020-10-21T07:48:53Z

Moving into a submenu, the focus is lost. I'm not sure what would be the best a11y-wise. Maybe we should focus on the first item or the back button? 🤔

EDIT: I don't think this is a blocker. Fine in a separate PR so we can merge this and get the ball rolling.

Copons · 2020-10-21T11:10:04Z

But coming back to my initial confusion, are we intending to use the search on the Site Editor sidebar?

@mattwiebe Yes.
There are at least two locations where it would be useful:

Search for templates, especially when we'll remove the "All templates" menu, currently used for testing purposes.
Search for content. When we moved the content switcher from the header toolbar, we lost the "search for post" functionality (because its component's style was heavily clashing with the sidebar). That imho would be a nice way to test this built-in search in a real case scenario.

Moving into a submenu, the focus is lost. I'm not sure what would be the best a11y-wise. Maybe we should focus on the first item or the back button? 🤔

EDIT: I don't think this is a blocker. Fine in a separate PR so we can merge this and get the ball rolling.

@david-szabo97 I see what you mean... 🤔
The focus is indeed lost, but a single tab hit would move it to the right place (the top of the sidebar, which in this case would be the "back to parent menu" button).

Probably not a big deal, and not really related to the search function. 🙂

vindl

This is testing fine for me and the code looks good! 🚀

Copons added [Status] In Progress Tracking issues with work in progress Needs Design Feedback Needs general design feedback. [Feature] Navigation Component A navigational waterfall component for hierarchy of items. labels Sep 14, 2020

Copons requested review from ajitbohra, chrisvanpatten and youknowriad as code owners September 14, 2020 18:42

Copons self-assigned this Sep 14, 2020

Copons mentioned this pull request Sep 15, 2020

Navigation component: Store navigation tree in context #25292

Closed

6 tasks

Copons force-pushed the update/navigation-search branch 2 times, most recently from 835cd6b to 654836e Compare September 17, 2020 15:07

Copons mentioned this pull request Sep 21, 2020

Navigation Component: What to Search and How #25509

Closed

Copons force-pushed the update/navigation-search branch 3 times, most recently from 24613fb to 4776170 Compare September 23, 2020 10:02

Copons changed the title ~~[WIP] Navigation Component: Search Control in Menu Titles~~ Navigation Component: Search Control in Menu Titles Sep 23, 2020

Copons removed the [Status] In Progress Tracking issues with work in progress label Sep 23, 2020

Copons requested review from david-szabo97, joshuatf, psealock, vindl, shaunandrews and mattwiebe September 23, 2020 17:02

Copons added 9 commits October 20, 2020 12:28

Update the story

c4c947e

Add aria labels

34a0ce5

Simplify CSS

1e1daf3

Use space function instead of hardcoded values

b51edcd

Announce the number of search results

f40f9ac

Add group roles and labels

8b90be6

Add more aria-labelledby

5a63a2e

Test without listitem role

642332d

Add an internal unique ID to NavigationItem instead of using the opti…

25f33f9

…onal item prop

Copons force-pushed the update/navigation-search branch from e5e3d2b to 25f33f9 Compare October 20, 2020 11:28

Copons added 2 commits October 20, 2020 13:02

Add a group context to handle empty groups

792f368

Remove tooltips (but keep labels)

15a9fe6

Copons mentioned this pull request Oct 20, 2020

Navigation Component: Hide navigation item if target menu is empty #25746

Merged

6 tasks

david-szabo97 reviewed Oct 21, 2020

View reviewed changes

packages/components/src/navigation/menu/menu-title.js Outdated Show resolved Hide resolved

david-szabo97 reviewed Oct 21, 2020

View reviewed changes

Move the search focus delay into a constant

4dc2c3b

vindl added the [Feature] Full Site Editing label Oct 21, 2020

vindl approved these changes Oct 21, 2020

View reviewed changes

david-szabo97 approved these changes Oct 22, 2020

View reviewed changes

Copons merged commit 384c30f into master Oct 23, 2020

Copons deleted the update/navigation-search branch October 23, 2020 10:45

github-actions bot added this to the Gutenberg 9.3 milestone Oct 23, 2020

This was referenced Oct 27, 2020

Site Editor: Add Search Template and Template Part to the Sidebar #26512

Closed

Site Editor: Add Search Posts and Pages to the Sidebar #26513

Closed

joshuatf mentioned this pull request Mar 10, 2021

Fix hidden menu title on smaller screens woocommerce/woocommerce-admin#6562

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Navigation Component: Search Control in Menu Titles #25315

Navigation Component: Search Control in Menu Titles #25315

Copons commented Sep 14, 2020 •

edited

Loading

github-actions bot commented Sep 14, 2020 •

edited

Loading

Copons commented Sep 17, 2020

david-szabo97 commented Sep 18, 2020 •

edited

Loading

david-szabo97 commented Sep 18, 2020

Copons commented Sep 20, 2020 •

edited

Loading

david-szabo97 commented Sep 21, 2020

Copons commented Sep 21, 2020 •

edited

Loading

shaunandrews commented Sep 24, 2020 •

edited

Loading

mattwiebe commented Oct 20, 2020

david-szabo97 Oct 21, 2020

Copons Oct 21, 2020

david-szabo97 commented Oct 21, 2020 •

edited

Loading

Copons commented Oct 21, 2020

vindl left a comment

Navigation Component: Search Control in Menu Titles #25315

Navigation Component: Search Control in Menu Titles #25315

Conversation

Copons commented Sep 14, 2020 • edited Loading

Description

Things to do

How has this been tested?

Screenshots

Types of changes

Checklist:

github-actions bot commented Sep 14, 2020 • edited Loading

Copons commented Sep 17, 2020

Item _isVisible instead of menu isActive

Implementation

david-szabo97 commented Sep 18, 2020 • edited Loading

david-szabo97 commented Sep 18, 2020

Copons commented Sep 20, 2020 • edited Loading

david-szabo97 commented Sep 21, 2020

Copons commented Sep 21, 2020 • edited Loading

shaunandrews commented Sep 24, 2020 • edited Loading

mattwiebe commented Oct 20, 2020

david-szabo97 Oct 21, 2020

Choose a reason for hiding this comment

Copons Oct 21, 2020

Choose a reason for hiding this comment

david-szabo97 commented Oct 21, 2020 • edited Loading

Copons commented Oct 21, 2020

vindl left a comment

Choose a reason for hiding this comment

Copons commented Sep 14, 2020 •

edited

Loading

github-actions bot commented Sep 14, 2020 •

edited

Loading

Item `_isVisible` instead of menu `isActive`

david-szabo97 commented Sep 18, 2020 •

edited

Loading

Copons commented Sep 20, 2020 •

edited

Loading

Copons commented Sep 21, 2020 •

edited

Loading

shaunandrews commented Sep 24, 2020 •

edited

Loading

david-szabo97 commented Oct 21, 2020 •

edited

Loading