Skip to content

fix: prevent tooltip action when there is no ref element#1

Merged
Baael merged 1 commit intomasterfrom
CDT-1382-tooltip-not-disappear-after-scroll
Aug 3, 2020
Merged

fix: prevent tooltip action when there is no ref element#1
Baael merged 1 commit intomasterfrom
CDT-1382-tooltip-not-disappear-after-scroll

Conversation

@Baael
Copy link
Copy Markdown

@Baael Baael commented Aug 3, 2020

Simply check if there is a ref element, to prevent calling getBoundingClientRect on undefined or null.

@Baael Baael requested a review from TheMightyPenguin August 3, 2020 12:14
Copy link
Copy Markdown

@TheMightyPenguin TheMightyPenguin left a comment

Choose a reason for hiding this comment

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

💯

Comment thread src/index.js

// Calculation the position
updatePosition() {
if (!this.tooltipRef) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This means we always need to pass a ref for this to work right?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

react-tooltip must have ref to be able to perform getBoundingClientRect on its node, otherwise we are trying to call getBoundingClientRect on undefined.

@TheMightyPenguin
Copy link
Copy Markdown

@Baael do you think it would be possible to contribute this back to the original library? You mentioned before that there are multiple issues there with this problem, so not sure if they accept this. Also, could you add some more context in the description of the PR opf why is this required? I know you mentioned it before but just so it stays documented!

@Baael
Copy link
Copy Markdown
Author

Baael commented Aug 3, 2020

@Baael do you think it would be possible to contribute this back to the original library? You mentioned before that there are multiple issues there with this problem, so not sure if they accept this. Also, could you add some more context in the description of the PR opf why is this required? I know you mentioned it before but just so it stays documented!

yes, I think that we will make our master to official master. Sure, sorry for lack of info.

@Baael Baael merged commit ad070e4 into master Aug 3, 2020
@delete-merged-branch delete-merged-branch bot deleted the CDT-1382-tooltip-not-disappear-after-scroll branch August 3, 2020 13:00
@Baael Baael restored the CDT-1382-tooltip-not-disappear-after-scroll branch August 3, 2020 13:06
@Baael Baael deleted the CDT-1382-tooltip-not-disappear-after-scroll branch August 3, 2020 13:08
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