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 panel: Add search to content menus #27280

Merged
merged 20 commits into from
Feb 9, 2021
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,5 @@ export const MENU_TEMPLATES = 'templates';
export const MENU_TEMPLATES_ALL = 'templates-all';
export const MENU_TEMPLATES_PAGES = 'templates-pages';
export const MENU_TEMPLATES_POSTS = 'templates-posts';

export const SEARCH_DEBOUNCE_IN_MS = 75;
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* WordPress dependencies
*/
import { __experimentalNavigationItem as NavigationItem } from '@wordpress/components';
import { useDispatch } from '@wordpress/data';
import { useCallback } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { getPathAndQueryString } from '@wordpress/url';

const getTitle = ( entity ) =>
entity.taxonomy ? entity.name : entity?.title?.rendered;

export default function ContentNavigationItem( { item } ) {
const { setPage } = useDispatch( 'core/edit-site' );

const onActivateItem = useCallback( () => {
const { type, slug, link, id } = item;
setPage( {
type,
slug,
path: getPathAndQueryString( link ),
context: {
postType: type,
postId: id,
},
} );
}, [ setPage, item ] );

if ( ! item ) {
return null;
}

return (
<NavigationItem
className="edit-site-navigation-panel__content-item"
item={ `${ item.taxonomy || item.type }-${ item.id }` }
title={ getTitle( item ) || __( '(no title)' ) }
onClick={ onActivateItem }
/>
);
}
Original file line number Diff line number Diff line change
@@ -1,23 +1,85 @@
/**
* WordPress dependencies
*/
import { __experimentalNavigationMenu as NavigationMenu } from '@wordpress/components';
import {
__experimentalNavigationMenu as NavigationMenu,
__experimentalNavigationItem as NavigationItem,
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { useSelect } from '@wordpress/data';

/**
* Internal dependencies
*/
import NavigationEntityItems from '../navigation-entity-items';
import { MENU_CONTENT_CATEGORIES, MENU_ROOT } from '../constants';
import ContentNavigationItem from '../content-navigation-item';
import SearchResults from '../search-results';
import useDebouncedSearch from '../use-debounced-search';

export default function ContentCategoriesMenu() {
const {
search,
searchQuery,
onSearch,
isDebouncing,
} = useDebouncedSearch();

const { categories, isResolved } = useSelect(
( select ) => {
const { getEntityRecords, hasFinishedResolution } = select(
'core'
);
const getEntityRecordsArgs = [
'taxonomy',
'category',
{
search: searchQuery,
},
];
const hasResolvedPosts = hasFinishedResolution(
'getEntityRecords',
getEntityRecordsArgs
);
return {
categories: getEntityRecords( ...getEntityRecordsArgs ),
isResolved: hasResolvedPosts,
};
},
[ searchQuery ]
);

const shouldShowLoadingForDebouncing = search && isDebouncing;
const showLoading = ! isResolved || shouldShowLoadingForDebouncing;

return (
<NavigationMenu
menu={ MENU_CONTENT_CATEGORIES }
title={ __( 'Categories' ) }
parentMenu={ MENU_ROOT }
hasSearch={ true }
onSearch={ onSearch }
search={ search }
Addison-Stavlo marked this conversation as resolved.
Show resolved Hide resolved
isSearchDebouncing={ isDebouncing || ! isResolved }
>
<NavigationEntityItems kind="taxonomy" name="category" />
{ search && ! isDebouncing && (
<SearchResults
items={ categories }
search={ search }
disableFilter
/>
) }

{ ! search &&
categories?.map( ( category ) => (
<ContentNavigationItem
item={ category }
key={ `${ category.taxonomy }-${ category.id }` }
/>
) ) }

{ showLoading && (
<NavigationItem title={ __( 'Loading…' ) } isText />
) }
</NavigationMenu>
);
}
Original file line number Diff line number Diff line change
@@ -1,23 +1,85 @@
/**
* WordPress dependencies
*/
import { __experimentalNavigationMenu as NavigationMenu } from '@wordpress/components';
import {
__experimentalNavigationMenu as NavigationMenu,
__experimentalNavigationItem as NavigationItem,
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { useSelect } from '@wordpress/data';

/**
* Internal dependencies
*/
import NavigationEntityItems from '../navigation-entity-items';
import { MENU_CONTENT_PAGES, MENU_ROOT } from '../constants';
import ContentNavigationItem from '../content-navigation-item';
import SearchResults from '../search-results';
import useDebouncedSearch from '../use-debounced-search';

export default function ContentPagesMenu() {
const {
search,
searchQuery,
onSearch,
isDebouncing,
} = useDebouncedSearch();

const { pages, isResolved } = useSelect(
( select ) => {
const { getEntityRecords, hasFinishedResolution } = select(
'core'
);
const getEntityRecordsArgs = [
'postType',
'page',
{
search: searchQuery,
Copy link
Member

Choose a reason for hiding this comment

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

},
];
const hasResolvedPosts = hasFinishedResolution(
'getEntityRecords',
getEntityRecordsArgs
);
return {
pages: getEntityRecords( ...getEntityRecordsArgs ),
isResolved: hasResolvedPosts,
};
},
[ searchQuery ]
);

const shouldShowLoadingForDebouncing = search && isDebouncing;
const showLoading = ! isResolved || shouldShowLoadingForDebouncing;

return (
<NavigationMenu
menu={ MENU_CONTENT_PAGES }
title={ __( 'Pages' ) }
parentMenu={ MENU_ROOT }
hasSearch={ true }
onSearch={ onSearch }
search={ search }
isSearchDebouncing={ isDebouncing || ! isResolved }
>
<NavigationEntityItems kind="postType" name="page" />
{ search && ! isDebouncing && (
<SearchResults
items={ pages }
search={ search }
disableFilter
/>
) }

{ ! search &&
pages?.map( ( page ) => (
<ContentNavigationItem
item={ page }
key={ `${ page.type }-${ page.id }` }
/>
) ) }

{ showLoading && (
<NavigationItem title={ __( 'Loading…' ) } isText />
) }
</NavigationMenu>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,57 @@ import {
__experimentalNavigationMenu as NavigationMenu,
__experimentalNavigationItem as NavigationItem,
} from '@wordpress/components';
import { useDispatch, useSelect } from '@wordpress/data';
import { __ } from '@wordpress/i18n';
import { useCallback } from '@wordpress/element';
import { useDispatch, useSelect } from '@wordpress/data';

/**
* Internal dependencies
*/
import NavigationEntityItems from '../navigation-entity-items';
import { MENU_CONTENT_POSTS, MENU_ROOT } from '../constants';
import ContentNavigationItem from '../content-navigation-item';
import SearchResults from '../search-results';
import useDebouncedSearch from '../use-debounced-search';

export default function ContentPostsMenu() {
const showOnFront = useSelect(
( select ) =>
select( 'core' ).getEditedEntityRecord( 'root', 'site' )
.show_on_front,
[]
const {
search,
searchQuery,
onSearch,
isDebouncing,
} = useDebouncedSearch();

const { posts, showOnFront, isResolved } = useSelect(
( select ) => {
const {
getEntityRecords,
getEditedEntityRecord,
hasFinishedResolution,
} = select( 'core' );
const getEntityRecodsArgs = [
'postType',
'post',
{
search: searchQuery,
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 that this will implicitly search the content of the posts too. My expectation was that the search will only be applied to post titles so as to make it consistent with existing template searching, but this might make sense too. What do you all think? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

My expectation was that the search will only be applied to post titles so as to make it consistent with existing template searching, but this might make sense too.

Same thought here. While I would expect it the component to search only by titles, returning results for some matching content may be nice! Although if we go with that we should probably ensure the results are prioritized with those matching titles to show up first.

Copy link
Member Author

Choose a reason for hiding this comment

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

As far as I know, there is no way to search only in the title. Is there? 🤔 We might need to come up with a custom endpoint if we want to search by title only or do the prioritization.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think its a bad thing that we search content, its a feature not a bug! However, I do have a complaint about how it orders things. The results seem to be ordered based on the date of the post. So if I am look for 'example', the posts with the word 'example' in their title show up after more recent posts that have the word 'example' in their content:

Screen Shot 2021-01-27 at 7 30 10 PM

I guess my recommendation would be to sort the results such that results with titles corresponding to the search are at the top.

Copy link
Member Author

Choose a reason for hiding this comment

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

Pushed a commit which puts results with titles at the top.

},
];
const hasResolvedPosts = hasFinishedResolution(
'getEntityRecords',
getEntityRecodsArgs
);
return {
posts: getEntityRecords( ...getEntityRecodsArgs ),
isResolved: hasResolvedPosts,
showOnFront: getEditedEntityRecord( 'root', 'site' )
.show_on_front,
};
},
[ searchQuery ]
);

const { setPage } = useDispatch( 'core/edit-site' );

const onActivateFrontItem = () => {
const onActivateFrontItem = useCallback( () => {
setPage( {
type: 'page',
path: '/',
Expand All @@ -33,22 +64,51 @@ export default function ContentPostsMenu() {
queryContext: { page: 1 },
},
} );
};
}, [ setPage ] );

const shouldShowLoadingForDebouncing = search && isDebouncing;
const showLoading = ! isResolved || shouldShowLoadingForDebouncing;

return (
<NavigationMenu
menu={ MENU_CONTENT_POSTS }
title={ __( 'Posts' ) }
parentMenu={ MENU_ROOT }
hasSearch={ true }
onSearch={ onSearch }
search={ search }
isSearchDebouncing={ isDebouncing || ! isResolved }
>
{ showOnFront === 'posts' && (
<NavigationItem
item={ 'content-/' }
title={ __( 'All Posts' ) }
onClick={ onActivateFrontItem }
{ search && ! isDebouncing && (
<SearchResults
items={ posts }
search={ search }
disableFilter
/>
) }
<NavigationEntityItems kind="postType" name="post" />

{ ! search && (
<>
{ showOnFront === 'posts' && (
<NavigationItem
item={ 'post-/' }
title={ __( 'All Posts' ) }
onClick={ onActivateFrontItem }
/>
) }
Comment on lines +93 to +99
Copy link
Member Author

Choose a reason for hiding this comment

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

All Posts is not included in the search results. Should we also include this?

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 it's fine to omit it. Furthermore I'm not sure if we should be displaying it in posts menu anyway since it sounds more like page template to me. This should probably be reserved just for individual posts and not a collection of them.


{ posts?.map( ( post ) => (
<ContentNavigationItem
item={ post }
key={ `${ post.type }-${ post.id }` }
/>
) ) }
</>
) }

{ showLoading && (
<NavigationItem title={ __( 'Loading…' ) } isText />
) }
</NavigationMenu>
);
}
Loading