From c160a7f2602b080b3cd55ee3e72e1825f23c4b42 Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Tue, 24 Jan 2023 16:28:58 -0600 Subject: [PATCH] Refactor `Button` component to TypeScript (#46997) * Rename button/index.js to .jsx * Possible approach to the prop types * Simplify the tag container * Allow passing childrent to TagElement * Fix the type of onKeyDown * Rename test/index.js to .tsx, fix some errors * Fix an eslint error from children being defined in the outer scope * Rename stories/index.js to .tsx, add type annotations * Alphabetize types.ts types * Refactor TagElement to element * Alphabetize an import * Revert changes to element, this will fail type checking * Use the [] array type syntax * Improve the typing of hasChildren * Fix the typing of chilren.length * Revert "Revert changes to element, this will fail type checking" This reverts commit 0b616c785677c407879b6b0f7708409b6ca3dbaf. * Pass the generic of WordPressComponentProps a second argument * Don't destructure out children, simply let them be in props * Use React.MousEvent * Update README.md with types from types.ts * Fix the type of shortcut * Remove unnecessary Type annotations * In README.md, alphabetize the props * Add a Required annotation to the variant prop * Add a JS DocBlock to Button * Add a CHANGELOG entry * Commit Lena's suggestion: Update packages/components/src/button/types.ts Co-authored-by: Lena Morita * Commit Lena's suggestion: Update packages/components/src/button/deprecated.tsx Co-authored-by: Lena Morita * Commit Lena's suggestion: Update packages/components/src/button/index.tsx Co-authored-by: Lena Morita * Apply Lena's suggestion to change 'div' to 'button' Also, correct some typing errors. * Remvoe needless type DisabledEvent * Remove needless argTypes that Storybook will infer from types * Restore a deleted test, thanks to Lena's idea As Lena mentioned, the intent of this test looks like ensuring that additional props are passed to the element. * Don't expect the console to error * Update tooltipPosition in README.md * Move the CHANGELOG entry to Unreleased * Cherry-pick @mirka 's commit to make Button types specific to a or button * Commit Lena's static tests verbatim https://github.com/WordPress/gutenberg/pull/46997#pullrequestreview-1262338583 * Fix a failed test run that I caused * Rename CommonButtonProps to BaseButtonProps * Commit Lena's suggestion: Update packages/components/src/button/stories/index.tsx Co-authored-by: Lena Morita * Commit Lena's diff for static typing test Co-authored-by: Lena Morita --- packages/components/CHANGELOG.md | 1 + .../component.tsx | 2 +- .../border-box-control-linked-button/hook.ts | 2 +- .../border-control-dropdown/component.tsx | 2 +- packages/components/src/button/README.md | 104 +++++----- .../button/{deprecated.js => deprecated.tsx} | 23 ++- .../src/button/{index.js => index.tsx} | 129 +++++++++---- .../components/src/button/stories/index.js | 179 ------------------ .../components/src/button/stories/index.tsx | 106 +++++++++++ .../src/button/test/{index.js => index.tsx} | 37 +++- packages/components/src/button/types.ts | 138 ++++++++++++++ .../components/src/date-time/date/index.tsx | 3 +- .../components/src/dropdown/stories/index.tsx | 2 +- .../components/src/form-token-field/token.tsx | 2 +- packages/components/src/tab-panel/index.tsx | 2 +- packages/components/src/tab-panel/types.ts | 2 +- 16 files changed, 447 insertions(+), 287 deletions(-) rename packages/components/src/button/{deprecated.js => deprecated.tsx} (54%) rename packages/components/src/button/{index.js => index.tsx} (59%) delete mode 100644 packages/components/src/button/stories/index.js create mode 100644 packages/components/src/button/stories/index.tsx rename packages/components/src/button/test/{index.js => index.tsx} (91%) create mode 100644 packages/components/src/button/types.ts diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 118e3b8cbaa221..5190af098b75e4 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -14,6 +14,7 @@ - `DropdownMenu`: migrate Storybook to controls ([47149](https://github.com/WordPress/gutenberg/pull/47149)). - Removed deprecated `@storybook/addon-knobs` dependency from the package ([47152](https://github.com/WordPress/gutenberg/pull/47152)). - `ColorListPicker`: Convert to TypeScript ([#46358](https://github.com/WordPress/gutenberg/pull/46358)). +- `Button`: Convert to TypeScript ([#46997](https://github.com/WordPress/gutenberg/pull/46997)). ### Bug Fix diff --git a/packages/components/src/border-box-control/border-box-control-linked-button/component.tsx b/packages/components/src/border-box-control/border-box-control-linked-button/component.tsx index c4001369f8e015..d5fe6a9eb1f761 100644 --- a/packages/components/src/border-box-control/border-box-control-linked-button/component.tsx +++ b/packages/components/src/border-box-control/border-box-control-linked-button/component.tsx @@ -16,7 +16,7 @@ import { useBorderBoxControlLinkedButton } from './hook'; import type { LinkedButtonProps } from '../types'; const BorderBoxControlLinkedButton = ( - props: WordPressComponentProps< LinkedButtonProps, 'div' >, + props: WordPressComponentProps< LinkedButtonProps, 'button' >, forwardedRef: React.ForwardedRef< any > ) => { const { className, isLinked, ...buttonProps } = diff --git a/packages/components/src/border-box-control/border-box-control-linked-button/hook.ts b/packages/components/src/border-box-control/border-box-control-linked-button/hook.ts index 043c101b05f5a4..27ed54351aaf81 100644 --- a/packages/components/src/border-box-control/border-box-control-linked-button/hook.ts +++ b/packages/components/src/border-box-control/border-box-control-linked-button/hook.ts @@ -13,7 +13,7 @@ import { useCx } from '../../utils/hooks/use-cx'; import type { LinkedButtonProps } from '../types'; export function useBorderBoxControlLinkedButton( - props: WordPressComponentProps< LinkedButtonProps, 'div' > + props: WordPressComponentProps< LinkedButtonProps, 'button' > ) { const { className, diff --git a/packages/components/src/border-control/border-control-dropdown/component.tsx b/packages/components/src/border-control/border-control-dropdown/component.tsx index 1f5bef1a54c1cd..3ec28331ffc425 100644 --- a/packages/components/src/border-control/border-control-dropdown/component.tsx +++ b/packages/components/src/border-control/border-control-dropdown/component.tsx @@ -166,7 +166,7 @@ const BorderControlDropdown = ( onClick={ onToggle } variant="tertiary" aria-label={ toggleAriaLabel } - position={ dropdownPosition } + tooltipPosition={ dropdownPosition } label={ __( 'Border color and style picker' ) } showTooltip={ true } > diff --git a/packages/components/src/button/README.md b/packages/components/src/button/README.md index b8b3a7f3111d2b..66bbae67ebcd57 100644 --- a/packages/components/src/button/README.md +++ b/packages/components/src/button/README.md @@ -121,131 +121,125 @@ The presence of a `href` prop determines whether an `anchor` element is rendered Props not included in this set will be applied to the `a` or `button` element. -#### disabled +#### `children`: `ReactNode` -Whether the button is disabled. If `true`, this will force a `button` element to be rendered. +The button's children. -- Type: `Boolean` - Required: No -#### href +#### `className`: `string` -If provided, renders `a` instead of `button`. +An optional additional class name to apply to the rendered button. -- Type: `String` - Required: No -#### variant +#### `describedBy`: `string` -Specifies the button's style. The accepted values are `'primary'` (the primary button styles), `'secondary'` (the default button styles), `'tertiary'` (the text-based button styles), and `'link'` (the link button styles). +An accessible description for the button. -- Type: `String` - Required: No -#### isDestructive +#### `disabled`: `boolean` -Renders a red text-based button style to indicate destructive behavior. +Whether the button is disabled. If `true`, this will force a `button` element to be rendered. -- Type: `Boolean` - Required: No -#### isSmall +#### `focus`: `boolean` -Decreases the size of the button. +Whether the button is focused. -- Type: `Boolean` - Required: No -#### isPressed +#### `href`: `string` -Renders a pressed button style. +If provided, renders `a` instead of `button`. -- Type: `Boolean` - Required: No -#### isBusy +#### `icon`: `IconProps< unknown >[ 'icon' ]` -Indicates activity while a action is being performed. +If provided, renders an [Icon](/packages/components/src/icon/README.md) component inside the button. -- Type: `Boolean` - Required: No -#### focus +#### `iconPosition`: `'left' | 'right'` -Whether the button is focused. +If provided with `icon`, sets the position of icon relative to the `text`. Available options are `left|right`. -- Type: `Boolean` - Required: No +- Default: `left` -#### target +#### `iconSize`: `IconProps< unknown >[ 'size' ]` -If provided with `href`, sets the `target` attribute to the `a`. +If provided with `icon`, sets the icon size. Please refer to the [Icon](/packages/components/src/icon/README.md) component for more details regarding the default value of its `size` prop. -- Type: `String` - Required: No -#### className +#### `isBusy`: `boolean` -An optional additional class name to apply to the rendered button. +Indicates activity while a action is being performed. -- Type: `String` - Required: No -#### icon +#### `isDestructive`: `boolean` -If provided, renders an [Icon](/packages/components/src/icon/README.md) component inside the button. +Renders a red text-based button style to indicate destructive behavior. -- Type: `String|Function|WPComponent|null` - Required: No -#### iconSize +#### `isPressed`: `boolean` -If provided with `icon`, sets the icon size. Please refer to the [Icon](/packages/components/src/icon/README.md) component for more details regarding the default value of its `size` prop. +Renders a pressed button style. -- Type: `Number` - Required: No -#### iconPosition +#### `isSmall`: `boolean` -If provided with `icon`, sets the position of icon relative to the `text`. Available options are `left|right`. +Decreases the size of the button. -- Type: `string` - Required: No -- Default: `left` -#### text +#### `label`: `string` -If provided, displays the given text inside the button. If the button contains children elements, the text is displayed before them. +Sets the `aria-label` of the component, if none is provided. Sets the Tooltip content if `showTooltip` is provided. -- Type: `String` - Required: No -#### showTooltip +#### `shortcut`: `string | { display: string; ariaLabel: string; }` + +If provided with `showTooltip`, appends the Shortcut label to the tooltip content. If an object is provided, it should contain `display` and `ariaLabel` keys. + +- Required: No + +#### `showTooltip`: `boolean` If provided, renders a [Tooltip](/packages/components/src/tooltip/README.md) component for the button. -- Type: `Boolean` - Required: No -#### tooltipPosition +#### `target`: `string` -If provided with`showTooltip`, sets the position of the tooltip. Please refer to the [Tooltip](/packages/components/src/tooltip/README.md) component for more details regarding the defaults. +If provided with `href`, sets the `target` attribute to the `a`. -- Type: `String` -- Require: No +- Required: No -#### shortcut +#### `text`: `string` -If provided with `showTooltip`, appends the Shortcut label to the tooltip content. If an `Object` is provided, it should contain `display` and `ariaLabel` keys. +If provided, displays the given text inside the button. If the button contains children elements, the text is displayed before them. -- Type: `String|Object` - Required: No -#### label +#### `tooltipPosition`: `PopoverProps[ 'position' ]` -Sets the `aria-label` of the component, if none is provided. Sets the Tooltip content if `showTooltip` is provided. +If provided with`showTooltip`, sets the position of the tooltip. Please refer to the [Tooltip](/packages/components/src/tooltip/README.md) component for more details regarding the defaults. + +- Required: No + +#### `variant`: `'primary' | 'secondary' | 'tertiary' | 'link'` + +Specifies the button's style. The accepted values are `'primary'` (the primary button styles), `'secondary'` (the default button styles), `'tertiary'` (the text-based button styles), and `'link'` (the link button styles). -- Type: `String` - Required: No ## Related components diff --git a/packages/components/src/button/deprecated.js b/packages/components/src/button/deprecated.tsx similarity index 54% rename from packages/components/src/button/deprecated.js rename to packages/components/src/button/deprecated.tsx index 5f18e0e5eb1648..5d39e53c1008c1 100644 --- a/packages/components/src/button/deprecated.js +++ b/packages/components/src/button/deprecated.tsx @@ -1,4 +1,8 @@ -// @ts-nocheck +/** + * External dependencies + */ +import type { ForwardedRef } from 'react'; + /** * WordPress dependencies */ @@ -8,9 +12,20 @@ import { forwardRef } from '@wordpress/element'; /** * Internal dependencies */ -import Button from '../button'; +import Button from '.'; +import type { DeprecatedIconButtonProps } from './types'; -function IconButton( { labelPosition, size, tooltip, label, ...props }, ref ) { +function UnforwardedIconButton( + { + label, + labelPosition, + size, + tooltip, + ...props + }: React.ComponentPropsWithoutRef< typeof Button > & + DeprecatedIconButtonProps, + ref: ForwardedRef< any > +) { deprecated( 'wp.components.IconButton', { since: '5.4', alternative: 'wp.components.Button', @@ -29,4 +44,4 @@ function IconButton( { labelPosition, size, tooltip, label, ...props }, ref ) { ); } -export default forwardRef( IconButton ); +export default forwardRef( UnforwardedIconButton ); diff --git a/packages/components/src/button/index.js b/packages/components/src/button/index.tsx similarity index 59% rename from packages/components/src/button/index.js rename to packages/components/src/button/index.tsx index 1b0fd57a441a91..5ffc22d3020e1c 100644 --- a/packages/components/src/button/index.js +++ b/packages/components/src/button/index.tsx @@ -1,8 +1,14 @@ -// @ts-nocheck /** * External dependencies */ import classnames from 'classnames'; +import type { + ComponentPropsWithoutRef, + ForwardedRef, + HTMLAttributes, + MouseEvent, + ReactElement, +} from 'react'; /** * WordPress dependencies @@ -17,8 +23,9 @@ import { useInstanceId } from '@wordpress/compose'; import Tooltip from '../tooltip'; import Icon from '../icon'; import { VisuallyHidden } from '../visually-hidden'; +import type { ButtonProps, DeprecatedButtonProps } from './types'; -const disabledEventsOnDisabledButton = [ 'onMouseDown', 'onClick' ]; +const disabledEventsOnDisabledButton = [ 'onMouseDown', 'onClick' ] as const; function useDeprecatedProps( { isDefault, @@ -28,7 +35,7 @@ function useDeprecatedProps( { isLink, variant, ...otherProps -} ) { +}: ButtonProps & DeprecatedButtonProps ): ButtonProps { let computedVariant = variant; if ( isPrimary ) { @@ -63,10 +70,11 @@ function useDeprecatedProps( { }; } -export function Button( props, ref ) { +export function UnforwardedButton( + props: ButtonProps, + ref: ForwardedRef< any > +) { const { - href, - target, isSmall, isPressed, isBusy, @@ -85,18 +93,26 @@ export function Button( props, ref ) { variant, __experimentalIsFocusable: isFocusable, describedBy, - ...additionalProps + ...buttonOrAnchorProps } = useDeprecatedProps( props ); + + const { href, target, ...additionalProps } = + 'href' in buttonOrAnchorProps + ? buttonOrAnchorProps + : { href: undefined, target: undefined, ...buttonOrAnchorProps }; + const instanceId = useInstanceId( Button, 'components-button__description' ); const hasChildren = - children?.[ 0 ] && - children[ 0 ] !== null && - // Tooltip should not considered as a child - children?.[ 0 ]?.props?.className !== 'components-tooltip'; + ( 'string' === typeof children && !! children ) || + ( Array.isArray( children ) && + children?.[ 0 ] && + children[ 0 ] !== null && + // Tooltip should not considered as a child + children?.[ 0 ]?.props?.className !== 'components-tooltip' ); const classes = classnames( 'components-button', className, { 'is-secondary': variant === 'secondary', @@ -113,24 +129,29 @@ export function Button( props, ref ) { const trulyDisabled = disabled && ! isFocusable; const Tag = href !== undefined && ! trulyDisabled ? 'a' : 'button'; - const tagProps = - Tag === 'a' - ? { href, target } - : { + const buttonProps: ComponentPropsWithoutRef< 'button' > = + Tag === 'button' + ? { type: 'button', disabled: trulyDisabled, 'aria-pressed': isPressed, - }; + } + : {}; + const anchorProps: ComponentPropsWithoutRef< 'a' > = + Tag === 'a' ? { href, target } : {}; if ( disabled && isFocusable ) { // In this case, the button will be disabled, but still focusable and // perceivable by screen reader users. - tagProps[ 'aria-disabled' ] = true; + buttonProps[ 'aria-disabled' ] = true; + anchorProps[ 'aria-disabled' ] = true; for ( const disabledEvent of disabledEventsOnDisabledButton ) { - additionalProps[ disabledEvent ] = ( event ) => { - event.stopPropagation(); - event.preventDefault(); + additionalProps[ disabledEvent ] = ( event: MouseEvent ) => { + if ( event ) { + event.stopPropagation(); + event.preventDefault(); + } }; } } @@ -145,24 +166,24 @@ export function Button( props, ref ) { // There's a label and... ( !! label && // The children are empty and... - ! children?.length && + ! ( children as string | ReactElement[] )?.length && // The tooltip is not explicitly disabled. false !== showTooltip ) ); - const descriptionId = describedBy ? instanceId : null; + const descriptionId = describedBy ? instanceId : undefined; const describedById = additionalProps[ 'aria-describedby' ] || descriptionId; - const element = ( - + const commonProps = { + className: classes, + 'aria-label': additionalProps[ 'aria-label' ] || label, + 'aria-describedby': describedById, + ref, + }; + + const elementChildren = ( + <> { icon && iconPosition === 'left' && ( ) } @@ -171,9 +192,28 @@ export function Button( props, ref ) { ) } { children } - + ); + const element = + Tag === 'a' ? ( + ) } + { ...commonProps } + > + { elementChildren } + + ) : ( + + ); + if ( ! shouldShowTooltip ) { return ( <> @@ -190,7 +230,12 @@ export function Button( props, ref ) { return ( <> @@ -205,4 +250,20 @@ export function Button( props, ref ) { ); } -export default forwardRef( Button ); +/** + * Lets users take actions and make choices with a single click or tap. + * + * ```jsx + * import { Button } from '@wordpress/components'; + * const Mybutton = () => ( + * + * ); + * ``` + */ +export const Button = forwardRef( UnforwardedButton ); +export default Button; diff --git a/packages/components/src/button/stories/index.js b/packages/components/src/button/stories/index.js deleted file mode 100644 index 569bd845c34253..00000000000000 --- a/packages/components/src/button/stories/index.js +++ /dev/null @@ -1,179 +0,0 @@ -/** - * WordPress dependencies - */ -import { - formatBold, - formatItalic, - link, - more, - wordpress, -} from '@wordpress/icons'; - -/** - * Internal dependencies - */ -import './style.css'; -import Button from '../'; - -export default { - title: 'Components/Button', - component: Button, - argTypes: { - label: { - control: { type: 'text' }, - description: - 'Sets the `aria-label` of the component, if none is provided. Sets the Tooltip content if `showTooltip` is provided.', - }, - children: { - control: { type: 'text' }, - }, - href: { - control: { type: 'text' }, - description: 'If provided, renders `a` instead of `button`.', - }, - icon: { - control: { type: 'select' }, - description: - 'If provided, renders an `Icon` component inside the button.', - options: [ 'wordpress', 'link', 'more' ], - mapping: { - wordpress, - link, - more, - }, - }, - iconSize: { - control: { type: 'number' }, - description: 'If provided with `icon`, sets the icon size.', - }, - iconPosition: { - control: { type: 'select' }, - description: - 'If provided with `icon`, sets the position of icon relative to the `text`. Available options are `left|right`.', - options: [ 'left', 'right' ], - table: { - defaultValue: { summary: `left` }, - }, - }, - isBusy: { - control: 'boolean', - description: - 'Indicates activity while a action is being performed.', - }, - isDestructive: { - control: 'boolean', - description: - 'Renders a red text-based button style to indicate destructive behavior.', - }, - isPressed: { - control: 'boolean', - description: 'Renders a pressed button style.', - }, - isSmall: { - control: 'boolean', - description: 'Decreases the size of the button.', - }, - disabled: { - control: 'boolean', - description: - 'Whether the button is disabled. If `true`, this will force a `button` element to be rendered.', - }, - shortcut: { - control: { type: 'text' }, - description: - 'If provided with `showTooltip`, appends the Shortcut label to the tooltip content. If an `Object` is provided, it should contain `display` and `ariaLabel` keys.', - }, - showTooltip: { - control: 'boolean', - description: - 'If provided, renders a `Tooltip` component for the button.', - }, - tooltipPosition: { - control: { type: 'text' }, - description: - 'If provided with `showTooltip`, sets the position of the tooltip.', - }, - text: { - control: { type: 'text' }, - description: - 'If provided, displays the given text inside the button. If the button contains `children` elements, the text is displayed before them.', - }, - target: { - control: { type: 'text' }, - description: - 'If provided with `href`, sets the `target` attribute to the `a`.', - }, - variant: { - control: { type: 'select' }, - description: "Specifies the button's style.", - options: [ 'primary', 'secondary', 'tertiary', 'link' ], - }, - __experimentalIsFocusable: { - control: 'boolean', - description: 'Makes the button focusable even when disabled', - }, - }, - parameters: { - controls: { expanded: true }, - docs: { source: { state: 'open' } }, - }, -}; - -function Template( { children, ...props } ) { - return ; -} - -export const Default = Template.bind( {} ); -Default.args = { - children: 'Code is poetry', -}; - -export const Primary = Template.bind( {} ); -Primary.args = { - ...Default.args, - variant: 'primary', -}; - -export const Secondary = Template.bind( {} ); -Secondary.args = { - ...Default.args, - variant: 'secondary', -}; - -export const Tertiary = Template.bind( {} ); -Tertiary.args = { - ...Default.args, - variant: 'tertiary', -}; - -export const Link = Template.bind( {} ); -Link.args = { - ...Default.args, - variant: 'link', -}; - -export const IsDestructive = Template.bind( {} ); -IsDestructive.args = { - ...Default.args, - isDestructive: true, -}; - -export const Icon = Template.bind( {} ); -Icon.args = { - label: 'Code is poetry', - icon: 'wordpress', -}; - -export const groupedIcons = () => { - const GroupContainer = ( { children } ) => ( -
{ children }
- ); - - return ( - - ; +}; + +export const Default: ComponentStory< typeof Button > = Template.bind( {} ); +Default.args = { + children: 'Code is poetry', +}; + +export const Primary: ComponentStory< typeof Button > = Template.bind( {} ); +Primary.args = { + ...Default.args, + variant: 'primary', +}; + +export const Secondary: ComponentStory< typeof Button > = Template.bind( {} ); +Secondary.args = { + ...Default.args, + variant: 'secondary', +}; + +export const Tertiary: ComponentStory< typeof Button > = Template.bind( {} ); +Tertiary.args = { + ...Default.args, + variant: 'tertiary', +}; + +export const Link: ComponentStory< typeof Button > = Template.bind( {} ); +Link.args = { + ...Default.args, + variant: 'link', +}; + +export const IsDestructive: ComponentStory< typeof Button > = Template.bind( + {} +); +IsDestructive.args = { + ...Default.args, + isDestructive: true, +}; + +export const Icon: ComponentStory< typeof Button > = Template.bind( {} ); +Icon.args = { + label: 'Code is poetry', + icon: 'wordpress', +}; + +export const GroupedIcons: ComponentStory< typeof Button > = () => { + const GroupContainer = ( { children }: { children: ReactNode } ) => ( +
{ children }
+ ); + + return ( + + ), diff --git a/packages/components/src/form-token-field/token.tsx b/packages/components/src/form-token-field/token.tsx index 9d2c522037d8e9..e5633aa3b75cf0 100644 --- a/packages/components/src/form-token-field/token.tsx +++ b/packages/components/src/form-token-field/token.tsx @@ -73,7 +73,7 @@ export default function Token( {