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

Make variant / image updates use useOptimistic and startTransition #1327

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
71 changes: 39 additions & 32 deletions components/product/gallery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,62 +4,69 @@ import { ArrowLeftIcon, ArrowRightIcon } from '@heroicons/react/24/outline';
import { GridTileImage } from 'components/grid/tile';
import { createUrl } from 'lib/utils';
import Image from 'next/image';
import Link from 'next/link';
import { usePathname, useSearchParams } from 'next/navigation';
import { usePathname, useRouter, useSearchParams } from 'next/navigation';
import { useOptimistic, useTransition } from 'react';

export function Gallery({ images }: { images: { src: string; altText: string }[] }) {
const router = useRouter();
const pathname = usePathname();
const searchParams = useSearchParams();
const imageSearchParam = searchParams.get('image');
const imageIndex = imageSearchParam ? parseInt(imageSearchParam) : 0;

const nextSearchParams = new URLSearchParams(searchParams.toString());
const nextImageIndex = imageIndex + 1 < images.length ? imageIndex + 1 : 0;
nextSearchParams.set('image', nextImageIndex.toString());
const nextUrl = createUrl(pathname, nextSearchParams);

const previousSearchParams = new URLSearchParams(searchParams.toString());
const previousImageIndex = imageIndex === 0 ? images.length - 1 : imageIndex - 1;
previousSearchParams.set('image', previousImageIndex.toString());
const previousUrl = createUrl(pathname, previousSearchParams);
const [optimisticIndex, setOptimisticIndex] = useOptimistic(imageIndex);
// eslint-disable-next-line no-unused-vars
const [pending, startTransition] = useTransition();

const buttonClassName =
'h-full px-6 transition-all ease-in-out hover:scale-110 hover:text-black dark:hover:text-white flex items-center justify-center';

function updateIndex(newIndex: number) {
setOptimisticIndex(newIndex);
const newSearchParams = new URLSearchParams(searchParams.toString());
newSearchParams.set('image', newIndex.toString());
router.replace(createUrl(pathname, newSearchParams), { scroll: false });
}

return (
<>
<div className="relative aspect-square h-full max-h-[550px] w-full overflow-hidden">
{images[imageIndex] && (
{images[optimisticIndex] && (
<Image
className="h-full w-full object-contain"
fill
sizes="(min-width: 1024px) 66vw, 100vw"
alt={images[imageIndex]?.altText as string}
src={images[imageIndex]?.src as string}
alt={images[optimisticIndex]?.altText as string}
src={images[optimisticIndex]?.src as string}
priority={true}
/>
)}

{images.length > 1 ? (
<div className="absolute bottom-[15%] flex w-full justify-center">
<div className="mx-auto flex h-11 items-center rounded-full border border-white bg-neutral-50/80 text-neutral-500 backdrop-blur dark:border-black dark:bg-neutral-900/80">
<Link
<button
aria-label="Previous product image"
href={previousUrl}
onClick={() => {
startTransition(() => {
updateIndex(optimisticIndex - 1);
});
}}
className={buttonClassName}
scroll={false}
>
<ArrowLeftIcon className="h-5" />
</Link>
</button>
<div className="mx-1 h-6 w-px bg-neutral-500"></div>
<Link
<button
aria-label="Next product image"
href={nextUrl}
onClick={() => {
startTransition(() => {
updateIndex(optimisticIndex + 1);
});
}}
className={buttonClassName}
scroll={false}
>
<ArrowRightIcon className="h-5" />
</Link>
</button>
</div>
</div>
) : null}
Expand All @@ -68,18 +75,18 @@ export function Gallery({ images }: { images: { src: string; altText: string }[]
{images.length > 1 ? (
<ul className="my-12 flex items-center justify-center gap-2 overflow-auto py-1 lg:mb-0">
{images.map((image, index) => {
const isActive = index === imageIndex;
const imageSearchParams = new URLSearchParams(searchParams.toString());

imageSearchParams.set('image', index.toString());
const isActive = index === optimisticIndex;

return (
<li key={image.src} className="h-20 w-20">
<Link
aria-label="Enlarge product image"
href={createUrl(pathname, imageSearchParams)}
scroll={false}
<button
aria-label="Select product image"
className="h-full w-full"
onClick={() => {
startTransition(() => {
updateIndex(index);
});
}}
>
<GridTileImage
alt={image.altText}
Expand All @@ -88,7 +95,7 @@ export function Gallery({ images }: { images: { src: string; altText: string }[]
height={80}
active={isActive}
/>
</Link>
</button>
</li>
);
})}
Expand Down
45 changes: 33 additions & 12 deletions components/product/variant-selector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import clsx from 'clsx';
import { ProductOption, ProductVariant } from 'lib/shopify/types';
import { createUrl } from 'lib/utils';
import { usePathname, useRouter, useSearchParams } from 'next/navigation';
import { useOptimistic, useTransition } from 'react';

type Combination = {
id: string;
Expand All @@ -21,14 +22,21 @@ export function VariantSelector({
const router = useRouter();
const pathname = usePathname();
const searchParams = useSearchParams();
const [optimisticVariants, setOptimsticVariants] = useOptimistic(variants);
Copy link

Choose a reason for hiding this comment

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

Nit

Suggested change
const [optimisticVariants, setOptimsticVariants] = useOptimistic(variants);
const [optimisticVariants, setOptimisticVariants] = useOptimistic(variants);

const [optimisticOptions, setOptimisticOptions] = useOptimistic(
new URLSearchParams(searchParams.toString())
);
// eslint-disable-next-line no-unused-vars
const [pending, startTransition] = useTransition();

const hasNoOptionsOrJustOneOption =
!options.length || (options.length === 1 && options[0]?.values.length === 1);

if (hasNoOptionsOrJustOneOption) {
return null;
}

const combinations: Combination[] = variants.map((variant) => ({
const combinations: Combination[] = optimisticVariants.map((variant) => ({
id: variant.id,
availableForSale: variant.availableForSale,
// Adds key / value pairs for each variant (ie. "color": "Black" and "size": 'M").
Expand All @@ -45,14 +53,6 @@ export function VariantSelector({
{option.values.map((value) => {
const optionNameLowerCase = option.name.toLowerCase();

// Base option params on current params so we can preserve any other param state in the url.
const optionSearchParams = new URLSearchParams(searchParams.toString());

// Update the option params using the current option to reflect how the url *would* change,
// if the option was clicked.
optionSearchParams.set(optionNameLowerCase, value);
const optionUrl = createUrl(pathname, optionSearchParams);

// In order to determine if an option is available for sale, we need to:
//
// 1. Filter out all other param state
Expand All @@ -62,7 +62,7 @@ export function VariantSelector({
// This is the "magic" that will cross check possible variant combinations and preemptively
// disable combinations that are not available. For example, if the color gray is only available in size medium,
// then all other sizes should be disabled.
const filtered = Array.from(optionSearchParams.entries()).filter(([key, value]) =>
const filtered = Array.from(optimisticOptions.entries()).filter(([key, value]) =>
options.find(
(option) => option.name.toLowerCase() === key && option.values.includes(value)
)
Expand All @@ -74,15 +74,36 @@ export function VariantSelector({
);

// The option is active if it's in the url params.
const isActive = searchParams.get(optionNameLowerCase) === value;
const isActive = optimisticOptions.get(optionNameLowerCase) === value;

return (
<button
key={value}
aria-disabled={!isAvailableForSale}
disabled={!isAvailableForSale}
onClick={() => {
router.replace(optionUrl, { scroll: false });
startTransition(() => {
const newOptimisticVariants = optimisticVariants.map((variant) => {
const updatedOptions = variant.selectedOptions.map((option) => {
if (option.name.toLowerCase() === optionNameLowerCase) {
return { ...option, value: value };
}
return option;
});

return { ...variant, selectedOptions: updatedOptions };
});

optimisticOptions.set(optionNameLowerCase, value);

setOptimsticVariants(newOptimisticVariants);
setOptimisticOptions(new URLSearchParams(optimisticOptions.toString()));

const optionUrl = createUrl(pathname, optimisticOptions);

// Navigate without page reload
Copy link

Choose a reason for hiding this comment

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

I'm not sure this comment's 100% accurate. It seems that the navigation can't complete without a full page render – at least in the sense that the search params are up-to-date, etc. While the UI is ready to interact with due to the useOptimistic hook, it's still waiting for the full page render before the search params update, so it's now open to race conditions for anything reading from the search params (eg, adding to cart)

router.replace(optionUrl, { scroll: false });
});
}}
title={`${option.name} ${value}${!isAvailableForSale ? ' (Out of Stock)' : ''}`}
className={clsx(
Expand Down
Loading