Store: Update `ProductSearch` component for better variation support, multiple selections #19531

ryelle · 2017-11-06T18:11:06Z

See #15681. We need a reusable product search component for orders and promotions, so I've pulled together the code from what's currently in the promotions section and added variation support.

We now have a component that renders out all the products in a store, which can be filtered by searching. Products with variations are listed with a Count component (mostly to take up the space where the checkbox should be), and a Customize button, which opens the variations form.

As products are selected in the form, they become selected rows in the list. Unchecking them deletes them from the list. They're also displayed with the correct variation info - price and image.

This component also supports enforcing a single selection. This uses radio buttons, instead of checkboxes.

It only allows for one variation to be selected at a time, and will replace the currently-selected variation the new one if it's changed.

You can also check out the new README for examples/expected props.

ℹ️ Note: This PR also updates the per_page for the variations API request. It was set to 10, so my Mug example with 12 variations (3 sizes, 4 colors) was not fully loading here or on the product edit screen.

⚠️ Known issue: currently when searching, selected simple products stay in the list even if they don't match (a holdover from the appliesTo component). This isn't true for variations right now, but I wanted to finally get this PR up before it took even longer.

❓ Question: Currently the text says that the user can add the "base product" without picking variations, but I haven't added that functionality yet. Is that something we should support?

To test

This isn't in use anywhere real yet, but you can still test it by checking out the order create screen

View order create page by visiting /store/order/:site
You can find the search under "Add products to the order"
Make sure all your products load in (they come in either 50 or 100 at a time, so if you have a lot of products they might not all be there immediately)
Try selecting some products
Click around the variations, select and unselect them
You can edit order-create/index.js and add the singular prop to see the single-select view
Additional testing of the Products edit screen to make sure all variations load in the variations table would also help 👍

As usual, if anyone has better copy I'm happy to update anything here 🙂

matticbot · 2017-11-06T18:11:13Z

Test live: https://calypso.live/?branch=add/store-new-product-search

coderkevin · 2017-11-06T18:36:52Z

❓ Question: Currently the text says that the user can add the "base product" without picking variations, but I haven't added that functionality yet. Is that something we should support?

We'll need this for coupon-based promotions, as it's possible to select entire variable products or individual variations of that product for a coupon. However, for product sales, it's not possible to select entire variable products. So maybe make it optional?

coderkevin

Thanks for all the code on this, @ryelle! I've got just a couple of comments.

coderkevin · 2017-11-06T18:43:13Z

client/extensions/woocommerce/components/product-search/row.js

+		}
+
+		const { oldSiteId, oldProductId } = this.props;
+		const { newSiteId, newProductId } = newProps;


You're naming props here as oldSiteId, oldProductId, newSiteId, and newProductId? Wouldn't the incoming props be just siteId and productId?

Fixed this, thanks for catching it

coderkevin · 2017-11-06T19:05:51Z

client/extensions/woocommerce/components/product-search/row.js

