-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat(Tooltip): Tooltip target element option #356
Conversation
Changed the tooltip target property to also allow for DOM elements.
This is the same concern. @nlrowe and I work together. |
src/Tooltip.js
Outdated
getTetherConfig() { | ||
const attachments = getTetherAttachments(this.props.placement); | ||
return { | ||
...defaultTetherConfig, | ||
...attachments, | ||
target: '#' + this.props.target, | ||
target: this.getTarget(), |
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.
I forgot that I used the string selector because I wasn't sure if the target element would be available at the time getTetherConfig is called. Tether also accepts a function as target. This might be safer as:
target: this.getTarget
this._target = this.getTarget()
in componentDidMount
should be fine since it's mounted.
Good callout for |
If we change The check in if (!(this.props.isOpen && this._target)) {
return null;
} Or, we can use the function and it will go get the element on its own. Which would you prefer good sir? |
@nlrowe ah I was hoping the timing/lifecycle would be fine. Good to know, I think we should pass in the function instead. Thanks for working through this! |
Pass getTarget function instead of the target retrieved in componentDidMount as it is not always available at render.
Added the option to pass a DOM element to the tooltip target. The 'getTarget' function was also used for the tether config target to maintain post component mounting behavior. If this is not a concern, it can just reference this._target that is created on mount.
Addresses #337