Add storybook story to the Tooltip component #20322
Conversation
brentswisher
requested review from
ajitbohra,
chrisvanpatten,
jorgefilipecosta and
youknowriad
as code owners
brentswisher
added
[Package] Components
|
Size Change: 0 B Total Size: 864 kB ℹ️ View Unchanged
|
| const tooltipText = text( 'Text', 'More information' ); | ||
| return ( | ||
| <Tooltip text={ tooltipText }> | ||
| <Button isSecondary>Hover for more information</Button> |
There was a problem hiding this comment.
I wonder if we should use a Button in the example or just some random div. The reason is the Button already supports a "tooltip" prop (and label) which should have the same behavior.
There was a problem hiding this comment.
Yeah, that makes sense, should I update the readme too? That's where I usually pull the examples from.
| <Button isSecondary>Hover for more information</Button> | ||
| </Tooltip> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Do you think it would be good to show multiple variations on the same story (top, left, right, bottom) like we do for the "buttons" story. Or we could use a knob for the position.
There was a problem hiding this comment.
Yeah, I started to do that but because storybook renders without any padding, changing the position didn't have any effect. I can add a wrapper element that positions the tooltip in the center, so you could see the difference?
There was a problem hiding this comment.
Sounds like a good plan to me.
brentswisher
force-pushed
the
add/tooltip-storybook
branch
from
89b5ca3 to
9fbd6b7
Compare
|
Screenshot with div instead of a button: I'm working on updating the snapshot that is failing, but for some reason, test-unit isn't finding any tests to run 🤷♂ |
| <Tooltip text={ tooltipText } position={ position }> | ||
| <div | ||
| style={ { | ||
| margin: '100px', |
There was a problem hiding this comment.
Maybe more something like that
| margin: '100px', | |
| margin: '50px auto', | |
| width: '200px', | |
| padding: '20px' |
There was a problem hiding this comment.
Works for me, thanks
| component: Tooltip, | ||
| }; | ||
|
|
||
| export const _default = () => { |
There was a problem hiding this comment.
Maybe we can just remove this one and just keep the "withPosition" one as default?
There was a problem hiding this comment.
No problem, updated
… have a tooltip prop with is the recommended was to add a tooltip to them.
brentswisher
force-pushed
the
add/tooltip-storybook
branch
from
89c560e to
67ae82c
Compare
Description
Add a story for the tooltip component
How has this been tested?
Run storybook and see the new tooltip option under the Components section
Screenshots
Types of changes
New feature (non-breaking change which adds functionality)
Checklist: