Use TypeScript and controls for DateTimePicker's stories
#40986
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
noisysocks
changed the title
noisysocks
changed the title
DateTimePicker's stories
noisysocks
added
[Package] Components
|
Size Change: 0 B Total Size: 1.24 MB ℹ️ View Unchanged
|
noisysocks
force-pushed
the
update/use-typescript-and-controls-for-date-time-stories
branch
from
bca2fdb
to
6167a7a
Compare
noisysocks
commented
May 11, 2022
| @@ -21,15 +21,11 @@ export type UpdateOnBlurAsIntegerFieldProps = { | |||
| children?: ReactNode; | |||
| }; | |||
|
|
|||
| export type DateTimeString = string; | |||
|
|
|||
| export type ValidDateTimeInput = Date | string | number | null; | |||
There was a problem hiding this comment.
@ciampo was right that this doesn't work so well with stories. I mean, it works, but it's not very useful for someone reading the story as documentation to just see ValidDateTimeInput.
There was a problem hiding this comment.
Nice work @noisysocks 👍
This is looking good. I did leave a couple of general questions and suggestions but feel free to take those with a grain of salt.
✅ Type changes make sense, no typing errors
✅ Storybook examples are consistent between trunk and this PR
✅ Storybook examples function as expected
noisysocks
force-pushed
the
update/use-typescript-and-controls-for-date-time-stories
branch
2 times, most recently
from
b467327
to
9a6f84b
Compare
|
Thanks @aaronrobertshaw! |
There was a problem hiding this comment.
Thanks for continuing to refine this @noisysocks 👍
After re-testing the component in the Storybook I think we should add some basic change handling like the example conversion of the UnitControl stories in the guidelines.
It makes the examples much more interactive.
Something like the following rough patch for the DateTime might work. What do you think?
Click for example
diff --git a/packages/components/src/date-time/stories/index.tsx b/packages/components/src/date-time/stories/index.tsx
index 7983b31701..6d9d9a735f 100644
--- a/packages/components/src/date-time/stories/index.tsx
+++ b/packages/components/src/date-time/stories/index.tsx
@@ -3,6 +3,11 @@
*/
import type { ComponentMeta, ComponentStory } from '@storybook/react';
+/**
+ * WordPress dependencies
+ */
+import { useState } from '@wordpress/element';
+
/**
* Internal dependencies
*/
@@ -14,6 +19,10 @@ const meta: ComponentMeta< typeof DateTimePicker > = {
component: DateTimePicker,
argTypes: {
currentDate: { control: 'date' },
+ onChange: {
+ action: 'onChange',
+ control: { type: null },
+ },
},
parameters: {
controls: { expanded: true },
@@ -22,9 +31,22 @@ const meta: ComponentMeta< typeof DateTimePicker > = {
};
export default meta;
-const Template: ComponentStory< typeof DateTimePicker > = ( args ) => (
- <DateTimePicker { ...args } />
-);
+const Template: ComponentStory< typeof DateTimePicker > = ( {
+ onChange,
+ ...args
+} ) => {
+ const [ value, setValue ] = useState< string | null >();
+ return (
+ <DateTimePicker
+ { ...args }
+ currentDate={ value }
+ onChange={ ( v ) => {
+ setValue( v );
+ onChange?.( v );
+ } }
+ />
+ );
+};
export const Default: ComponentStory< typeof DateTimePicker > = Template.bind(
{}
|
Good suggestion @aaronrobertshaw. I've done that to all three stories. |
noisysocks
force-pushed
the
update/use-typescript-and-controls-for-date-time-stories
branch
from
85859e3
to
2c98110
Compare
noisysocks
deleted the
update/use-typescript-and-controls-for-date-time-stories
branch
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Converts the stories for
DateTimePicker,DatePickerandTimePickerto TypeScript and rewrites them to use controls not knobs.Why?
Continues from #40775 which converted
DateTimePickeritself to TypeScript.See #35665 for why we're switching from knobs to controls.
How?
I pieced most of this together by reading the controls documentation, the package's
CONTRIBUTING.mdguide, and copying what we've done forInputControl.Testing Instructions
npm run storybook:dev.DateTimePicker,DatePickerandTimePickerstories.