Skip to content

Commit

Permalink
Don't filter product options with only one value in the VariantSelect…
Browse files Browse the repository at this point in the history
…or (#2113)
  • Loading branch information
blittle authored Jun 12, 2024
1 parent e96b332 commit 8b9c726
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 104 deletions.
19 changes: 19 additions & 0 deletions .changeset/cold-foxes-draw.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
'skeleton': patch
'@shopify/hydrogen': patch
---

**Breaking change**

Previously the `VariantSelector` component would filter out options that only had one value. This is undesireable for some apps. We've removed that filter, if you'd like to retain the existing functionality, simply filter the options prop before it is passed to the `VariantSelector` component:

```diff
<VariantSelector
handle={product.handle}
+ options={product.options.filter((option) => option.values.length > 1)}
- options={product.options}
variants={variants}>
</VariantSelector>
```

Fixes [#1198](https://github.com/Shopify/hydrogen/discussions/1198)
3 changes: 2 additions & 1 deletion examples/b2b/app/routes/products.$handle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,8 @@ function ProductForm({
<div className="product-form">
<VariantSelector
handle={product.handle}
options={product.options}
// Only show options with more than one value
options={product.options.filter((option) => option.values.length > 1)}
variants={variants}
>
{({option}) => <ProductOptions key={option.name} option={option} />}
Expand Down
51 changes: 14 additions & 37 deletions packages/hydrogen/src/product/VariantSelector.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,43 +198,6 @@ describe('<VariantSelector>', () => {
`);
});

it('automatically appends options with only one value to the URL', () => {
const {asFragment} = render(
createElement(VariantSelector, {
handle: 'snowboard',
options: [
{name: 'Color', values: ['Red']},
{name: 'Size', values: ['S', 'M']},
],
children: ({option}) =>
createElement(
'div',
null,
option.values.map(({value, to}) =>
createElement('a', {key: option.name + value, href: to}, value),
),
),
}),
);

expect(asFragment()).toMatchInlineSnapshot(`
<DocumentFragment>
<div>
<a
href="/products/snowboard?Size=S&Color=Red"
>
S
</a>
<a
href="/products/snowboard?Size=M&Color=Red"
>
M
</a>
</div>
</DocumentFragment>
`);
});

it('prepends localization', () => {
vi.mocked(useLocation).mockReturnValueOnce(
fillLocation({search: '?Size=M', pathname: '/en-us/'}),
Expand Down Expand Up @@ -268,6 +231,13 @@ describe('<VariantSelector>', () => {

expect(asFragment()).toMatchInlineSnapshot(`
<DocumentFragment>
<div>
<a
href="/en-us/products/snowboard?Color=Red"
>
Red
</a>
</div>
<div>
<a
href="/en-us/products/snowboard?Size=S&Color=Red"
Expand Down Expand Up @@ -318,6 +288,13 @@ describe('<VariantSelector>', () => {

expect(asFragment()).toMatchInlineSnapshot(`
<DocumentFragment>
<div>
<a
href="/products/snowboard?Color=Red"
>
Red
</a>
</div>
<div>
<a
href="/products/snowboard?Size=S&Color=Red"
Expand Down
125 changes: 60 additions & 65 deletions packages/hydrogen/src/product/VariantSelector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,71 +64,66 @@ export function VariantSelector({
Fragment,
null,
...useMemo(() => {
return (
options
// Only show options with more than one value
.filter((option) => option?.values?.length! > 1)
.map((option) => {
let activeValue;
let availableValues: VariantOptionValue[] = [];

for (let value of option.values!) {
// The clone the search params for each value, so we can calculate
// a new URL for each option value pair
const clonedSearchParams = new URLSearchParams(
alreadyOnProductPage ? searchParams : undefined,
);
clonedSearchParams.set(option.name!, value!);

// Because we hide options with only one value, they aren't selectable,
// but they still need to get into the URL
optionsWithOnlyOneValue.forEach((option) => {
clonedSearchParams.set(option.name!, option.values![0]!);
});

// Find a variant that matches all selected options.
const variant = variants.find((variant) =>
variant?.selectedOptions?.every(
(selectedOption) =>
clonedSearchParams.get(selectedOption?.name!) ===
selectedOption?.value,
),
);

const currentParam = searchParams.get(option.name!);

const calculatedActiveValue = currentParam
? // If a URL parameter exists for the current option, check if it equals the current value
currentParam === value!
: false;

if (calculatedActiveValue) {
// Save out the current value if it's active. This should only ever happen once.
// Should we throw if it happens a second time?
activeValue = value;
}

const searchString = '?' + clonedSearchParams.toString();

availableValues.push({
value: value!,
isAvailable: variant ? variant.availableForSale! : true,
to: path + searchString,
search: searchString,
isActive: calculatedActiveValue,
variant,
});
}

return children({
option: {
name: option.name!,
value: activeValue,
values: availableValues,
},
});
})
);
return options.map((option) => {
let activeValue;
let availableValues: VariantOptionValue[] = [];

for (let value of option.values!) {
// The clone the search params for each value, so we can calculate
// a new URL for each option value pair
const clonedSearchParams = new URLSearchParams(
alreadyOnProductPage ? searchParams : undefined,
);
clonedSearchParams.set(option.name!, value!);

// Because we hide options with only one value, they aren't selectable,
// but they still need to get into the URL
optionsWithOnlyOneValue.forEach((option) => {
clonedSearchParams.set(option.name!, option.values![0]!);
});

// Find a variant that matches all selected options.
const variant = variants.find((variant) =>
variant?.selectedOptions?.every(
(selectedOption) =>
clonedSearchParams.get(selectedOption?.name!) ===
selectedOption?.value,
),
);

const currentParam = searchParams.get(option.name!);

const calculatedActiveValue = currentParam
? // If a URL parameter exists for the current option, check if it equals the current value
currentParam === value!
: false;

if (calculatedActiveValue) {
// Save out the current value if it's active. This should only ever happen once.
// Should we throw if it happens a second time?
activeValue = value;
}

const searchString = '?' + clonedSearchParams.toString();

availableValues.push({
value: value!,
isAvailable: variant ? variant.availableForSale! : true,
to: path + searchString,
search: searchString,
isActive: calculatedActiveValue,
variant,
});
}

return children({
option: {
name: option.name!,
value: activeValue,
values: availableValues,
},
});
});
}, [options, variants, children]),
);
}
Expand Down
2 changes: 1 addition & 1 deletion templates/skeleton/app/routes/products.$handle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ function ProductForm({
<div className="product-form">
<VariantSelector
handle={product.handle}
options={product.options}
options={product.options.filter((option) => option.values.length > 1)}
variants={variants}
>
{({option}) => <ProductOptions key={option.name} option={option} />}
Expand Down

0 comments on commit 8b9c726

Please sign in to comment.