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

DataViews: Type the ItemActions component #61400

Merged
merged 4 commits into from
May 14, 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
@@ -1,3 +1,8 @@
/**
* External dependencies
*/
import type { MouseEventHandler, ReactElement } from 'react';

/**
* WordPress dependencies
*/
Expand All @@ -15,6 +20,7 @@ import { moreVertical } from '@wordpress/icons';
* Internal dependencies
*/
import { unlock } from './lock-unlock';
import type { Action, ActionModal as ActionModalType, Item } from './types';

const {
DropdownMenuV2: DropdownMenu,
Expand All @@ -24,7 +30,48 @@ const {
kebabCase,
} = unlock( componentsPrivateApis );

function ButtonTrigger( { action, onClick } ) {
interface ButtonTriggerProps {
action: Action;
onClick: MouseEventHandler;
}

interface DropdownMenuItemTriggerProps {
action: Action;
onClick: MouseEventHandler;
}

interface ActionModalProps {
action: ActionModalType;
items: Item[];
closeModal?: () => void;
onActionStart?: ( items: Item[] ) => void;
onActionPerformed?: ( items: Item[] ) => void;
}

interface ActionWithModalProps extends ActionModalProps {
ActionTrigger: (
props: ButtonTriggerProps | DropdownMenuItemTriggerProps
) => ReactElement;
isBusy?: boolean;
}

interface ActionsDropdownMenuGroupProps {
actions: Action[];
item: Item;
}

interface ItemActionsProps {
item: Item;
actions: Action[];
isCompact: boolean;
}

interface CompactItemActionsProps {
item: Item;
actions: Action[];
}

function ButtonTrigger( { action, onClick }: ButtonTriggerProps ) {
return (
<Button
label={ action.label }
Expand All @@ -36,11 +83,14 @@ function ButtonTrigger( { action, onClick } ) {
);
}

function DropdownMenuItemTrigger( { action, onClick } ) {
function DropdownMenuItemTrigger( {
action,
onClick,
}: DropdownMenuItemTriggerProps ) {
return (
<DropdownMenuItem
onClick={ onClick }
hideOnClick={ ! action.RenderModal }
hideOnClick={ ! ( 'RenderModal' in action ) }
>
<DropdownMenuItemLabel>{ action.label }</DropdownMenuItemLabel>
</DropdownMenuItem>
Expand All @@ -53,12 +103,12 @@ export function ActionModal( {
closeModal,
onActionStart,
onActionPerformed,
} ) {
}: ActionModalProps ) {
return (
<Modal
title={ action.modalHeader || action.label }
__experimentalHideHeader={ !! action.hideModalHeader }
onRequestClose={ closeModal }
onRequestClose={ closeModal ?? ( () => {} ) }
overlayClassName={ `dataviews-action-modal dataviews-action-modal__${ kebabCase(
action.id
) }` }
Expand All @@ -80,7 +130,7 @@ export function ActionWithModal( {
onActionStart,
onActionPerformed,
isBusy,
} ) {
}: ActionWithModalProps ) {
const [ isModalOpen, setIsModalOpen ] = useState( false );
const actionTriggerProps = {
action,
Expand All @@ -106,11 +156,14 @@ export function ActionWithModal( {
);
}

export function ActionsDropdownMenuGroup( { actions, item } ) {
export function ActionsDropdownMenuGroup( {
actions,
item,
}: ActionsDropdownMenuGroupProps ) {
return (
<DropdownMenuGroup>
{ actions.map( ( action ) => {
if ( !! action.RenderModal ) {
if ( 'RenderModal' in action ) {
Copy link
Member

@oandregal oandregal May 7, 2024

Choose a reason for hiding this comment

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

This is more readable, I like it. Is this related to any TypeScript thing I should be aware of or just an unrelated change that was nice to have?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, basically when you do this, typescript is able to figure out the type of action that has the "RenderModal" defined. If you keep the old check, typescript will complain that RenderModal may or may not be defined.

return (
<ActionWithModal
key={ action.id }
Expand All @@ -132,7 +185,11 @@ export function ActionsDropdownMenuGroup( { actions, item } ) {
);
}

export default function ItemActions( { item, actions, isCompact } ) {
export default function ItemActions( {
item,
actions,
isCompact,
}: ItemActionsProps ) {
const { primaryActions, eligibleActions } = useMemo( () => {
// If an action is eligible for all items, doesn't need
// to provide the `isEligible` function.
Expand Down Expand Up @@ -162,7 +219,7 @@ export default function ItemActions( { item, actions, isCompact } ) {
>
{ !! primaryActions.length &&
primaryActions.map( ( action ) => {
if ( !! action.RenderModal ) {
if ( 'RenderModal' in action ) {
return (
<ActionWithModal
key={ action.id }
Expand All @@ -185,7 +242,7 @@ export default function ItemActions( { item, actions, isCompact } ) {
);
}

function CompactItemActions( { item, actions } ) {
function CompactItemActions( { item, actions }: CompactItemActionsProps ) {
return (
<DropdownMenu
trigger={
Expand Down
77 changes: 76 additions & 1 deletion packages/dataviews/src/types.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import type { ReactNode } from 'react';
import type { ReactElement, ReactNode } from 'react';

interface Option {
value: any;
Expand Down Expand Up @@ -164,3 +164,78 @@ export interface ViewList extends ViewBase {
}

export type View = ViewList | ViewBase;

interface ActionBase {
/**
* The unique identifier of the action.
*/
id: string;

/**
* The label of the action.
*/
label: string;

/**
* The icon of the action. (Either a string or an SVG element)
* This should be IconType from the components package
* but that import is breaking typescript build for the moment.
*/
icon?: any;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it turns out that it was the import of IconType from the components package that was breaking the build in this PR. Using any temporarily should fix it.

Copy link
Member

Choose a reason for hiding this comment

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

😆 Just when #61486 is almost ready.

Copy link
Member

@sirreal sirreal May 14, 2024

Choose a reason for hiding this comment

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

Some detail:

I think this works around the issue that #61486 addresses. The problem was a symptom of the skipLibCheck not being present in this package's tsconfig. Because of problems in library types, anything that depended on the components package was effectively "infected" by invalid library types and would then require skipLibCheck.


/**
* Whether the action is disabled.
*/
disabled?: boolean;

/**
* Whether the action is destructive.
*/
isDestructive?: boolean;

/**
* Whether the action is a primary action.
*/
isPrimary?: boolean;

/**
* Whether the item passed as an argument supports the current action.
*/
isEligible?: ( item: Item ) => boolean;
}

export interface ActionModal extends ActionBase {
/**
* Modal to render when the action is triggered.
*/
RenderModal: ( {
items,
closeModal,
onActionStart,
onActionPerformed,
}: {
items: Item[];
closeModal?: () => void;
onActionStart?: ( items: Item[] ) => void;
onActionPerformed?: ( items: Item[] ) => void;
} ) => ReactElement;

/**
* Whether to hide the modal header.
*/
hideModalHeader?: boolean;

/**
* The header of the modal.
*/
modalHeader?: string;
}

export interface ActionButton extends ActionBase {
/**
* The callback to execute when the action is triggered.
*/
callback: ( items: Item[] ) => void;
}

export type Action = ActionModal | ActionButton;
60 changes: 25 additions & 35 deletions packages/dataviews/src/view-list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { moreVertical } from '@wordpress/icons';
*/
import { unlock } from './lock-unlock';
import type {
Action,
Data,
Item,
NormalizedField,
Expand All @@ -41,18 +42,6 @@ import type {

import { ActionsDropdownMenuGroup, ActionModal } from './item-actions';

interface Action {
callback: ( items: Item[] ) => void;
icon: any;
id: string;
isDestructive: boolean | undefined;
isEligible: ( item: Item ) => boolean | undefined;
isPrimary: boolean | undefined;
label: string;
modalHeader: string;
RenderModal: ( props: any ) => JSX.Element;
}

interface ListViewProps {
actions: Action[];
data: Data;
Expand Down Expand Up @@ -215,7 +204,7 @@ function ListItem( {
width: 'auto',
} }
>
{ primaryAction && !! primaryAction.RenderModal && (
{ primaryAction && 'RenderModal' in primaryAction && (
<div role="gridcell">
<CompositeItem
store={ store }
Expand Down Expand Up @@ -247,28 +236,29 @@ function ListItem( {
</CompositeItem>
</div>
) }
{ primaryAction && ! primaryAction.RenderModal && (
<div role="gridcell" key={ primaryAction.id }>
<CompositeItem
store={ store }
render={
<Button
label={ primaryAction.label }
icon={ primaryAction.icon }
isDestructive={
primaryAction.isDestructive
}
size="compact"
onClick={ () =>
primaryAction.callback( [
item,
] )
}
/>
}
/>
</div>
) }
{ primaryAction &&
! ( 'RenderModal' in primaryAction ) && (
<div role="gridcell" key={ primaryAction.id }>
<CompositeItem
store={ store }
render={
<Button
label={ primaryAction.label }
icon={ primaryAction.icon }
isDestructive={
primaryAction.isDestructive
}
size="compact"
onClick={ () =>
primaryAction.callback( [
item,
] )
}
/>
}
/>
</div>
) }
<div role="gridcell">
<DropdownMenu
trigger={
Expand Down
Loading