-
Notifications
You must be signed in to change notification settings - Fork 2k
Dashboard V2: Introduce ActionList and ActionItem components #103371
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
Changes from all commits
0e7560e
75af112
4202bfe
0067479
3f221be
eda5467
af52bfd
2354559
eef916c
9fc183b
b3089b0
fff675e
c9dc67f
ea5989f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
@import "@wordpress/base-styles/mixins"; | ||
@import "@wordpress/base-styles/variables"; | ||
|
||
.action-item { | ||
padding: $grid-unit-20 0; | ||
border-bottom: 1px solid $gray-100; | ||
|
||
&:last-child { | ||
border-bottom: none; | ||
} | ||
|
||
.action-item__decoration { | ||
display: inline-flex; | ||
align-items: center; | ||
justify-content: center; | ||
width: $grid-unit-50; | ||
height: $grid-unit-50; | ||
flex-shrink: 0; | ||
border-radius: $radius-small; | ||
overflow: hidden; | ||
|
||
&:has(svg) { | ||
border: 1px solid $gray-200; | ||
} | ||
|
||
svg { | ||
width: $grid-unit-30; | ||
height: $grid-unit-30; | ||
flex-shrink: 0; | ||
fill: $gray-700; | ||
} | ||
|
||
img { | ||
width: 100%; | ||
height: 100%; | ||
flex-shrink: 0; | ||
object-fit: cover; | ||
} | ||
} | ||
|
||
.action-item__actions { | ||
flex-shrink: 0; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
import { Meta, StoryObj } from '@storybook/react'; | ||
import { Button, Icon } from '@wordpress/components'; | ||
import { cog } from '@wordpress/icons'; | ||
import ActionItem from './action-item'; | ||
|
||
const meta = { | ||
title: 'client/dashboard/ActionList/ActionItem', | ||
component: ActionItem, | ||
tags: [ 'autodocs' ], | ||
parameters: { | ||
actions: { argTypesRegex: '^on.*' }, | ||
}, | ||
} satisfies Meta< typeof ActionItem >; | ||
|
||
export default meta; | ||
type Story = StoryObj< typeof meta >; | ||
|
||
export const Default: Story = { | ||
args: { | ||
title: 'Action item title', | ||
description: 'Action item description', | ||
actions: ( | ||
<Button variant="secondary" size="compact"> | ||
Action | ||
</Button> | ||
), | ||
}, | ||
}; | ||
|
||
export const WithMultipleActions: Story = { | ||
args: { | ||
title: 'Action item title', | ||
description: 'Action item description', | ||
actions: ( | ||
<> | ||
<Button variant="secondary" size="compact"> | ||
Action 1 | ||
</Button> | ||
<Button variant="secondary" size="compact" isDestructive> | ||
Action 2 | ||
</Button> | ||
</> | ||
), | ||
}, | ||
}; | ||
|
||
export const WithIcon: Story = { | ||
args: { | ||
title: 'Action item title', | ||
description: 'Action item description', | ||
decoration: <Icon icon={ cog } />, | ||
actions: ( | ||
<Button variant="secondary" size="compact"> | ||
Action | ||
</Button> | ||
), | ||
}, | ||
}; | ||
|
||
export const WithImage: Story = { | ||
args: { | ||
title: 'Action item title', | ||
description: 'Action item description', | ||
decoration: <Icon icon={ <img src="https://placecats.com/300/200" alt="Cat" /> } />, | ||
actions: ( | ||
<Button variant="secondary" size="compact"> | ||
Action | ||
</Button> | ||
), | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
import { | ||
__experimentalVStack as VStack, | ||
__experimentalHStack as HStack, | ||
__experimentalText as Text, | ||
} from '@wordpress/components'; | ||
import { forwardRef } from 'react'; | ||
import type { ActionItemProps } from './types'; | ||
import './action-item.scss'; | ||
|
||
function UnforwardedActionItem( | ||
{ title, description, decoration, actions }: ActionItemProps, | ||
ref: React.ForwardedRef< HTMLAnchorElement | HTMLButtonElement > | ||
) { | ||
return ( | ||
<VStack className="action-item" ref={ ref } as="span"> | ||
<HStack spacing={ 3 } justify="flex-start" alignment="center" as="span"> | ||
{ !! decoration && <span className="action-item__decoration">{ decoration }</span> } | ||
<HStack as="span"> | ||
<VStack spacing={ 1 } as="span"> | ||
<Text weight={ 500 } lineHeight="20px"> | ||
{ title } | ||
</Text> | ||
{ description && ( | ||
<Text variant="muted" lineHeight="20px"> | ||
{ description } | ||
</Text> | ||
) } | ||
</VStack> | ||
<HStack | ||
className="action-item__actions" | ||
spacing={ 2 } | ||
justify="flex-end" | ||
expanded={ false } | ||
as="span" | ||
> | ||
{ actions } | ||
</HStack> | ||
</HStack> | ||
</HStack> | ||
</VStack> | ||
); | ||
} | ||
|
||
export const ActionItem = forwardRef( UnforwardedActionItem ); | ||
|
||
export default ActionItem; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,150 @@ | ||
import { Meta, StoryObj } from '@storybook/react'; | ||
import { Button, Icon } from '@wordpress/components'; | ||
import { cog } from '@wordpress/icons'; | ||
import ActionList from './index'; | ||
|
||
const meta = { | ||
title: 'client/dashboard/ActionList', | ||
component: ActionList, | ||
tags: [ 'autodocs' ], | ||
parameters: { | ||
actions: { argTypesRegex: '^on.*' }, | ||
}, | ||
} satisfies Meta< typeof ActionList >; | ||
|
||
export default meta; | ||
type Story = StoryObj< typeof meta >; | ||
|
||
export const Default: Story = { | ||
args: { | ||
children: ( | ||
<ActionList.ActionItem | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be just ActionList.Item? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I considered that earlier but I think it would be better to align the name with the Figma file. I'm fine with either 🙂 |
||
title="Action item title" | ||
description="Action item description" | ||
actions={ | ||
<Button variant="secondary" size="compact"> | ||
Action | ||
</Button> | ||
} | ||
/> | ||
), | ||
}, | ||
}; | ||
|
||
export const WithTitle: Story = { | ||
args: { | ||
title: 'Action List', | ||
children: ( | ||
<ActionList.ActionItem | ||
title="Action item title" | ||
description="Action item description" | ||
actions={ | ||
<Button variant="secondary" size="compact"> | ||
Action | ||
</Button> | ||
} | ||
/> | ||
), | ||
}, | ||
}; | ||
|
||
export const WithDescription: Story = { | ||
args: { | ||
title: 'Action List', | ||
description: 'description', | ||
children: ( | ||
<ActionList.ActionItem | ||
title="Action item title" | ||
description="Action item description" | ||
actions={ | ||
<Button variant="secondary" size="compact"> | ||
Action | ||
</Button> | ||
} | ||
/> | ||
), | ||
}, | ||
}; | ||
|
||
export const WithMultipleActionItems: Story = { | ||
args: { | ||
title: 'Action List', | ||
description: 'description', | ||
children: ( | ||
<> | ||
<ActionList.ActionItem | ||
title="Action item title" | ||
description="Action item description" | ||
actions={ | ||
<Button variant="secondary" size="compact"> | ||
Action | ||
</Button> | ||
} | ||
/> | ||
<ActionList.ActionItem | ||
title="Action item title" | ||
description="Action item description" | ||
actions={ | ||
<Button variant="secondary" size="compact"> | ||
Action | ||
</Button> | ||
} | ||
/> | ||
</> | ||
), | ||
}, | ||
}; | ||
|
||
export const FullExample: Story = { | ||
args: { | ||
title: 'Action List', | ||
description: 'description', | ||
children: ( | ||
<> | ||
<ActionList.ActionItem | ||
title="Action item title" | ||
description="Action item description" | ||
actions={ | ||
<Button variant="secondary" size="compact"> | ||
Action | ||
</Button> | ||
} | ||
/> | ||
<ActionList.ActionItem | ||
title="Action item title" | ||
description="Action item description" | ||
actions={ | ||
<> | ||
<Button variant="secondary" size="compact"> | ||
Action 1 | ||
</Button> | ||
<Button variant="secondary" size="compact" isDestructive> | ||
Action 2 | ||
</Button> | ||
</> | ||
} | ||
/> | ||
<ActionList.ActionItem | ||
title="Action item title" | ||
description="Action item description" | ||
decoration={ <Icon icon={ cog } /> } | ||
actions={ | ||
<Button variant="secondary" size="compact"> | ||
Action | ||
</Button> | ||
} | ||
/> | ||
<ActionList.ActionItem | ||
title="Action item title" | ||
description="Action item description" | ||
decoration={ <Icon icon={ <img src="https://placecats.com/300/200" alt="Cat" /> } /> } | ||
actions={ | ||
<Button variant="secondary" size="compact"> | ||
Action | ||
</Button> | ||
} | ||
/> | ||
</> | ||
), | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
import { | ||
__experimentalVStack as VStack, | ||
__experimentalText as Text, | ||
Card, | ||
CardBody, | ||
} from '@wordpress/components'; | ||
import { forwardRef } from 'react'; | ||
import ActionItem from './action-item'; | ||
import type { ActionListProps } from './types'; | ||
import './style.scss'; | ||
|
||
function UnforwardedActionList( | ||
{ title, description, children }: ActionListProps, | ||
ref: React.ForwardedRef< HTMLAnchorElement | HTMLButtonElement > | ||
) { | ||
return ( | ||
<Card className="action-list" ref={ ref }> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't taken a deep look at the code, but one thing that we should consider (even while this component is still implemented locally) is to introduce list semantics — ie. action list could render a title + Additionally, the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also that classname is way too generic and doesn't follow the guidelines. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup. We should try to scope our class names better (ie. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. per our guidelines, this one would be |
||
<CardBody> | ||
{ ( title || description ) && ( | ||
<VStack className="action-list__heading" spacing={ 2 }> | ||
{ title && ( | ||
<Text size="15px" weight={ 500 } lineHeight="20px"> | ||
{ title } | ||
</Text> | ||
) } | ||
{ description && ( | ||
<Text variant="muted" lineHeight="20px"> | ||
{ description } | ||
</Text> | ||
) } | ||
</VStack> | ||
) } | ||
<VStack className="action-list__actions" spacing={ 0 }> | ||
{ children } | ||
</VStack> | ||
</CardBody> | ||
</Card> | ||
); | ||
} | ||
|
||
export const ActionList = Object.assign( forwardRef( UnforwardedActionList ), { | ||
/** | ||
* Renders a action item inside the `ActionList` component. | ||
*/ | ||
ActionItem: Object.assign( ActionItem, { | ||
displayName: 'ActionList.ActionItem', | ||
} ), | ||
} ); | ||
|
||
export default ActionList; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
@import "@wordpress/base-styles/mixins"; | ||
@import "@wordpress/base-styles/variables"; | ||
|
||
.action-list { | ||
.action-list__heading { | ||
padding-top: $grid-unit-10; | ||
padding-bottom: $grid-unit-15; | ||
} | ||
|
||
.action-list__heading ~ .action-list__actions { | ||
margin: -$grid-unit-10 0; | ||
} | ||
|
||
.action-list__actions { | ||
margin: -$grid-unit-20 0; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My feeling is that there's too much CSS here, it's probably ok because this is a reusable UI component but I wonder if we're missing some composition within.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... do you have any ideas for reducing the CSS here 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard to tell, some questions:
That said, it's not a blocker or a big concern in any way, just sharing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not very familiar with
Text
, but I noticed thatPageHeader
also applies styles to the heading, so I initially added styles directly here as well. I've now updated it to pass props to theText
component insteadI think we still need to handle the padding with CSS, depending on whether there's a heading or not 🥲
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately
Text
andHeading
need updating to use the canonical design tokens 😭 It was suggested to not useHeading
inPageHeader
, so it probably makes sense to mimic that here? It goes without saying that ideally we updateText
andHeading
, but that's a separate effort.