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: visually editable PDP #1808

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 3 additions & 1 deletion core/app/[locale]/(default)/product/[slug]/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import { getFormatter, getTranslations, setRequestLocale } from 'next-intl/serve

import { Stream } from '@/vibes/soul/lib/streamable';
import { FeaturedProductsCarousel } from '@/vibes/soul/sections/featured-products-carousel';
import { ProductDetail } from '@/vibes/soul/sections/product-detail';
import { pricesTransformer } from '~/data-transformers/prices-transformer';
import { productCardTransformer } from '~/data-transformers/product-card-transformer';
import { productOptionsTransformer } from '~/data-transformers/product-options-transformer';
import { ProductDetail } from '~/lib/makeswift/components/product-detail';

import { addToCart } from './_actions/add-to-cart';
import { ProductSchema } from './_components/product-schema';
Expand Down Expand Up @@ -100,6 +100,7 @@ const getProduct = async (productPromise: ReturnType<typeof getProductData>) =>
description: (
<div className="prose" dangerouslySetInnerHTML={{ __html: product.description }} />
),
plainTextDescription: product.plainTextDescription,
href: product.path,
images: product.defaultImage
? [{ src: product.defaultImage.url, alt: product.defaultImage.altText }, ...images]
Expand Down Expand Up @@ -232,6 +233,7 @@ export default async function Product(props: Props) {
incrementLabel={t('ProductDetails.increaseQuantity')}
prefetch={true}
product={getProduct(productPromise)}
productId={productId}
quantityLabel={t('ProductDetails.quantity')}
thumbnailLabel={t('ProductDetails.thumbnail')}
/>
Expand Down
5 changes: 3 additions & 2 deletions core/lib/makeswift/component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ import { getComponentSnapshot } from '~/lib/makeswift/client';

export const Component = async ({
snapshotId,
label,
...props
}: {
type: string;
label: string;
label: string | Promise<string>;
Copy link
Author

Choose a reason for hiding this comment

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

Allow label to be a promise so we can provide a descriptive label here.

Note that waiting for the label to resolve below feels a bit off as strictly speaking label is not required for rendering the snapshot element tree and could theoretically be resolved in parallel. Might be something we want to pursue later.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be Streamable<string>?

Copy link
Author

Choose a reason for hiding this comment

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

I didn't want to make this dependent on Vibes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Catalyst itself depends on VIBES so this shouldn't be an issue. We're also planning on creating a react-streamable library and Catalyst will have it as it will be required to use with VIBES. Not a big deal, though

Copy link
Author

Choose a reason for hiding this comment

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

Catalyst itself depends on VIBES so this shouldn't be an issue.

I know it does, but I view the collection of "userland" Makeswift boilerplate in this dir as a prototype for our updated integration guidelines, and I don't think they should depend on VIBES.

snapshotId: string;
}) => {
const snapshot = await getComponentSnapshot(snapshotId);

return <MakeswiftComponent snapshot={snapshot} {...props} />;
return <MakeswiftComponent label={await label} snapshot={snapshot} {...props} />;
};
1 change: 1 addition & 0 deletions core/lib/makeswift/components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import './components/site-footer/site-footer.makeswift';
import './components/site-header/site-header.makeswift';
import './components/slideshow/slideshow.makeswift';
import './components/sticky-sidebar/sticky-sidebar.makeswift';
import './components/product-detail/register';

import './components/site-theme/register';

Expand Down
125 changes: 125 additions & 0 deletions core/lib/makeswift/components/product-detail/client.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
'use client';

import React, {
type ComponentPropsWithoutRef,
createContext,
forwardRef,
type PropsWithChildren,
type ReactNode,
type Ref,
useCallback,
useContext,
} from 'react';

import { Stream, type Streamable } from '@/vibes/soul/lib/streamable';
import { ProductDetail, ProductDetailSkeleton } from '@/vibes/soul/sections/product-detail';
import { mergeSections } from '~/lib/makeswift/utils/merge-sections';

type VibesProductDetailProps = ComponentPropsWithoutRef<typeof ProductDetail>;
type VibesProductDetail = Exclude<Awaited<VibesProductDetailProps['product']>, null>;

export type ProductDetail = VibesProductDetail & {
plainTextDescription?: string;
};

export type Props = Omit<VibesProductDetailProps, 'product'> & {
product: Streamable<ProductDetail>;
};

const PropsContext = createContext<Props | null>(null);

export const PropsContextProvider = ({ value, children }: PropsWithChildren<{ value: Props }>) => (
<PropsContext.Provider value={value}>{children}</PropsContext.Provider>
);

export const DescriptionSource = {
CatalogPlainText: 'CatalogPlainText',
CatalogRichText: 'CatalogRichText',
Custom: 'Custom',
} as const;

type DescriptionSource = (typeof DescriptionSource)[keyof typeof DescriptionSource];

interface EditableProps {
summaryText: string | undefined;
description: { source: DescriptionSource; slot: ReactNode };
accordions: Exclude<Awaited<VibesProductDetail['accordions']>, undefined>;
}

const ProductDetailImpl = ({
summaryText,
description,
accordions,
product: streamableProduct,
...props
}: Props & EditableProps) => {
const getProductDescription = useCallback(
(product: ProductDetail): ProductDetail['description'] => {
switch (description.source) {
case DescriptionSource.CatalogPlainText:
return product.plainTextDescription;

case DescriptionSource.CatalogRichText:
return product.description;

case DescriptionSource.Custom:
return description.slot;
}
},
[description.source, description.slot],
);

const getProductAccordions = useCallback(
(
productAccordions: Awaited<ProductDetail['accordions']>,
): Awaited<ProductDetail['accordions']> =>
productAccordions != null
? mergeSections(productAccordions, accordions, (left, right) => ({
...left,
content: right.content,
}))
: undefined,
[accordions],
);

return (
<Stream fallback={<ProductDetailSkeleton />} value={streamableProduct}>
{(product) => (
<Stream fallback={<ProductDetailSkeleton />} value={product.accordions}>
{(productAccordions) => (
agurtovoy marked this conversation as resolved.
Show resolved Hide resolved
<ProductDetail
{...{
...props,
product: {
...product,
summary: summaryText,
description: getProductDescription(product),
accordions: getProductAccordions(productAccordions),
},
}}
/>
)}
</Stream>
)}
</Stream>
);
};

export const MakeswiftProductDetail = forwardRef(
(props: EditableProps, ref: Ref<HTMLDivElement>) => {
const passedProps = useContext(PropsContext);

if (passedProps == null) {
// eslint-disable-next-line no-console
console.error('No context provided for MakeswiftProductDetail');

return <p ref={ref}>There was an error rendering the product detail.</p>;
}

return (
<div className="flex flex-col" ref={ref}>
<ProductDetailImpl {...{ ...passedProps, ...props }} />
</div>
);
},
);
17 changes: 17 additions & 0 deletions core/lib/makeswift/components/product-detail/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { Component } from '~/lib/makeswift/component';

import { type Props as ClientProps, PropsContextProvider } from './client';
import { COMPONENT_TYPE } from './register';

type Props = ClientProps & { productId: number };

export const ProductDetail = ({ productId, ...props }: Props) => (
<PropsContextProvider value={props}>
<Component
label={(async () =>
`Detail for ${await Promise.resolve(props.product).then(({ title }) => title)}`)()}
snapshotId={`product-detail-${productId}`}
type={COMPONENT_TYPE}
/>
</PropsContextProvider>
);
46 changes: 46 additions & 0 deletions core/lib/makeswift/components/product-detail/register.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { List, Select, Shape, Slot, TextArea, TextInput } from '@makeswift/runtime/controls';

import { runtime } from '~/lib/makeswift/runtime';

import { DescriptionSource, MakeswiftProductDetail } from './client';

export const COMPONENT_TYPE = 'catalyst-makeswift-product-detail-description';

const description = Shape({
label: 'Description',
type: {
source: Select({
label: 'Source',
options: [
{ label: 'Catalog (plain text)', value: DescriptionSource.CatalogPlainText },
{ label: 'Catalog (rich text)', value: DescriptionSource.CatalogRichText },
{ label: 'Custom', value: DescriptionSource.Custom },
],
defaultValue: DescriptionSource.CatalogRichText,
}),
slot: Slot(),
},
});

runtime.registerComponent(MakeswiftProductDetail, {
type: COMPONENT_TYPE,
label: 'MakeswiftProductDetail (private)',
hidden: true,
props: {
summaryText: TextArea({
label: 'Summary',
}),
description,
accordions: List({
label: 'Product info',
type: Shape({
label: 'Product info section',
type: {
title: TextInput({ label: 'Title', defaultValue: 'Section' }),
content: Slot(),
},
}),
getItemLabel: (section) => section?.title || 'Section',
}),
},
});
77 changes: 77 additions & 0 deletions core/vibes/soul/lib/streamable.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,84 @@
import { Suspense, use } from 'react';
import { v4 as uuid } from 'uuid';
agurtovoy marked this conversation as resolved.
Show resolved Hide resolved

export type Streamable<T> = T | Promise<T>;

// eslint-disable-next-line func-names
const stableKeys = (function () {
const cache = new WeakMap<object, string>();

function getObjectKey(obj: object): string {
const key = cache.get(obj);

if (key !== undefined) {
return key;
}

const keyValue = uuid();

cache.set(obj, keyValue);

return keyValue;
}

return {
get: (streamable: unknown): string =>
streamable != null && typeof streamable === 'object'
? getObjectKey(streamable)
: JSON.stringify(streamable),
};
})();

function getCompositeKey(streamables: readonly unknown[]): string {
return streamables.map(stableKeys.get).join('.');
}

function weakRefCache<K, T extends object>() {
const cache = new Map<K, WeakRef<T>>();

const registry = new FinalizationRegistry((key: K) => {
const valueRef = cache.get(key);

if (valueRef && !valueRef.deref()) cache.delete(key);
});

return {
get: (key: K) => cache.get(key)?.deref(),
set: (key: K, value: T) => {
cache.set(key, new WeakRef(value));
registry.register(value, key);
},
};
}

const promiseCache = weakRefCache<string, Promise<unknown>>();

// eslint-disable-next-line valid-jsdoc
/**
* A suspense-friendly upgrade to `Promise.all`, guarantees stability of
* the returned promise instance if passed an identical set of inputs.
*/
export function all<T extends readonly unknown[] | []>(
streamables: T,
): Streamable<{ -readonly [P in keyof T]: Awaited<T[P]> }> {
const cacheKey = getCompositeKey(streamables);

const cached = promiseCache.get(cacheKey);

// eslint-disable-next-line @typescript-eslint/consistent-type-assertions
if (cached != null) return cached as { -readonly [P in keyof T]: Awaited<T[P]> };

const result = Promise.all(streamables);

promiseCache.set(cacheKey, result);

return result;
}

export const Streamable = {
all,
};

export function useStreamable<T>(streamable: Streamable<T>): T {
return streamable instanceof Promise ? use(streamable) : streamable;
}
Expand Down
4 changes: 2 additions & 2 deletions core/vibes/soul/primitives/products-list/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export function ProductsList({
<>
<Stream
fallback={<ProductsListSkeleton pending placeholderCount={placeholderCount} />}
value={Promise.all([streamableProducts, streamableCompareLabel])}
value={Streamable.all([streamableProducts, streamableCompareLabel])}
>
{([products, compareLabel]) => {
if (products.length === 0) {
Expand Down Expand Up @@ -76,7 +76,7 @@ export function ProductsList({
);
}}
</Stream>
<Stream value={Promise.all([streamableCompareProducts, streamableCompareLabel])}>
<Stream value={Streamable.all([streamableCompareProducts, streamableCompareLabel])}>
{([compareProducts, compareLabel]) =>
compareProducts && (
<CompareDrawer
Expand Down
7 changes: 3 additions & 4 deletions core/vibes/soul/sections/product-detail/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export function ProductDetail<F extends Field>({

<Stream
fallback={<ProductDetailFormSkeleton />}
value={Promise.all([
value={Streamable.all([
streamableFields,
streamableCtaLabel,
streamableCtaDisabled,
Expand All @@ -129,8 +129,7 @@ export function ProductDetail<F extends Field>({

<Stream fallback={<ProductDescriptionSkeleton />} value={product.description}>
{(description) =>
description !== null &&
description !== undefined && (
description != null && (
<div className="border-t border-contrast-100 py-8 text-contrast-500">
{description}
</div>
Expand Down Expand Up @@ -284,7 +283,7 @@ function ProductAccordionsSkeleton() {
);
}

function ProductDetailSkeleton() {
export function ProductDetailSkeleton() {
Copy link
Author

@agurtovoy agurtovoy Jan 8, 2025

Choose a reason for hiding this comment

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

TODO: put up a PR for VIBES that exports this.

return (
<div className="grid animate-pulse grid-cols-1 items-stretch gap-x-6 gap-y-8 @2xl:grid-cols-2 @5xl:gap-x-12">
<div className="hidden @2xl:block">
Expand Down
2 changes: 1 addition & 1 deletion core/vibes/soul/sections/products-list-section/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export function ProductsListSection({
<div className="flex gap-2">
<Stream
fallback={<SortingSkeleton />}
value={Promise.all([
value={Streamable.all([
streamableSortLabel,
streamableSortOptions,
streamableSortPlaceholder,
Expand Down
Loading