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

Refactor Button component to TypeScript #46997

Merged
merged 46 commits into from
Jan 24, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
c05948f
Rename button/index.js to .jsx
kienstra Jan 3, 2023
5b57ef6
Possible approach to the prop types
kienstra Jan 5, 2023
718959c
Simplify the tag container
kienstra Jan 9, 2023
d8cae0c
Allow passing childrent to TagElement
kienstra Jan 9, 2023
d007837
Fix the type of onKeyDown
kienstra Jan 10, 2023
6ab8806
Rename test/index.js to .tsx, fix some errors
kienstra Jan 10, 2023
878c77d
Fix an eslint error from children being defined in the outer scope
kienstra Jan 10, 2023
78e0aaf
Rename stories/index.js to .tsx, add type annotations
kienstra Jan 10, 2023
00c05da
Merge branch 'trunk' into update/button-ts
kienstra Jan 10, 2023
9e6cc7d
Alphabetize types.ts types
kienstra Jan 10, 2023
9298bd7
Refactor TagElement to element
kienstra Jan 10, 2023
7314d0a
Alphabetize an import
kienstra Jan 10, 2023
0b616c7
Revert changes to element, this will fail type checking
kienstra Jan 10, 2023
3998f8b
Merge branch 'trunk' into update/button-ts
kienstra Jan 15, 2023
67efafe
Use the [] array type syntax
kienstra Jan 15, 2023
bbc7cd4
Improve the typing of hasChildren
kienstra Jan 15, 2023
0f7a242
Fix the typing of chilren.length
kienstra Jan 15, 2023
ca6ded1
Revert "Revert changes to element, this will fail type checking"
kienstra Jan 15, 2023
dadbb4b
Pass the generic of WordPressComponentProps a second argument
kienstra Jan 15, 2023
a447deb
Don't destructure out children, simply let them be in props
kienstra Jan 15, 2023
e702dd4
Use React.MousEvent
kienstra Jan 15, 2023
0badec8
Update README.md with types from types.ts
kienstra Jan 15, 2023
b9a3b50
Fix the type of shortcut
kienstra Jan 15, 2023
57866b7
Remove unnecessary Type annotations
kienstra Jan 15, 2023
d8dcaee
In README.md, alphabetize the props
kienstra Jan 15, 2023
5e270a1
Add a Required annotation to the variant prop
kienstra Jan 15, 2023
8ce4937
Add a JS DocBlock to Button
kienstra Jan 15, 2023
fd4144a
Add a CHANGELOG entry
kienstra Jan 15, 2023
916f9ac
Commit Lena's suggestion: Update packages/components/src/button/types.ts
kienstra Jan 21, 2023
3632a42
Commit Lena's suggestion: Update packages/components/src/button/depre…
kienstra Jan 21, 2023
309db5c
Commit Lena's suggestion: Update packages/components/src/button/index…
kienstra Jan 21, 2023
31cbdab
Apply Lena's suggestion to change 'div' to 'button'
kienstra Jan 22, 2023
2ca61e5
Remvoe needless type DisabledEvent
kienstra Jan 22, 2023
be1359d
Remove needless argTypes that Storybook will infer from types
kienstra Jan 22, 2023
35f8efb
Restore a deleted test, thanks to Lena's idea
kienstra Jan 22, 2023
b84337d
Don't expect the console to error
kienstra Jan 22, 2023
14e1137
Update tooltipPosition in README.md
kienstra Jan 22, 2023
12fa70f
Merge branch 'trunk' into update/button-ts
kienstra Jan 22, 2023
a38c275
Move the CHANGELOG entry to Unreleased
kienstra Jan 22, 2023
cea1f6b
Cherry-pick @mirka 's commit to make Button types specific to a or b…
kienstra Jan 23, 2023
ccd87bc
Commit Lena's static tests verbatim
kienstra Jan 23, 2023
561d6aa
Fix a failed test run that I caused
kienstra Jan 23, 2023
3892543
Rename CommonButtonProps to BaseButtonProps
kienstra Jan 23, 2023
01f3c9c
Merge in trunk, resolve conflict
kienstra Jan 23, 2023
07b7002
Commit Lena's suggestion: Update packages/components/src/button/stori…
kienstra Jan 23, 2023
dfef117
Commit Lena's diff for static typing test
kienstra Jan 24, 2023
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
Prev Previous commit
Next Next commit
Simplify the tag container
  • Loading branch information
kienstra committed Jan 9, 2023
commit 718959ceed41f93e8a008d316f8f7fed910e2ffa
6 changes: 6 additions & 0 deletions packages/components/src/button/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,12 @@ Whether the button is focused.
- Type: `Boolean`
- Required: No

#### `onMouseDown`: ( event: MouseEvent< HTMLElement > ) => void;`

Called when the mouse is down.

- Required: No

#### target

If provided with `href`, sets the `target` attribute to the `a`.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
// @ts-nocheck
/**
* External dependencies
*/
import type { ForwardedRef } from 'react';

/**
* WordPress dependencies
*/
Expand All @@ -8,9 +12,23 @@ import { forwardRef } from '@wordpress/element';
/**
* Internal dependencies
*/
import Button from '../button';
import Button from '.';
import type { ButtonProps, DeprecatedIconButtonProps, TagName } from './types';
import type { WordPressComponentProps } from '../ui/context';

