search

storybookjs / design-system

🗃 Storybook Design System
https://master--5ccbc373887ca40020446347.chromatic.com/
1.91k stars 585 forks source link

Improve tooltip stories #242

Closed domyen closed 3 years ago

domyen commented 3 years ago

The tooltip stories should use buttons not divs.

kylesuss commented 3 years ago

There is some logic to this, I believe. A little while back, Jimmy updated WithTooltip to render a button already for accessibility reasons. You can see that play out here:

https://github.com/storybookjs/design-system/blob/1957d198f335706b02c5af82ff5d9ed5a20c9d5a/src/components/tooltip/WithTooltip.js#L10

https://github.com/storybookjs/design-system/blob/1957d198f335706b02c5af82ff5d9ed5a20c9d5a/src/components/tooltip/WithTooltip.js#L43

The general idea was that you should be able to tab to the element and hit enter to open/close the tooltip. Unless you explicitly pass in a "tagName" to override the default rendering of a button, the component already renders a button for you. With that in mind, it doesn't really make sense to me to render another button as a child of that. Thoughts?

domyen commented 3 years ago

That's helpful context! Closing now.