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

Add support for borders #55

Merged
merged 2 commits into from
Dec 1, 2015
Merged

Add support for borders #55

merged 2 commits into from
Dec 1, 2015

Conversation

ahstro
Copy link
Contributor

@ahstro ahstro commented Nov 27, 2015

This adds the ability to add a one pixel border of the same color as the text by adding the border attribute to the JSX component, like this <ReactTooltip border /> or <ReactTooltip border={true} />.
This also adds the new feature to the example page and tweaks heights accordingly.

dark-top
light-right

Also, I threw a small typo fix in there.

Fixes a typo in the active class in the example caused the
`Light`-button to never stay in the `active` state even though the
tooltip was changed.
This adds the ability to add a one pixel border of the same color as the
text by adding the `border` attribute to the JSX component, like this
`<ReactTooltip border />` or `<ReactTooltip border={true} />`.
This also adds the new feature to the example page and tweaks heights
accordingly.
@wwayne
Copy link
Collaborator

wwayne commented Nov 27, 2015

I used to think users can just use the feature class to achieve this, but looks it would be better to support a border feature because it's really complicated to implement a border...

But I'd like to support in the way of <ReactTooltip border='1px solid #fff' /> that would be easier and flexible to use, so I will do some development before merge this PR or you can keep push your thoughts.

Anyway, thanks 👍

@ahstro
Copy link
Contributor Author

ahstro commented Nov 27, 2015

My original thought was to do it like that as well, but I thought I'd create a basic version first, especially since you don't seem to support custom colors in any way other than custom class.

Borders around speech bubbles (specifically with the little arrow) are quite complicated, so I don't think you could support anything other than solid without making it insanely difficult and complex.

Let me know if you want me to keep working on this or if you want to take it from here? :)

@wwayne
Copy link
Collaborator

wwayne commented Nov 28, 2015

Just checked, yes it's also difficult to set the border-with on the little arrow.
maybe something like <ReactTooltip border='#fff' /> would be better I think

@ahstro
Copy link
Contributor Author

ahstro commented Nov 30, 2015

To me, it would make more sense to just go with these changes now for a minor release and then go for a major one where styling of the entire component is doable without custom CSS and !important, but if you want to I can start working on setting the border color like you mentioned above.

@wwayne
Copy link
Collaborator

wwayne commented Nov 30, 2015

I prefer setting the border color, then I can release after polishing some details.
I can do it if you don't time, anyway, thanks a lot 👍

@ahstro
Copy link
Contributor Author

ahstro commented Nov 30, 2015

Okay, go ahead.
No problem, thank you for a great component :)

wwayne added a commit that referenced this pull request Dec 1, 2015
Add support for borders
@wwayne wwayne merged commit 0430573 into ReactTooltip:master Dec 1, 2015
@ahstro
Copy link
Contributor Author

ahstro commented Dec 1, 2015

Nice! :) Would you mind publishing the new version to NPM?

@wwayne
Copy link
Collaborator

wwayne commented Dec 1, 2015

Just published in npm, there is a bug just being found, so I wanted to fix it before publishing.

@ahstro
Copy link
Contributor Author

ahstro commented Dec 1, 2015

Ah, I see. Thanks :)

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.

2 participants