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

tech: Create Default ProductCard story #4700

Merged
merged 4 commits into from
Sep 19, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { DetailsList } from './DetailsList'
type Controls = ComponentProps<typeof DetailsList.Root>

const meta: Meta<Controls> = {
title: 'Components / Product Card / DetailsList',
title: 'Components / ProductCard / DetailsList',
component: DetailsList.Root,
parameters: {
design: {
Expand Down
14 changes: 14 additions & 0 deletions apps/store/src/components/ProductCard/Divider.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { tokens } from 'ui'

export const Divider = () => {
return (
<div
style={{
height: 1,
borderBottomWidth: 1,
borderBottomStyle: 'solid',
borderBottomColor: tokens.colors.borderOpaque1,
}}
/>
)
}
121 changes: 121 additions & 0 deletions apps/store/src/components/ProductCard/ProductCard.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
import type { Meta, StoryObj } from '@storybook/react'
import { type ComponentProps } from 'react'
import { Badge, BasePillow, Button, Card, CrossIcon, IconButton, sprinkles, Text, tokens } from 'ui'
import { CurrencyCode } from '@/services/graphql/graphql'
import { InputStartDay } from '../InputDay/InputStartDay'
import { Price } from '../Price'
import { DetailsList } from './DetailsList/DetailsList'
import { Divider } from './Divider'
import { ProductCardDetails } from './ProductCardDetails'

type Controls = ComponentProps<typeof Card.Root>

const meta: Meta<Controls> = {
title: 'Components / ProductCard',
component: Card.Root,
argTypes: {
variant: {
options: ['primary', 'secondary'],
control: { type: 'select' },
},
},
parameters: {
design: {
allowFullscreen: true,
type: 'figma',
url: 'https://www.figma.com/file/5kmmDdh6StpXzbEfr7WevV/Hedvig-UI-Kit?type=design&node-id=18673-5100',
},
},
}
export default meta

type Story = StoryObj<Controls>

export const Default: Story = {
render: (args: Controls) => (
<div style={{ maxWidth: '400px' }}>
<Card.Root variant={args.variant}>
<Card.Aside>
<IconButton variant="secondary">
<CrossIcon />
</IconButton>
</Card.Aside>
<Card.Header>
<Card.Media>
<BasePillow fill={tokens.colors.amber300} />
</Card.Media>
<Card.Heading>
<Card.Title>Homeowner Insurance</Card.Title>
<Card.Subtitle>Bellmansgatan 19A</Card.Subtitle>
</Card.Heading>
</Card.Header>

<ProductCardDetails.Root>
<ProductCardDetails.Trigger>
Copy link
Contributor

Choose a reason for hiding this comment

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

Trigger part could be simpler

<ProductCardDetails.Trigger collapsedLabel="Show details" expendedLabel="hideDetails">

This is both easier to read and you can now express that both properties are required

Copy link
Contributor Author

@Youakeem Youakeem Sep 17, 2024

Choose a reason for hiding this comment

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

Sure! we could do that. I prefer the composition tho so we can nest whatever JSX normally. But will update to use render props 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

We can pass JSX to props just as easily - any prop that is ReactNode will accept JSX

Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking what the good cases for nesting could be

  • Some child needs to be deeper in the tree and surrounded by other components
  • Child is optional, there could be 0, 1 or N in a given parent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can pass JSX to props just as easily - any prop that is ReactNode will accept JSX

Yup, that's what I meant!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the implementation to a children render prop. I think it hits the right balance between flexibility and readability. Let me know what you think!

{(isOpen) => (isOpen ? 'Hide details' : 'Show details')}
</ProductCardDetails.Trigger>

<ProductCardDetails.Content className={sprinkles({ paddingBlock: 'md' })}>
<Text className={sprinkles({ mb: 'xxs' })}>Details</Text>
<DetailsList.Root size="md" className={sprinkles({ mb: 'md' })}>
<DetailsList.Item>
<DetailsList.Label>Home type</DetailsList.Label>
<DetailsList.Value>Homeowner</DetailsList.Value>
</DetailsList.Item>

<DetailsList.Item>
<DetailsList.Label>Address</DetailsList.Label>
<DetailsList.Value>Bellmansgatan 19A</DetailsList.Value>
</DetailsList.Item>

<DetailsList.Item>
<DetailsList.Label>Zip code</DetailsList.Label>
<DetailsList.Value>118 47</DetailsList.Value>
</DetailsList.Item>
</DetailsList.Root>
<Button variant="secondary" size="medium" fullWidth>
Edit information
</Button>
</ProductCardDetails.Content>
</ProductCardDetails.Root>

<InputStartDay />

<DetailsList.Root>
<DetailsList.Item>
<DetailsList.Label>
Homeowner Insurance{' '}
<Badge color="pinkFill1" size="tiny">
Max
</Badge>
</DetailsList.Label>
<DetailsList.Value>379 kr/mo</DetailsList.Value>
</DetailsList.Item>

<DetailsList.Item>
<DetailsList.Label>Extended travel 60 days</DetailsList.Label>
<DetailsList.Value>79 kr/mo</DetailsList.Value>
</DetailsList.Item>
</DetailsList.Root>

<Divider />

<DetailsList.Root size="md">
<DetailsList.Item className={sprinkles({ color: 'textPrimary' })}>
<DetailsList.Label>Total</DetailsList.Label>
<DetailsList.Value>
<Price
className={sprinkles({ justifyContent: 'flex-end' })}
currencyCode={CurrencyCode.Sek}
amount={458}
/>
</DetailsList.Value>
</DetailsList.Item>
</DetailsList.Root>
</Card.Root>
</div>
),
args: {
variant: 'primary',
},
}
66 changes: 66 additions & 0 deletions apps/store/src/components/ProductCard/ProductCardDetails.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import { type ComponentProps, createContext, type ReactNode, useContext, useState } from 'react'
import { Button } from 'ui'
import Collapsible from '@/components/Collapsible/Collapsible'

type ContextValue = {
isOpen: boolean
toggle: () => void
}

const Context = createContext<ContextValue | null>(null)

const useProductDetails = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

You probably no longer need a context and a hook to consume it once Trigger becomes a single component

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We would still need it to render the right text, there is no way to know the open state inside of Trigger right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but this could be just const [isOpen, setIsOpen] = useState(false) - no need to share it between components when only Trigger uses it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yes, true!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thinking about this, we need the state in the Root as well so we can open and close the Collapsible. The state is being set in Trigger but read in both Trigger and Root.

You think we can do this without context?

const context = useContext(Context)

if (!context) {
throw new Error('useProductDetails must be used inside ProductCardDetails')
}

return context
}

type RootProps = ComponentProps<typeof Collapsible.Root>
const Root = ({ children, ...props }: RootProps) => {
const [isOpen, setIsOpen] = useState(false)

const toggle = () => {
setIsOpen((isOpen) => !isOpen)
}

return (
<Collapsible.Root {...props} open={isOpen}>
<Context.Provider value={{ isOpen, toggle }}>{children}</Context.Provider>
</Collapsible.Root>
)
}

type TriggerProps = Omit<ComponentProps<typeof Collapsible.Trigger>, 'children'> & {
children?: ((isOpen: boolean) => ReactNode) | ReactNode
}
const Trigger = ({ children, ...props }: TriggerProps) => {
const { isOpen, toggle } = useProductDetails()

return (
<Collapsible.Trigger asChild {...props}>
<Button variant="outline" size="medium" onClick={toggle} fullWidth>
{typeof children === 'function' ? children(isOpen) : children}
</Button>
</Collapsible.Trigger>
)
}

type ContentProps = ComponentProps<'div'>
const Content = ({ children, ...props }: ContentProps) => {
return (
<Collapsible.Content>
{/* This `div` wraps all content not to conflict with `Collapsible.Content` animation */}
<div {...props}>{children}</div>
</Collapsible.Content>
)
}

export const ProductCardDetails = {
Root,
Trigger,
Content,
}