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

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

Merged
merged 28 commits into from
Nov 9, 2017
Merged
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
766b65a
Remove existing `ProductSearch` component & style, copy over `Applies…
ryelle Nov 2, 2017
e02af8f
Split each product row into a new component, `ProductSearchRow`, whic…
ryelle Nov 2, 2017
4c79b30
Pull in product variations
ryelle Nov 3, 2017
d123a70
Style the variations form
ryelle Nov 3, 2017
7421d53
Clean up component, bring in the price display
ryelle Nov 3, 2017
7a8b10a
Update row display when a variation is selected
ryelle Nov 3, 2017
ead4472
Increase number of variations we can load
ryelle Nov 3, 2017
dd7df28
Move general functions into a helper utils file
ryelle Nov 3, 2017
1e04682
Clean up component
ryelle Nov 3, 2017
8f1f4b8
Add support for passing arrays to add/remove product IDs
ryelle Nov 4, 2017
988d6ac
Pull row-rendering into a separate function, so we can use the same t…
ryelle Nov 4, 2017
1dafc04
Handle selecting variations when the parent is unselected, delete uns…
ryelle Nov 4, 2017
c87bba0
Split up row-rendering function into smaller functions
ryelle Nov 6, 2017
74ee098
Add button to “Customize” variable products
ryelle Nov 6, 2017
3ecada0
Retain selected variations even if the components are hidden by search
ryelle Nov 6, 2017
aa5733b
Add attribute support into search by collapsing all attributes into a…
ryelle Nov 6, 2017
ecebc67
Fix singular display/interactions
ryelle Nov 6, 2017
ae3e2d9
Fix “Customize” button behavior
ryelle Nov 6, 2017
94a3b97
Update README
ryelle Nov 6, 2017
3a4188f
Fix props destructuring, use the correct name but map to the new/old …
ryelle Nov 7, 2017
c05afa8
Style the variation selector
jameskoster Nov 8, 2017
6e14d29
Bring back FormLabel for accessibility, update variation form button …
ryelle Nov 8, 2017
5528e92
Address PR feedback
ryelle Nov 8, 2017
24a6c25
Fix destructuring of new/old props
ryelle Nov 8, 2017
69a0cea
Fix issue where selecting an already-selected variation clears the re…
ryelle Nov 8, 2017
a3370f8
Update label text & the default select text.
ryelle Nov 8, 2017
d488f4b
Reset the form back to initial state after selecting a variation
ryelle Nov 8, 2017
fd909c2
Address PR feedback: don’t assign to this.state directly, update opti…
ryelle Nov 8, 2017
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
Prev Previous commit
Next Next commit
Update label text & the default select text.
  • Loading branch information
ryelle committed Nov 8, 2017
commit a3370f89c305e1c927395a98b01a3ba693cab166
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ import FormLegend from 'components/forms/form-legend';
import FormSelect from 'components/forms/form-select';
import Notice from 'components/notice';

// Use a constant for the default attribute state.
const DEFAULT_ATTR = 'any';

class ProductVariations extends Component {
static propTypes = {
onChange: PropTypes.func,
Expand All @@ -27,7 +30,7 @@ class ProductVariations extends Component {
this.state = {};
const attributes = filter( props.product.attributes, { variation: true } );
forEach( attributes, attr => {
this.state[ attr.name ] = 'any'; // Values default to any.
this.state[ attr.name ] = DEFAULT_ATTR;
} );
}

Expand All @@ -47,8 +50,8 @@ class ProductVariations extends Component {
id={ `select-${ kebabCase( attribute.name ) }` }
onChange={ this.onChange( attribute.name ) }
>
<option key={ 'any' } value="any">
{ translate( 'Any' ) }
<option key={ DEFAULT_ATTR } value={ DEFAULT_ATTR }>
{ translate( 'Select one' ) }
Copy link

Choose a reason for hiding this comment

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

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).

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, who forgot to use the … character?

Copy link

Choose a reason for hiding this comment

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

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).

</option>
{ attribute.options.map( ( opt, i ) => <option key={ i }>{ opt }</option> ) }
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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

</FormSelect>
Expand All @@ -71,7 +74,7 @@ class ProductVariations extends Component {
<FormLegend>
<Notice showDismiss={ false }>
{ translate(
'%(product)s has variations. Select a specific customization, or add the base product.',
'%(product)s has variations. Choose a specific customization to select.',
{ args: { product: product.name } }
) }
</Notice>
Expand Down