-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
ResizableBox: Remove knobs in stories #46774
Conversation
const ResizableBox = forwardRef( UnforwardedResizableBox ); | ||
|
||
export default ResizableBox; |
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.
Tweaked the component export so we can use the TypeScript data in the Storybook props table.
height: height + delta.height, | ||
width: width + delta.width, |
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.
The parseInt()
that used to be here turned out to be unnecessary. Thanks TypeScript 🙏
}; | ||
|
||
export const _default = () => { | ||
const content = text( 'Example: Content', 'Resize' ); |
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.
content
is not an actual component prop.
Size Change: -384 B (0%) Total Size: 1.32 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.
__experimentalShowTooltip: { control: 'boolean' }, | ||
minHeight: { control: 'number' }, | ||
minWidth: { control: 'number' }, | ||
onResizeStop: { action: 'onResizeStop' }, | ||
showHandle: { control: 'boolean' }, |
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.
Do you know why are these props not shown by default in the Storybook docs?
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.
Good question. I tweaked the export so at least our first-party types are visible ✅
However, all the props coming from the upstream re-resizable
are not shown at all 🤔 So I think what will happen is that in the full TypeScript migration, we'll decide which ones of the upstream props are noteworthy, and add our own typings/descriptions for them so they'll be shown in the Storybook props table. For all the other upstream props, the main component description will nudge people toward the re-resizable
docs as written in the readme.
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 plan
✅ Done in 7cd7eab |
Flaky tests detected in 4cc4395. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/3857597185
|
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.
One last smaller comment and then you can go ahead and merge this one too 🚀
const meta: ComponentMeta< typeof ResizableBox > = { | ||
title: 'Components/ResizableBox', | ||
component: ResizableBox, | ||
argTypes: { |
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.
We should probably hide children
's control, as it currently shows a react object that can't really be edited
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.
True 👍
Part of #35665
What?
Remove the Knobs add-on from
ResizableBox
stories.Why?
This is now a blocker for supporting npm 8 (#46443).
How?
Just a quick, minimal refactor.
Testing Instructions
npm run storybook:dev
ResizableBox
component.