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

Remove tooltip default styles #666

Merged
merged 6 commits into from
May 1, 2020

Conversation

dennisja
Copy link
Contributor

@dennisja dennisja commented Apr 25, 2020

💥 Breaking Changes

🚀 Enhancements

  • Enable users to use or remove default tooltip styles

📝 Documentation

  • Update tooltip docs

🐛 Bug Fix

🏠 Internal

Fixes #650

@dennisja dennisja changed the title Remove default styles Remove tooltip default styles Apr 25, 2020
@williaster
Copy link
Collaborator

@dennisja thanks for the PR including tests! 🙌

It differs slightly from the solution @hshoff discussed/proposed in #650, instead of adding withDefaultStyles, can we add unstyled (with a default to = false), and also update the default of style to be = defaultStyles? (along with the export of defaultStyles)

this would support the current behavior + styles by default, but allow you override all of (or a subset of) defaultStyles, or apply no styles whatsoever.

@dennisja
Copy link
Contributor Author

I have changed it to behave like how @hshoff suggested but there is an issue with the implementation. If a user accidentally uses the tooltip like

<Tooltip unstyled style={{...someStylesAUserWantsToApply}} />

The styles will not be applied

@williaster williaster added this to the v0.0.196 milestone May 1, 2020
@williaster
Copy link
Collaborator

@dennisja thanks for this contribution! I think that the case you call out is okay, unstyled seems like it should be self-explanatory and we can revisit if we get negative feedback about this 👍

will try and get this out in the 0.0.196 release today.

@williaster williaster merged commit 5fa4fbf into airbnb:master May 1, 2020
@williaster
Copy link
Collaborator

out in 0.0.196

@hellosmithy
Copy link

Just a note that this is a breaking change for anyone who was passing in a style prop to Tooltip. Might be worth updating the CHANGELOG to reflect that.

@hshoff
Copy link
Member

hshoff commented May 13, 2020

We’ll update the changelog.

For now, work around is to import defaultStyles and extend it through style prop.

<Tooltip style={{ ...defaultStyle, style }} />

@williaster
Copy link
Collaborator

Updated the changelog and included a before/after 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove default styling for Tooltip
4 participants