function IconButton( { labelPosition, size, tooltip, label, ...props }, ref ) {
function UnforwardedIconButton(
{
labelPosition,
size,
tooltip,
label,
...props
}: WordPressComponentProps<
ButtonProps & DeprecatedIconButtonProps,
TagName
>,
kienstra marked this conversation as resolved.
Show resolved Hide resolved
ref: ForwardedRef< any >
) {
deprecated( 'wp.components.IconButton', {
since: '5.4',
alternative: 'wp.components.Button',
Expand All @@ -29,4 +47,4 @@ function IconButton( { labelPosition, size, tooltip, label, ...props }, ref ) {
);
}

export default forwardRef( IconButton );
export default forwardRef( UnforwardedIconButton );
38 changes: 21 additions & 17 deletions packages/components/src/button/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@
* External dependencies
*/
import classnames from 'classnames';
import type { ForwardedRef, HTMLProps, ReactNode } from 'react';
import type {
ForwardedRef,
HTMLProps,
MouseEvent,
ReactNode,
} from 'react';

/**
* WordPress dependencies
Expand All @@ -21,11 +26,11 @@ import type { WordPressComponentProps } from '../ui/context';
import type {
ButtonProps,
DeprecatedButtonProps,
DisabledEvents,
DisabledEvent,
TagName,
} from './types';

const disabledEventsOnDisabledButton: Array< keyof DisabledEvents > = [
const disabledEventsOnDisabledButton: Array< DisabledEvent > = [
'onMouseDown',
'onClick',
];
Expand Down Expand Up @@ -142,9 +147,13 @@ export function UnforwardedButton(
tagProps[ 'aria-disabled' ] = true;

for ( const disabledEvent of disabledEventsOnDisabledButton ) {
additionalProps[ disabledEvent ] = ( event ) => {
event.stopPropagation();
event.preventDefault();
additionalProps[ disabledEvent ] = (
event: MouseEvent< HTMLElement >
) => {
if ( event ) {
event.stopPropagation();
event.preventDefault();
}
};
}
}
Expand Down Expand Up @@ -174,21 +183,16 @@ export function UnforwardedButton(
className: classes,
'aria-label': additionalProps[ 'aria-label' ] || label,
'aria-describedby': describedById,
ref: ref,
ref,
};

const TagElement = ( {
children: tagChildren,
...otherProps
}: { children: ReactNode } & typeof tagElementProps ) => {
const TagElement = ( tagProps: { children: ReactNode } & typeof tagElementProps ) => {
return Tag === 'a' ? (
<a { ...( otherProps as WordPressComponentProps< {}, 'a' > ) }>
{ tagChildren }
</a>
<Tag { ...( tagProps as WordPressComponentProps< {}, 'a' > ) } />
) : (
<button { ...( otherProps as WordPressComponentProps< {}, 'button' > ) }>
{ tagChildren }
</button>
<Tag
{ ...( tagProps as WordPressComponentProps< {}, 'button' > ) }
/>
);
};

Expand Down
25 changes: 17 additions & 8 deletions packages/components/src/button/types.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import type { KeyboardEvent, MouseEvent, ReactNode } from 'react';
import type { MouseEventHandler, ReactNode } from 'react';

/**
* Internal dependencies
Expand All @@ -26,7 +26,7 @@ export type ButtonProps = {
* `'tertiary'` (the text-based button styles), and
* `'link'` (the link button styles).
*/
variant: 'primary' | 'secondary' | 'tertiary' | 'link';
variant?: 'primary' | 'secondary' | 'tertiary' | 'link';
/**
* Renders a red text-based button style to indicate destructive behavior.
*/
Expand Down Expand Up @@ -68,13 +68,17 @@ export type ButtonProps = {
* Please refer to the Icon component for more details regarding
* the default value of its `size` prop.
*/
iconSize?: IconProps< unknown > [ 'size' ]
iconSize?: IconProps< unknown >[ 'size' ];
/**
* If provided with `icon`, sets the position of icon relative to the `text`.
*
* @default 'left'
*/
iconPosition?: 'left' | 'right';
/**
* Called when the mouse is down.
*/
onMouseDown?: MouseEventHandler< HTMLElement >;
/**
* If provided, displays the given text inside the button. If the button contains children elements, the text is displayed before them.
*/
Expand Down Expand Up @@ -102,19 +106,24 @@ export type ButtonProps = {
* Whether this is focusable.
*/
__experimentalIsFocusable?: boolean;
}
};

export type DeprecatedButtonProps = {
isDefault?: boolean;
isPrimary?: boolean;
isSecondary?: boolean;
isTertiary?: boolean;
isLink?: boolean;
}
};

export type DisabledEvents = {
onMouseDown: MouseEvent;
onClick: KeyboardEvent;
export type DeprecatedIconButtonProps = {
labelPosition: ButtonProps[ 'tooltipPosition' ];
size: ButtonProps[ 'iconSize' ];
showTooltip?: boolean;
label: ButtonProps[ 'label' ];
tooltip: ButtonProps[ 'label' ];
};

export type DisabledEvent = 'onMouseDown' | 'onClick';

export type TagName = 'a' | 'button';