-
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
TextareaControl: Convert component to TypeScript #41215
Conversation
Size Change: -13 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
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.
Thanks for putting this together @walbo. It's great to see the momentum in converting components to TS being maintained 🙇
✅ No typing errors
✅ Works in both Storybook and editor
I have a few minor nitpicks but all in all this is looking pretty good. Nice work.
In case it helps, here's a diff of the tweaks I mentioned via inline comments.
Click to expand diff for suggested tweaks
diff --git a/packages/components/src/textarea-control/README.md b/packages/components/src/textarea-control/README.md
index 45293758d2..1da2617ef6 100644
--- a/packages/components/src/textarea-control/README.md
+++ b/packages/components/src/textarea-control/README.md
@@ -100,9 +100,9 @@ The set of props accepted by the component will be specified below.
Props not included in this set will be applied to the textarea element.
-#### `label`: `string`
+#### `help`: `string | WPElement`
-If this property is added, a label will be generated using label property as the content.
+If this property is added, a help text will be generated using help property as the content.
- Required: No
@@ -112,12 +112,18 @@ If true, the label will only be visible to screen readers.
- Required: No
-#### `help`: `string | WPElement`
+#### `label`: `string`
-If this property is added, a help text will be generated using help property as the content.
+If this property is added, a label will be generated using label property as the content.
- Required: No
+#### `onChange`: `( value: string ) => void`
+
+A function that receives the new value of the textarea each time it changes.
+
+- Required: Yes
+
#### `rows`: `number`
The number of rows the textarea should contain.
@@ -131,12 +137,6 @@ The current value of the textarea.
- Required: Yes
-#### `onChange`: `( value: string ) => void`
-
-A function that receives the new value of the textarea each time it changes.
-
-- Required: Yes
-
## Related components
- For a field where users only enter one line of text, use the `TextControl` component.
diff --git a/packages/components/src/textarea-control/stories/index.tsx b/packages/components/src/textarea-control/stories/index.tsx
index 6988d1ae9c..b4cb1bcf48 100644
--- a/packages/components/src/textarea-control/stories/index.tsx
+++ b/packages/components/src/textarea-control/stories/index.tsx
@@ -53,12 +53,3 @@ export const Default: ComponentStory< typeof TextareaControl > = Template.bind(
{}
);
Default.args = {};
-
-export const WithLabelAndHelpText: ComponentStory<
- typeof TextareaControl
-> = Template.bind( {} );
-WithLabelAndHelpText.args = {
- ...Default.args,
- label: 'Label Text',
- help: 'Help text to explain the textarea.',
-};
diff --git a/packages/components/src/textarea-control/types.ts b/packages/components/src/textarea-control/types.ts
index 16b6823ea0..9a66164577 100644
--- a/packages/components/src/textarea-control/types.ts
+++ b/packages/components/src/textarea-control/types.ts
@@ -13,7 +13,8 @@ export type TextareaControlProps = Pick<
'hideLabelFromVision' | 'help' | 'label'
> & {
/**
- * A function that receives the value of the textarea.
+ * A function that receives the new value of the textarea each time it
+ * changes.
*/
onChange: ( value: string ) => void;
/**
|
||
export const WithLabelAndHelpText: ComponentStory< | ||
typeof TextareaControl | ||
> = Template.bind( {} ); | ||
WithLabelAndHelpText.args = { | ||
...Default.args, | ||
label: 'Label Text', | ||
help: 'Help text to explain the textarea.', | ||
}; |
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.
export const WithLabelAndHelpText: ComponentStory< | |
typeof TextareaControl | |
> = Template.bind( {} ); | |
WithLabelAndHelpText.args = { | |
...Default.args, | |
label: 'Label Text', | |
help: 'Help text to explain the textarea.', | |
}; |
Given the default story is fully interactive, I don't believe we need the overly simple example adding help and label text.
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.
Applied. I wounder if the default template should set a label
and a help
prop to make the default template align with the usage example?
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.
Sounds like a good idea to me 👍
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.
Hope you don't mind, I took the liberty to add this via f88fc21
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.
Don't mind at all. Thanks for the help @aaronrobertshaw 👍
- Alphabetize the props in the readme - Ensure the comments in the component's types match the readme - Remove storybook story Co-Authored-By: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com>
543b096
to
f88fc21
Compare
@walbo in the hope of saving you a little time, I've tweaked the default template and rebased this to resolve the pending changelog conflict. Once the e2es pass this should be right to go 🚢 |
* TextareaControl: Convert component to TypeScript * Update CHANGELOG.md * Update README.md * Apply feedback from code review - Alphabetize the props in the readme - Ensure the comments in the component's types match the readme - Remove storybook story Co-Authored-By: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> * Make default story template match usage example Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com>
What?
Converts the
TextareaControl
component to TypeScript.Why?
Part of the @wordpress/components's TypeScript migration (#35744).
How?
Refactors the current
TextareaControl
to TypeScript.Testing Instructions
TextareaControl
continues to function as expected