Skip to content
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

Fix tooltip with bounds #721

Merged
merged 8 commits into from
May 29, 2020
Merged

Conversation

dennisja
Copy link
Contributor

💥 Breaking Changes

🚀 Enhancements

  • Add unstyled prop to TooltipWithbounds

📝 Documentation

  • Update Tooltips documentation

🐛 Bug Fix

  • Make Typescript happy when the unstyled prop is passed to TooltipWithBounds

🏠 Internal

Fixes #691

@dennisja
Copy link
Contributor Author

@williaster What do you think about adding a warning when a user passes both the unstyled and style props to any of the Tooltip components?

@williaster
Copy link
Collaborator

@dennisja sorry for the delay. We don't have a precedent anywhere in vx for issuing warnings like this. I think we should see if users find it confusing/open issues about it, and if so we can worry about it then.

@@ -1,7 +1,25 @@
import { TooltipWithBounds } from '../src';
import React from 'react';
import { mount } from 'enzyme';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally we would use shallow over mount, is that possible?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason using shallow fails. I first suspected the fact that TooltipWithBounds is wrapped by a higher-order component that uses life cycle methods to be the reason but even when I use shallow and enable life cycle methods it still fails.
I have seen an issue pointing out that it might be a problem with HOCs but I'm tired now. I Will read through it tomorrow and see whether I can switch mount to shallow. Its been long ever since I used Enzyme

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@williaster I found a workaround to use shallow over mount. Let me know what you think about it

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is perfect! yeah usually you have to use .dive() to traverse as needed to whatever selection you are looking for. Thanks for making the change! 🙏

@williaster
Copy link
Collaborator

Great work @dennisja , thanks for the bug fix + enhancement!

@williaster williaster merged commit 5fb35e1 into airbnb:master May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TooltipWithBounds types do not have the new unstyled props
2 participants