RangeControl: Refactor stories to use Controls #39357
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
mirka
added
[Package] Components
mirka
commented
Mar 10, 2022
Comment on lines
29
to
39
| color: { control: { type: 'color' } }, | ||
| disabled: { control: { type: 'boolean' } }, | ||
| help: { control: { type: 'text' } }, | ||
| marks: { control: { type: 'object' } }, | ||
| min: { control: { type: 'number' } }, | ||
| max: { control: { type: 'number' } }, | ||
| railColor: { control: { type: 'color' } }, | ||
| showTooltip: { control: { type: 'boolean' } }, | ||
| step: { control: { type: 'number' } }, | ||
| trackColor: { control: { type: 'color' } }, | ||
| withInputField: { control: { type: 'boolean' } }, |
There was a problem hiding this comment.
As always, most of these lines will be unnecessary when the TypeScript conversion is done.
|
Size Change: +2.05 kB (0%) Total Size: 1.21 MB
ℹ️ View Unchanged
|
mirka
force-pushed
the
storybook/refactor-range-control
branch
from
1cc44ef to
6a4f4bb
Compare
ciampo
reviewed
Mar 17, 2022
| * Setting the `step` prop to `"any"` will allow users to select non-integer values. | ||
| * This also overrides both `withInputField` and `showTooltip` props to `false`. | ||
| */ | ||
| export const WithAnyStep = ( props ) => { |
There was a problem hiding this comment.
Should we use RangeControlWithState instead? (in that case, the additional initialValue prop would actually come in handy)
There was a problem hiding this comment.
I kind of thought it was useful to have the current value showing for this example, otherwise it's impossible to see that it's a float.
There was a problem hiding this comment.
That is a good point. I had forgotten about this 😅 Let's leave it as-is
ciampo
approved these changes
Mar 18, 2022
stokesman
approved these changes
Mar 21, 2022
|
Maybe this issue isn't the greatest place to bring it up but I'd love to know if Ladle is on the your radars. I've only just seen it and hope it might be something we could consider. I've long had grief with Storybook for how slow it is. |
It wasn't! Good to see some healthy competition going on, especially on the performance front. Ladle in its current state won't quite work for us since we're leaning into the auto-generated docs angle (for example here) as we do more TypeScript migrations. One big bottleneck for me is the package building on the Gutenberg side. The Lines 292 to 293 in 8abfa33 I'm hoping that Story Store v7 might give us a perf boost though. |
|
Thanks for the tip on Storybook and your quick take on Ladle. Support for docs is said to have been started tajo/ladle#58 (comment) so we'll see. |
jostnes
pushed a commit
to jostnes/gutenberg
that referenced
this pull request
Mar 23, 2022
* RangeControl: Refactor stories to use Controls * Remove unused `initialValue` * Add control for `initialPosition`
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.
Part of #35665
In preparation for #38720
What?
Reworks the Storybook stories for
RangeControlto use Controls instead of Knobs.Notable changes
Multiplestory (I don't think it currently has a clear purpose)Why?
Better docs, and easier composition of stories.
Testing Instructions
npm run storybook:devand see the Docs tab for theRangeControlcomponent. The reworked stories should be understandable, and capture the intent of the original stories.