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

feat: add an AutocompleteWithSuggestions component for reusability #952

Merged
merged 8 commits into from
May 17, 2021

Conversation

dkoo
Copy link
Contributor

@dkoo dkoo commented May 6, 2021

All Submissions:

Changes proposed in this Pull Request:

Adds a new component to the Newspack Components library, which is an extension of the AutocompleteTokenfield that also renders a list of suggestions to choose from. Takes functions as props for fetching suggestions, fetching saved tokens, and what to do when the AutocompleteTokenfield value changes.

This UI pattern is something that @thomasguillot has suggested for several features in progress, including this Author Profile block (not yet begun) and the Newspack Listings parent/child UI.

After this lands, we'll want to push a new version of the package to the NPM library.

How to test the changes in this Pull Request:

It's somewhat hard to test this as an NPM package, but you can test within the Newspack Plugin.

  1. In some wizard view, import an AutocompleteWithSuggestions component from the newspack-components directory. Here's an example of some working code (may need to import additional dependencies such as apiFetch, addQueryArgs, decodeEntities, etc.):
<AutocompleteWithSuggestions
	label={ __( 'Search for a post', 'newspack' ) }
	help={ __(
		'Begin typing post title, click autocomplete result to select.',
		'newspack'
	) }
	fetchSavedPosts={ async postIDs => {
		const posts = await apiFetch( {
			path: addQueryArgs( '/wp/v2/posts', {
				per_page: 100,
				include: postIDs.join( ',' ),
				_fields: 'id,title',
			} ),
		} );

		return posts.map( post => ( {
			value: post.id,
			label: decodeEntities( post.title ) || __( '(no title)', 'newspack' ),
		} ) );
	} }
	fetchSuggestions={ async search => {
		const posts = await apiFetch( {
			path: addQueryArgs( '/wp/v2/posts', {
				search,
				per_page: 10,
				_fields: 'id,title',
			} ),
		} );

		// Format suggestions for FormTokenField display.
		const result = posts.reduce( ( acc, post ) => {
			acc.push( {
				value: post.id,
				label: decodeEntities( post.title.rendered ) || __( '(no title)', 'newspack' ),
			} );

			return acc;
		}, [] );
		return result;
	} }
	postType={ 'post' }
	postTypeLabel={ 'Post' }
	maxLength={ 1 }
	onChange={ _value => console.log( _value ) }
	selectedPost={ null }
/>
  1. In the editor view, observe a component that looks like this, showing your 10 most recent posts in the suggestions list (as returned by the fetchSuggestions prop in the above example):

Screen Shot 2021-05-06 at 12 48 10 PM

  1. To test as an NPM package, you can fudge it by cding into the ./assets/components folder in this repo and running npm install && run prepublishOnly. Then, in another repo that already uses components from the newspack-components NPM package, copy the contents of the ./assets/components folder into your node_modules/newspack-components folder in the second repo. You can then import like import { AutocompleteWithSuggestions } from 'newspack-components' and test using the above example code in an editor page.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@dkoo dkoo added the Component label May 6, 2021
@dkoo dkoo self-assigned this May 6, 2021
@dkoo dkoo added [Status] Needs Review The issue or pull request needs to be reviewed [Status] Needs Design Review labels May 6, 2021
@dkoo dkoo requested a review from claudiulodro May 6, 2021 18:57
@dkoo
Copy link
Contributor Author

dkoo commented May 7, 2021

Putting this in [WIP] mode because we want to extend this component further. A new feature for Newspack Listings needs this component to be able to handle multiple selections at once, like so:

2-new-state

In this use case, the list of suggestions looks and behaves more like a taxonomy select component. I believe it would be possible to extend the component to handle this as well if we provide certain props (perhaps multiSelect={ true } or something similar). If the required props are passed, then the component would render everything in the above screenshot that appears inside the modal, from the "4 listings selected" message and the optional data type selector (which could let you choose which post type or other data type to query), through the list of checkbox options to select multiple options.

EDIT: Let's move forward with this with the single-select functionality only, as other PRs are currently blocked by this.

@dkoo dkoo changed the title feat: add an AutocompleteWithSuggestions component for reusability [WIP] feat: add an AutocompleteWithSuggestions component for reusability May 7, 2021
@dkoo dkoo added wip Do Not Merge! and removed [Status] Needs Design Review [Status] Needs Review The issue or pull request needs to be reviewed labels May 7, 2021
@dkoo
Copy link
Contributor Author

dkoo commented May 7, 2021

Actually, @thomasguillot—would you mind if we tackled the multiselect enhancement of this component in a separate PR (and a second, subsequent NPM package release)? I ask because the single-item component is going to be a dependency of this Campaigns feature in the works, and it'd be great to not block that feature in the meantime.

EDIT: opened an issue for the multiselect: #954

@thomasguillot
Copy link
Contributor

