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 multi-select capabillity to AutocompleteWithSuggestions #975

Merged
merged 15 commits into from
May 28, 2021

Conversation

dkoo
Copy link
Contributor

@dkoo dkoo commented May 21, 2021

All Submissions:

Changes proposed in this Pull Request:

This complete the AutocompleteWithSuggestions component to add multi-select capability. This is needed for the proposed child listings UI design in Newspack Listings.

Closes #954.

How to test the changes in this Pull Request:

  1. Navigate to the components demo page. Confirm there are now two instances of the AutocompleteWithSuggestions component: the first is single-select (unchanged from master), the second is multi-select with checkboxes instead of links.
  2. Test the single-select UI; its behavior should be unchanged from master, except that the selected item is now shown at the top of the component separate from the autocomplete field, which is now only used for search functionality:

Screen Shot 2021-05-21 at 11 52 54 AM

  1. Test the multi-select UI:
  • Post type selector: This dropdown lets you change which post type is shown in suggestions, both in the autocomplete field and in the suggestions list. (This can actually be used in both single-select and multi-select flavors of the component, but is only shown in multi-select to demonstrate that it's optional.)
  • Multi-select capability: You should now be able to select more than one item. Selected items appear at the top of the component, and are checked if displayed in the suggestions list. Checking an item from the list should add it to the selections, unchecking it should remove it. Typing a post title into the autocomplete field and selecting a suggestion should add the suggestion if it's not already selected, and remove it if it is.
  • "Clear all" feature: If more than one item is selected, a "Clear all" button will appear at the top of the component. Clicking this will remove all selections and return the component to an empty state.

Screen Shot 2021-05-21 at 12 00 08 PM

  1. Test load more functionality: For both single-select and multi-select flavors, if there are more posts to suggest than the suggestionsToFetch prop (default: 10), a "Load more" button will appear at the end of the suggestions list. Clicking this button will load suggestionsToFetch more posts until all posts of the selected post type are loaded.

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 [Type] Feature request Request for new features [Status] Needs Review The issue or pull request needs to be reviewed [Status] Needs Design Review labels May 21, 2021
@dkoo dkoo self-assigned this May 21, 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.

Nice! I like how the default props make it easier to use the component.

@dkoo
Copy link
Contributor Author

dkoo commented May 25, 2021

@thomasguillot One thing I should mention is that I deviated slightly from your design mockup with the selected items here:

Screen Shot 2021-05-25 at 10 44 55 AM

In your mockup:

Screen Shot 2021-05-25 at 10 43 12 AM

In the implemented version, we group all selections together regardless of their post type. I found that grouping different post types into separate sections was causing the selection area to be very tall if more than one post type is selected, and also caused a lot of screen jitter when post type sections get added or removed.

@dkoo
Copy link
Contributor Author

dkoo commented May 26, 2021

@adekbadek I've been testing this component with a Newspack Listings feature and doing so has uncovered a few bugs/shortcomings in the component as you reviewed it. Here's a summary in case you want to re-test and/or re-review:

  1. 6da0a28 - This fixes a bug where if you pass a fetchSuggestions function as a prop to the component, it didn't receive info about the currently selected post type. This is important when passing the fetch function as a prop because the selected post type determines what the suggestions endpoint needs to be.
  2. 8bda922 - This fixes a similar set of bugs when using the autocomplete field with a passed fetchSuggestions prop. The AutocompleteTokenfield component was not passing the extra param for selected post type, nor appending the extra postType property on selected items onChange (needed so that we can add and remove selection data using the correct post types via REST API).

To test (1), you can pass in a fetchSuggestions prop like this to the component:

fetchSuggestions={ ( search = null, offset = 0, postType ) => console.log( search, offset, postType ) }

Make sure the logged params match the component's state (tokenfield search term, postType matching selection in the dropdown). Offset will always be 0 unless you actually return posts data in the function which will let you use load more functionality in the suggestions list. Make sure this is true for both the first execution (which would be to fetch the list of suggestions) and any executions when entering search terms into the autocomplete field.

To test (2), pass in an onChange prop like so:

onChange={ items => console.log( items ) }

Select items by clicking them from the suggestion list and selecting them from the autocomplete search results. Confirm that each item in the array contains both the item's id and its correct postType. Confirm the same after removing some previously selected items.

@dkoo dkoo requested a review from adekbadek May 26, 2021 16:21
@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels May 28, 2021
@thomasguillot thomasguillot added [Status] Needs Review The issue or pull request needs to be reviewed and removed [Status] Approved The pull request has been reviewed and is ready to merge labels May 28, 2021
@dkoo
Copy link
Contributor Author

dkoo commented May 28, 2021

I've been using the component from this branch in Campaigns, Listings, and Blocks for several days now without issue, so I'm going to go ahead and merge/publish to NPM. If any further bugs or features need to be addressed, we can do that in additional and smaller PRs. Thanks to both of you for your reviews, @thomasguillot and @adekbadek!

@dkoo dkoo merged commit d7aebe2 into master May 28, 2021
@dkoo dkoo deleted the feat/multi-select-autocomplete branch May 28, 2021 15:48
matticbot pushed a commit that referenced this pull request Jun 1, 2021
# [1.42.0-alpha.1](v1.41.0...v1.42.0-alpha.1) (2021-06-01)

### Features

* add multi-select capabillity to AutocompleteWithSuggestions ([#975](#975)) ([d7aebe2](d7aebe2))
@matticbot
Copy link
Contributor

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

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Jun 2, 2021
# [1.42.0](v1.41.0...v1.42.0) (2021-06-02)

### Features

* add multi-select capabillity to AutocompleteWithSuggestions ([#975](#975)) ([d7aebe2](d7aebe2))
* use WPCOM as a proxy for Google OAuth2 flow ([#962](#962)) ([b95fcc0](b95fcc0))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.42.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
released on @alpha released [Status] Needs Review The issue or pull request needs to be reviewed [Type] Feature request Request for new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Newspack Components: Extend AutocompleteWithSuggestions to handle multi-select
4 participants