-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
TextControl: Convert component to TypeScript #40633
Changes from 2 commits
890237f
ef3a12e
fd4032c
e5fe71c
0224055
1667458
e30a8dd
ecb0f67
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import type { ChangeEvent, ForwardedRef } from 'react'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { useInstanceId } from '@wordpress/compose'; | ||
import { forwardRef } from '@wordpress/element'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import BaseControl from '../base-control'; | ||
import type { TextControlProps } from './types'; | ||
|
||
function UnforwardedTextControl( | ||
props: TextControlProps, | ||
ref: ForwardedRef< HTMLInputElement > | ||
) { | ||
const { | ||
label, | ||
hideLabelFromVision, | ||
value, | ||
help, | ||
className, | ||
onChange, | ||
type = 'text', | ||
...additionalProps | ||
} = props; | ||
const instanceId = useInstanceId( TextControl ); | ||
const id = `inspector-text-control-${ instanceId }`; | ||
const onChangeValue = ( event: ChangeEvent< HTMLInputElement > ) => | ||
onChange( event.target.value ); | ||
|
||
return ( | ||
<BaseControl | ||
label={ label } | ||
hideLabelFromVision={ hideLabelFromVision } | ||
id={ id } | ||
help={ help } | ||
className={ className } | ||
> | ||
<input | ||
className="components-text-control__input" | ||
type={ type } | ||
id={ id } | ||
value={ value } | ||
onChange={ onChangeValue } | ||
aria-describedby={ !! help ? id + '__help' : undefined } | ||
ref={ ref } | ||
{ ...additionalProps } | ||
/> | ||
</BaseControl> | ||
); | ||
} | ||
|
||
/** | ||
* TextControl components let users enter and edit text. | ||
* | ||
* | ||
* @example | ||
* ```jsx | ||
* import { TextControl } from '@wordpress/components'; | ||
* import { useState } from '@wordpress/element'; | ||
* | ||
* const MyTextControl = () => { | ||
* const [ className, setClassName ] = useState( '' ); | ||
* | ||
* return ( | ||
* <TextControl | ||
* label="Additional CSS Class" | ||
* value={ className } | ||
* onChange={ ( value ) => setClassName( value ) } | ||
* /> | ||
* ); | ||
* }; | ||
* ``` | ||
*/ | ||
export const TextControl = forwardRef( UnforwardedTextControl ); | ||
|
||
export default TextControl; |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import type { ComponentMeta, ComponentStory } from '@storybook/react'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { useState } from '@wordpress/element'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import TextControl from '..'; | ||
|
||
const meta: ComponentMeta< typeof TextControl > = { | ||
component: TextControl, | ||
title: 'Components/TextControl', | ||
argTypes: { | ||
onChange: { | ||
action: 'onChange', | ||
control: { type: null }, | ||
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. Nit: It looks like we could remove 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. Thanks. I didn't know that 👍 Removed in e30a8dd |
||
}, | ||
value: { | ||
control: { type: null }, | ||
ciampo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, | ||
}, | ||
parameters: { | ||
controls: { | ||
expanded: true, | ||
}, | ||
docs: { source: { state: 'open' } }, | ||
}, | ||
}; | ||
export default meta; | ||
|
||
const DefaultTemplate: ComponentStory< typeof TextControl > = ( { | ||
...args | ||
} ) => { | ||
const [ value, setValue ] = useState( '' ); | ||
|
||
return ( | ||
<TextControl | ||
{ ...args } | ||
value={ value } | ||
onChange={ ( v ) => { | ||
setValue( v ); | ||
} } | ||
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. Apparently, the Action thing (enabled on this line) only works by passing through the const DefaultTemplate: ComponentStory< typeof TextControl > = ( {
+ onChange,
...args
} ) => {
const [ value, setValue ] = useState( '' );
return (
<TextControl
{ ...args }
value={ value }
onChange={ ( v ) => {
setValue( v );
+ onChange( v ); CleanShot.2022-04-29.at.01.46.10-converted.mp4There 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. Thanks. Added in e30a8dd |
||
/> | ||
); | ||
}; | ||
|
||
export const Default: ComponentStory< | ||
typeof TextControl | ||
> = DefaultTemplate.bind( {} ); | ||
Default.args = { | ||
label: 'Label Text', | ||
hideLabelFromVision: false, | ||
help: 'Help text to explain the input.', | ||
type: 'text', | ||
}; | ||
walbo marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import type { HTMLInputTypeAttribute } from 'react'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import type { BaseControlProps } from '../base-control/types'; | ||
import type { WordPressComponentProps } from '../ui/context'; | ||
|
||
export type TextControlProps = WordPressComponentProps< | ||
walbo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Pick< | ||
BaseControlProps, | ||
'className' | 'hideLabelFromVision' | 'help' | 'label' | ||
> & { | ||
/** | ||
* A function that receives the value of the input. | ||
*/ | ||
onChange: ( value: string ) => void; | ||
/** | ||
* The current value of the input. | ||
*/ | ||
value: string | number; | ||
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. If I were to write this component from scratch, I would type Since this component is not experimental (and since the purpose of these TypeScript migrations is to introduce the least amount of changes possible), we may want to keep this type as it currently is, but consider updating it in a follow-up (see also this recent related conversation) 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 agree it should be 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. Let's keep it as Would you be up for that? 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. Yes. Lets do that. I will creeate a follow-up PR once this is merged. |
||
/** | ||
* Type of the input element to render. Defaults to "text". | ||
* | ||
* @default text | ||
walbo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
*/ | ||
type?: HTMLInputTypeAttribute; | ||
}, | ||
'input', | ||
false | ||
>; |
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.
By the way, sorry about ef3a12e! I fixed up the contributing guide in #40682.
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.
No worries :) Great that you updated the guide. Did follow the guide and seems to cover most of the stuff you need.
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.
Oooh, good to hear! 🙌
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.
Thank you for the feedback! And of course do let us know if you feel like there's something missing (or something wrong) in the guide 😄