Actually, @thomasguillot—would you mind if we tackled the multiselect enhancement of this component in a separate PR (and a second, subsequent NPM package release)? I ask because the single-item component is going to be a dependency of this Campaigns feature in the works, and it'd be great to not block that feature in the meantime.

Yep, no problem @dkoo

@dkoo dkoo changed the title [WIP] feat: add an AutocompleteWithSuggestions component for reusability feat: add an AutocompleteWithSuggestions component for reusability May 12, 2021
@dkoo dkoo added [Status] Needs Review The issue or pull request needs to be reviewed and removed Do Not Merge! wip labels May 12, 2021
Copy link
Member

@adekbadek adekbadek left a comment

Choose a reason for hiding this comment

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

Found some issues:

  1. After the component loads, but before typing anything into the input, selecting a post results in an empty item rendered:

image

  1. After selecting a post and typing in characters to select another one, user has to first remove the previously-selected post to insert the post from search results.
  2. The UI suggests that multiple items can be selected, while it's not true:

image

you can test within the Newspack Plugin

We do have a components demo page just for that purpose - I've added it there in 881d816

import { AutocompleteTokenField } from '../';
import './style.scss';

class AutocompleteWithSuggestions extends Component {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: would've expected any new components we add to be functional (non-class-based) components, but besides developer experience that's not that important.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored to a functional component in 74b51b9.

@adekbadek adekbadek added [Status] Needs changes or feedback The issue or pull request needs action from the original creator and removed [Status] Needs Review The issue or pull request needs to be reviewed labels May 14, 2021
@dkoo
Copy link
Contributor Author

dkoo commented May 14, 2021

  1. After the component loads, but before typing anything into the input, selecting a post results in an empty item rendered:

Ah, there was some weirdness in the interaction with the AutocompleteTokenfield. In 51bf59f I've extended that component to be able to take an array of tokens that include labels, and if the tokens include labels already, it just displays those instead of trying to fetch the labels.

  1. After selecting a post and typing in characters to select another one, user has to first remove the previously-selected post to insert the post from search results.

Also fixed in 51bf59f. Now, you should be able to type in a search term, select a suggestion, and it will overwrite the existing value.

  1. The UI suggests that multiple items can be selected, while it's not true

That's true, but for all use cases of the single-select version of this component, we immediately unmount the component and render something else as soon as a selected value exists, so it hasn't been an issue so far. I'm open to suggestions on how to display a selected state for this component when in single-select mode (maybe we separate the AutocompleteTokenfield from the selected tokens as in Thomas's multi-select design?).

We do have a components demo page just for that purpose - I've added it there in 881d816

Ah, I forgot about the demo page! Thanks for adding.

@dkoo dkoo requested a review from adekbadek May 14, 2021 17:25
Copy link
Member

@adekbadek adekbadek left a comment

Choose a reason for hiding this comment

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

👌

@github-actions github-actions bot added the [Status] Approved The pull request has been reviewed and is ready to merge label May 17, 2021
Co-authored-by: Adam Boro <adam@adamboro.com>
@dkoo dkoo merged commit 460728d into master May 17, 2021
@dkoo dkoo deleted the feat/autocomplete-with-suggestions-component branch May 17, 2021 16:00
matticbot pushed a commit that referenced this pull request May 18, 2021
# [1.40.0-alpha.1](v1.39.0...v1.40.0-alpha.1) (2021-05-18)

### Bug Fixes

* replace WP User Avatar with Simple Local Avatars ([#966](#966)) ([f980412](f980412))
* **oauth:** wpcom token saving ([24052d6](24052d6))
* **progress-bar:** radius when having headings ([#963](#963)) ([8347362](8347362))
* loading quiet anim time/height and margin when admin menu is folded ([#958](#958)) ([f297780](f297780))

### Features

* add an AutocompleteWithSuggestions component for reusability ([#952](#952)) ([460728d](460728d))
* add new ButtonCard component ([#961](#961)) ([eff9edf](eff9edf))
* add reusable blocks extended as supported plugin ([#968](#968)) ([10f9758](10f9758))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.40.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request May 18, 2021
# [1.40.0](v1.39.0...v1.40.0) (2021-05-18)

### Bug Fixes

* replace WP User Avatar with Simple Local Avatars ([#966](#966)) ([f980412](f980412))
* **oauth:** wpcom token saving ([24052d6](24052d6))
* **progress-bar:** radius when having headings ([#963](#963)) ([8347362](8347362))
* loading quiet anim time/height and margin when admin menu is folded ([#958](#958)) ([f297780](f297780))

### Features

* add an AutocompleteWithSuggestions component for reusability ([#952](#952)) ([460728d](460728d))
* add new ButtonCard component ([#961](#961)) ([eff9edf](eff9edf))
* add reusable blocks extended as supported plugin ([#968](#968)) ([10f9758](10f9758))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.40.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component released on @alpha released [Status] Approved The pull request has been reviewed and is ready to merge [Status] Needs changes or feedback The issue or pull request needs action from the original creator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants