-
Notifications
You must be signed in to change notification settings - Fork 88
Added tooltip prop to ActionButton.Item #3
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
base: master
Are you sure you want to change the base?
Conversation
Thanks @Ajackster! I have added some comments to be addressed before I can merge. Also, can you document your change in the Readme? |
Hey @geremih I'm not sure what comments you are talking about. Happy to fix or improve anything though! |
@@ -181,6 +181,9 @@ export default class ActionButton extends Component { | |||
|
|||
return ( | |||
React.Children.map(this.props.children, (button, index) => { | |||
const length = this.props.children.length; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is also a position
prop for ActionButton. Shouldn't this also cover the case in which the button is aligned to either of the sides?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ajackster that's what I meant. So if the button is positioned to the right, no button item should have the tooltip to the left side,? So in that case, one way to do that would be const middle = (index == length -1)
and the rest are aligned to the right. I saw that in the new screenshot you posted, all tooltips are at the top of the button item. Is that something you would prefer using, instead of aligning the tooltip with respect to the index?
@geremih Hm I'm not too sure what the
|
Would love to see this added. However, I think the tooltip text will also need a tooltipStyle prop to go with it to style the backdrop of the text and the text color. Or allow the user to supply a custom component rather than limiting them to a string. |
@AbhishekBiswal Haven't had the time to finish this yet, I've been very busy. I plan to finish this very soon. |
@Ajackster any work-in-progress branch I can pull this from? I'm trying to implement this: |
@Ajackster I tried this PR out and I have some feedback if you don't mind.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rknell do you know if my fitness pal is written in React Native? |
No idea! it just looks suspiciously like this project hey
<http://www.snappy-apps.com.au>
…On 4 August 2017 at 07:48, Monte Thakkar ***@***.***> wrote:
@rknell <https://github.com/rknell> do you know if my fitness pal is
written in React Native?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFmlbWrzoPFJhjLaHTGaj52f7Livi6Cgks5sUkA4gaJpZM4LxxtP>
.
|
This pull request adds tooltips to the ActionButtonItem component. It calculates the position of each action button and either places it to the left, right or on top of the action button item.
Here are pictures of what the tooltip looks like with an even and odd set of ActionButtonItems.
@geremih Would you be interested in adding this to your project?