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.

Sign up for GitHub

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

Fix/content picker improvements #19

Merged
merged 4 commits into from
Mar 31, 2021
Merged

Fix/content picker improvements #19

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6f3fa90
Make class namespaces more accurate; don't show type label in search …
tlovett1 Mar 18, 2021
0c83209
Styling for content search without type
tlovett1 Mar 18, 2021
cf8be4f
Add caching to content search
tlovett1 Mar 18, 2021
7f14c3e
Properly cache search query; bail on api fetch if unmounted
tlovett1 Mar 30, 2021
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
2 changes: 1 addition & 1 deletion components/ContentPicker/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { __ } from '@wordpress/i18n';
import { ContentSearch } from '../ContentSearch';
import SortableList from './SortableList';

const NAMESPACE = '10up-block-components';
const NAMESPACE = '10up-content-picker';

/**
* Content Picker
Expand Down
22 changes: 18 additions & 4 deletions components/ContentSearch/SearchItem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import { Button, TextHighlight } from '@wordpress/components';
* @param props.id
* @return {*} React JSX
*/
const SearchItem = ({ suggestion, onClick, searchTerm, isSelected, id }) => {
const SearchItem = ({ suggestion, onClick, searchTerm, isSelected, id, contentTypes }) => {
const showType = suggestion.type && contentTypes.length > 1;
johnwatkins0 marked this conversation as resolved.
Show resolved Hide resolved

return (
<Button
id={id}
Expand All @@ -27,14 +29,25 @@ const SearchItem = ({ suggestion, onClick, searchTerm, isSelected, id }) => {
}}
>
<span className="block-editor-link-control__search-item-header">
<span className="block-editor-link-control__search-item-title">
<span
className="block-editor-link-control__search-item-title"
style={{
paddingRight: !showType ? 0 : null,
}}
>
<TextHighlight text={decodeEntities(suggestion.title)} highlight={searchTerm} />
</span>
<span aria-hidden className="block-editor-link-control__search-item-info">
<span
aria-hidden
className="block-editor-link-control__search-item-info"
style={{
paddingRight: !showType ? 0 : null,
}}
>
{filterURLForDisplay(safeDecodeURI(suggestion.url)) || ''}
</span>
</span>
{suggestion.type && (
{showType && (
<span className="block-editor-link-control__search-item-type">
{/* Rename 'post_tag' to 'tag'. Ideally, the API would return the localised CPT or taxonomy label. */}
{suggestion.type === 'post_tag' ? 'tag' : suggestion.type}
Expand All @@ -56,6 +69,7 @@ SearchItem.propTypes = {
suggestion: PropTypes.object.isRequired,
onClick: PropTypes.func.isRequired,
isSelected: PropTypes.bool,
contentTypes: PropTypes.array.isRequired,
};

export default SearchItem;
66 changes: 38 additions & 28 deletions components/ContentSearch/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import PropTypes from 'prop-types';
import { __ } from '@wordpress/i18n';
import SearchItem from './SearchItem';

const NAMESPACE = '10up-block-components';
const NAMESPACE = '10up-content-search';

const searchCache = {};
johnwatkins0 marked this conversation as resolved.
Show resolved Hide resolved

const ContentSearch = ({ onSelectItem, placeholder, label, contentTypes, mode, excludeItems }) => {
const [searchString, setSearchString] = useState('');
Expand Down Expand Up @@ -51,30 +53,37 @@ const ContentSearch = ({ onSelectItem, placeholder, label, contentTypes, mode, e
setSearchString(keyword);
Copy link
Member

@johnwatkins0 johnwatkins0 Mar 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This may look like overkill and is totally optional for this PR, but once the search cache is converted to state, we can handle search changes a little more efficiently (and close #18) by responding to keyword changes with effect hooks.

/**
	 * handleSearchStringChange
	 *
	 * Using the keyword and the list of tags that are linked to the parent block
	 * search for posts/terms that match and return them to the autocomplete component.
	 *
	 * @param {string} keyword search query string
	 */
	const handleSearchStringChange = (keyword) => {
		setSearchString(keyword);
	}

	useEffect(() => {
		if (isLoading || searchString in searchCache) {
			return;
		}

		setIsLoading(true);

		const searchQuery = `wp/v2/search/?search=${searchString}&subtype=${contentTypes.join(
			',',
		)}&type=${mode}`;

		apiFetch({
			path: searchQuery,
		}).then((results) => {
			const newResults = results.filter((result) => {
				let keep = true;

				if (excludeItems.length) {
					excludeItems.forEach((item) => {
						if (item.id === result.id) {
							keep = false;
						}
					});
				}

				return keep;
			});

			setSearchCache(oldSearchCache => ({...oldSearchCache, [searchString]: newResults}));
			setIsLoading(false);
		});
	}, [isLoading, searchCache, searchString]);

	useEffect(() => {
		if (searchString in searchCache) {
			setSearchResults(searchCache[searchString])
		}
	}, [searchString, searchCache])

The first effect hook will run when the search term changes, when the cache changes, and when isLoading changes. This includes when isLoading becomes false -- guaranteeing that the latest search term will always be queried. But by having that isLoading check at the start of the effect hook, we also ensure that only one fetch request is running at a time.

The second effect hook responds to changes to the search cache. This will run after a successful fetch to populate the results.

setIsLoading(true);

const searchQuery = `wp/v2/search/?search=${keyword}&subtype=${contentTypes.join(
',',
)}&type=${mode}`;

apiFetch({
path: searchQuery,
}).then((results) => {
const newResults = results.filter((result) => {
let keep = true;

if (excludeItems.length) {
excludeItems.forEach((item) => {
if (item.id === result.id) {
keep = false;
}
});
}

return keep;
});

setSearchResults(newResults);
if (searchCache[keyword]) {
setSearchResults(searchCache[keyword]);
setIsLoading(false);
});
} else {
const searchQuery = `wp/v2/search/?search=${keyword}&subtype=${contentTypes.join(
',',
)}&type=${mode}`;

apiFetch({
path: searchQuery,
}).then((results) => {
const newResults = results.filter((result) => {
let keep = true;
johnwatkins0 marked this conversation as resolved.
Show resolved Hide resolved

if (excludeItems.length) {
excludeItems.forEach((item) => {
if (item.id === result.id) {
keep = false;
}
});
}

return keep;
});
johnwatkins0 marked this conversation as resolved.
Show resolved Hide resolved

searchCache[keyword] = newResults;

setSearchResults(newResults);
setIsLoading(false);
});
}
};

return (
Expand All @@ -88,7 +97,7 @@ const ContentSearch = ({ onSelectItem, placeholder, label, contentTypes, mode, e
/>
{hasSearchString ? (
<ul
className={`${NAMESPACE}-grid`}
className={`${NAMESPACE}-list`}
style={{
marginTop: '0',
marginBottom: '0',
Expand All @@ -100,7 +109,7 @@ const ContentSearch = ({ onSelectItem, placeholder, label, contentTypes, mode, e
{isLoading && <Spinner />}
{!isLoading && !hasSearchResults && (
<li
className={`${NAMESPACE}-grid-item components-button`}
className={`${NAMESPACE}-list-item components-button`}
style={{ color: 'inherit', cursor: 'default', paddingLeft: '3px' }}
>
{__('Nothing found.', '10up-block-components')}
Expand All @@ -114,7 +123,7 @@ const ContentSearch = ({ onSelectItem, placeholder, label, contentTypes, mode, e
return (
<li
key={item.id}
className={`${NAMESPACE}-grid-item`}
className={`${NAMESPACE}-list-item`}
style={{
marginBottom: '0',
}}
Expand All @@ -123,6 +132,7 @@ const ContentSearch = ({ onSelectItem, placeholder, label, contentTypes, mode, e
onClick={() => handleItemSelection(item)}
searchTerm={searchString}
suggestion={item}
contentTypes={contentTypes}
isSelected={selectedItem === index + 1}
/>
</li>
Expand All @@ -141,7 +151,7 @@ ContentSearch.defaultProps = {
excludeItems: [],
mode: 'post',
onSelectItem: () => {
console.log('Select!');
console.log('Select!'); // eslint-disable-line no-console
},
};

Expand Down