+		if ( matchingVariations.length === 1 ) {
+			// We found a match.
+			const variation = head( matchingVariations );
+			if ( this.props.singular ) {


couldn't this be simplified just by creating

const variations = ( this.props.singular ? [ head( matchingVariations ) ] : matchingVariations );

or something like that? And then removing the singular-specific code?

coderkevin · 2017-11-06T19:07:38Z

client/extensions/woocommerce/components/product-search/row.js

+	areAnySelected = () => {
+		const { product, singular } = this.props;
+		const { variations } = this.state;
+		if ( singular && variations[ 0 ] ) {


Same here, maybe

const variations = ( singular ? [ this.state.variations[ 0 ] ] : this.state.variations );

No to both, really, because of the line after both of these; behavior following is different for singular vs multiple selections. If you want to take a stab at updating this so that it's not the case once it's merged, feel free :)

coderkevin · 2017-11-06T19:18:21Z

client/extensions/woocommerce/components/product-search/row.js

+				{ this.renderInputImage( product ) }
+				{ this.renderInputName( product ) }
+			</FormLabel>
+		);


I like how clean this all stacks up!

coderkevin · 2017-11-06T22:26:48Z

client/extensions/woocommerce/components/product-search/utils.js

@@ -0,0 +1,48 @@
+/** @format */
+
+/**


Maybe some unit tests for these functions in a future PR?

allendav · 2017-11-07T16:50:32Z

Possible future PR - if you don't have any products, maybe a disabled placeholder that says as much instead of the search box?

allendav · 2017-11-07T17:07:28Z

client/extensions/woocommerce/components/product-search/index.js

-				/>
-				<ProductSearchResults search={ this.state.query } onSelect={ this.handleSelect } />
+				{ this.renderSearch( this.state.searchFilter ) }
+				{ this.renderList( this.props.singular ) }


Why not just destructure singular inside renderList instead of passing it? Same question for renderSearch too.

allendav · 2017-11-07T17:08:42Z

client/extensions/woocommerce/components/product-search/row.js

+	reduce,
+	uniqBy,
+	values,
+} from 'lodash';


10 lodash imports - you get your next one free :P

allendav

Tests very well. Just one comment on the code and one suggestion regarding zero-product stores. Pre-approving.

coderkevin

LGTM

ryelle · 2017-11-08T15:38:54Z

@jameskoster Feedback on the search component itself can go here :) There's also more info about the component itself in the description here.

It's a shame there's no way to add more than one variation at once, but I can't really see a solution for that.

If you're talking about the bug in your gif, that's definitely a bug. You can see in the screenshots above that you can select more than one variation. Does this only happen on products with two variations?

"Customize" doesn't feel like the right word for the variation button. How about "Select variations"?

Sure - like usual I basically threw words in hoping for feedback 🙂

One other quirk. If I check a product, then search for another product for which there's only one result and check that, it looks like only one product will be added. Perhaps checked products should be permanently visible?

That shouldn't be the case, but i'll test again. I did note in the description here that only selected simple products are shown when searching, and that I know that's wonky from a UX perspective but that I'll fix it in a follow up PR (I wanted this one and the add products to get merged first, so we would have the base to work off of).

a8ci18n · 2017-11-08T15:50:46Z

client/extensions/woocommerce/components/product-search/row.js

+				<span>
+					<span>{ nameWithPrice }</span>
+					<Button compact onClick={ this.toggleCustomizeForm }>
+						{ translate( 'Select variations' ) }


Hi! I've found a possible matching string that has already been translated 17 times:
translate( 'Multiple variations' ) ES Score: 8

Help me improve these suggestions: react with 👎 if the suggestion doesn't make any sense, or with 👍 if it's a particularly good one (even if not implemented).

jameskoster · 2017-11-08T17:48:58Z

Does this only happen on products with two variations?

OK this is odd, I just tried a product with 2 attributes + 6 variations, but when I try to add it I can only choose one attribute and the component say's there are only 2 variations. # #

I did note in the description here that only selected simple products are shown when searching

Oh the searching works fine, I was just saying it's an odd experience to have checked a product, then search and see one result, check that then click add and see 2 products added. It just feels odd cause when you click the button you can only see a single product. Like I said, permanently displaying checked products would fix this.

ryelle · 2017-11-08T17:52:59Z

Managed to reproduce the issue @jameskoster mentioned, it looks like if you select an already-selected variation it wipes out the rest of the selected variations? Not sure why, but I cleaned up some of the variation state logic and it's working much better now.

I just tried a product with 2 attributes + 6 variations, but when I try to add it I can only choose one attribute and the component say's there are only 2 variations.

hmm that's unusual. was this product created in wp-admin, or calypso?

the component say's there are only 2 variations.

The 2 is part of your product's name - the notice-label doesn't say anything about the count of variations (though it could, if you think that's useful).

…ToFilteredList`

…h also renders a variation form when a variable product is selected

…variable names

Adds a class to the variations wrapper Puts the variations notice inside a `Notice`

…text

…st of the selected variations

a8ci18n · 2017-11-08T18:39:15Z

client/extensions/woocommerce/components/product-search/variations.js

+					value={ this.state[ attribute.name ] }
+				>
+					<option key={ DEFAULT_ATTR } value={ DEFAULT_ATTR }>
+						{ translate( 'Select one' ) }


Hi! I've found a possible matching string that has already been translated 15 times:
translate( 'Select one...' ) ES Score: 11

Help me improve these suggestions: react with 👎 if the suggestion doesn't make any sense, or with 👍 if it's a particularly good one (even if not implemented).

Hmm, who forgot to use the … character?

coderkevin · 2017-11-08T18:54:21Z

client/extensions/woocommerce/components/product-search/row.js

+
+		const selectedIds = intersection( newProps.value, newProps.product.variations );
+		const selectedVariations = selectedIds.map( id => find( newProps.variations, { id } ) );
+		this.state = {


This should use setState instead. It's causing a console warning.

hah, bet that's new with the React upgrade, hadn't seen that the rest of the time I was working on this

coderkevin · 2017-11-08T19:03:08Z

client/extensions/woocommerce/components/product-search/variations.js

+					<option key={ DEFAULT_ATTR } value={ DEFAULT_ATTR }>
+						{ translate( 'Select one' ) }
+					</option>
+					{ attribute.options.map( ( opt, i ) => <option key={ i }>{ opt }</option> ) }


I think that i as a key isn't specific enough when you have more than one attribute. I'm getting a console warning on the same key 0 when I have two attributes on a variable product.

Strange, I'm not. But I'll update it regardless, seems like a good idea.

There are a couple of issues still.

coderkevin · 2017-11-08T19:07:09Z

On a variable product with two attributes, the first selector never displays the selected attribute value, although it still appears to be working. Maybe it's something to do with the React same key, 0 warning I mentioned in the code?

…on keys

ryelle · 2017-11-08T19:29:42Z

Maybe it's something to do with the React same key, 0 warning I mentioned in the code?

No, it was introduced with the change in d488f4b & had to do with setting this.state without setState. All the points you raised should be fixed now.

a8ci18n · 2017-11-08T19:42:42Z

client/extensions/woocommerce/components/product-search/variations.js

+					value={ this.state[ attribute.name ] }
+				>
+					<option key={ `${ attribute.name }-${ DEFAULT_ATTR }` } value={ DEFAULT_ATTR }>
+						{ translate( 'Select one' ) }


Hi! I've found a possible matching string that has already been translated 15 times:
translate( 'Select one...' ) ES Score: 11

Help me improve these suggestions: react with 👎 if the suggestion doesn't make any sense, or with 👍 if it's a particularly good one (even if not implemented).

ryelle · 2017-11-08T22:16:55Z

@coderkevin This PR is getting pretty long, are you OK with me merging it? The things still on my to-address list:

A placeholder view for a store with no products
Always show all selected products (including variations)
"Select variations" button should hide the form when clicked again
Whatever the issue with Jay's product is, though I can't reproduce it
Indent variations below products

coderkevin

Latest changes LGTM and test well. 🎉 Great job!

kellychoffman · 2017-11-09T14:05:53Z

One design request: Re: https://user-images.githubusercontent.com/541093/32455495-77ad6bdc-c2f0-11e7-8e92-a1d645d2106a.png

Could the variations listed below the "main product" be indented so it looks like they are part of it?

ryelle · 2017-11-09T15:33:53Z

@kellychoffman I've updated my "to-address list" with the design feedback, so I'll address that post-merge.

… multiple selections (Automattic#19531) * Remove existing `ProductSearch` component & style, copy over `AppliesToFilteredList` * Split each product row into a new component, `ProductSearchRow`, which also renders a variation form when a variable product is selected * Pull in product variations * Style the variations form * Clean up component, bring in the price display * Update row display when a variation is selected * Increase number of variations we can load * Clean up component * Move general functions into a helper utils file * Add support for passing arrays to add/remove product IDs * Pull row-rendering into a separate function, so we can use the same thing for products as variations. Move isSelected into this component so that we can check if variations are selected * Handle selecting variations when the parent is unselected, delete unselected options * Split up row-rendering function into smaller functions * Add button to “Customize” variable products * Retain selected variations even if the components are hidden by search * Add attribute support into search by collapsing all attributes into a searchable string * Fix singular display/interactions * Fix “Customize” button behavior * Update README * Fix props destructuring, use the correct name but map to the new/old variable names * Style the variation selector Adds a class to the variations wrapper Puts the variations notice inside a `Notice` * Bring back FormLabel for accessibility, update variation form button text * Address PR feedback * Fix destructuring of new/old props * Fix issue where selecting an already-selected variation clears the rest of the selected variations * Update label text & the default select text. * Reset the form back to initial state after selecting a variation * Address PR feedback: don’t assign to this.state directly, update option keys

ryelle added Store [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Nov 6, 2017

ryelle self-assigned this Nov 6, 2017

ryelle requested review from timmyc, justinshreve, jameskoster, coderkevin, allendav and kellychoffman November 6, 2017 18:11

This was referenced Nov 6, 2017

Store ProductSearch Component: Rewrite to allow for multiple selections #19191

Closed

Store Orders: Edit an existing order #15681

Closed

coderkevin reviewed Nov 6, 2017

View reviewed changes

allendav reviewed Nov 7, 2017

View reviewed changes

allendav approved these changes Nov 7, 2017

View reviewed changes

ryelle mentioned this pull request Nov 7, 2017

Store Orders: Add products to existing orders #19243

Merged

coderkevin previously approved these changes Nov 7, 2017

View reviewed changes

ryelle force-pushed the add/store-new-product-search branch from c47a28f to 8e3db38 Compare November 8, 2017 15:42

a8ci18n reviewed Nov 8, 2017

View reviewed changes

ryelle added 3 commits November 8, 2017 13:28

Remove existing ProductSearch component & style, copy over `Applies…

766b65a

…ToFilteredList`

Split each product row into a new component, ProductSearchRow, whic…

e02af8f

…h also renders a variation form when a variable product is selected

Pull in product variations

4c79b30

ryelle and others added 9 commits November 8, 2017 13:28

Update README

94a3b97

Fix props destructuring, use the correct name but map to the new/old …

3a4188f

…variable names

Style the variation selector

c05afa8

Adds a class to the variations wrapper Puts the variations notice inside a `Notice`

Bring back FormLabel for accessibility, update variation form button …

6e14d29

…text

Address PR feedback

5528e92

Fix destructuring of new/old props

24a6c25

Fix issue where selecting an already-selected variation clears the re…

69a0cea

…st of the selected variations

Update label text & the default select text.

a3370f8

Reset the form back to initial state after selecting a variation

d488f4b

ryelle force-pushed the add/store-new-product-search branch from f48c55d to d488f4b Compare November 8, 2017 18:29

a8ci18n reviewed Nov 8, 2017

View reviewed changes

coderkevin reviewed Nov 8, 2017

View reviewed changes

Address PR feedback: don’t assign to this.state directly, update opti…

fd909c2

…on keys

a8ci18n reviewed Nov 8, 2017

View reviewed changes

coderkevin approved these changes Nov 9, 2017

View reviewed changes

ryelle merged commit 805d743 into master Nov 9, 2017

matticbot removed the [Status] Ready to Merge label Nov 9, 2017

ryelle deleted the add/store-new-product-search branch November 13, 2017 15:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Store: Update `ProductSearch` component for better variation support, multiple selections #19531

Store: Update `ProductSearch` component for better variation support, multiple selections #19531

ryelle commented Nov 6, 2017

matticbot commented Nov 6, 2017

coderkevin commented Nov 6, 2017 •

edited

Loading

coderkevin left a comment

coderkevin Nov 6, 2017 •

edited

Loading

ryelle Nov 7, 2017

coderkevin Nov 6, 2017 •

edited

Loading

coderkevin Nov 6, 2017

ryelle Nov 6, 2017

coderkevin Nov 6, 2017

coderkevin Nov 6, 2017

allendav commented Nov 7, 2017

allendav Nov 7, 2017

allendav Nov 7, 2017

allendav left a comment •

edited

Loading

coderkevin left a comment

ryelle commented Nov 8, 2017

a8ci18n Nov 8, 2017

jameskoster commented Nov 8, 2017

ryelle commented Nov 8, 2017 •

edited

Loading

a8ci18n Nov 8, 2017

coderkevin Nov 8, 2017

coderkevin Nov 8, 2017

ryelle Nov 8, 2017

coderkevin Nov 8, 2017

ryelle Nov 8, 2017

coderkevin commented Nov 8, 2017

ryelle commented Nov 8, 2017

a8ci18n Nov 8, 2017

ryelle commented Nov 8, 2017 •

edited

Loading

coderkevin left a comment •

edited

Loading

kellychoffman commented Nov 9, 2017

ryelle commented Nov 9, 2017

Store: Update ProductSearch component for better variation support, multiple selections #19531

Store: Update ProductSearch component for better variation support, multiple selections #19531

Conversation

ryelle commented Nov 6, 2017

matticbot commented Nov 6, 2017

coderkevin commented Nov 6, 2017 • edited Loading

coderkevin left a comment

Choose a reason for hiding this comment

coderkevin Nov 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coderkevin Nov 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

allendav commented Nov 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

allendav left a comment • edited Loading

Choose a reason for hiding this comment

coderkevin left a comment

Choose a reason for hiding this comment

ryelle commented Nov 8, 2017

Choose a reason for hiding this comment

jameskoster commented Nov 8, 2017

ryelle commented Nov 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coderkevin commented Nov 8, 2017

ryelle commented Nov 8, 2017

Choose a reason for hiding this comment

ryelle commented Nov 8, 2017 • edited Loading

coderkevin left a comment • edited Loading

Choose a reason for hiding this comment

kellychoffman commented Nov 9, 2017

ryelle commented Nov 9, 2017

Store: Update `ProductSearch` component for better variation support, multiple selections #19531

Store: Update `ProductSearch` component for better variation support, multiple selections #19531

coderkevin commented Nov 6, 2017 •

edited

Loading

coderkevin Nov 6, 2017 •

edited

Loading

coderkevin Nov 6, 2017 •

edited

Loading

allendav left a comment •

edited

Loading

ryelle commented Nov 8, 2017 •

edited

Loading

ryelle commented Nov 8, 2017 •

edited

Loading

coderkevin left a comment •

edited

Loading