fix(tooltip): fix some issues in new tooltip. #13398
Conversation
(1) `transform` may not be working in IE. (add vendor prefixes) (2) disable transition only when tooltip shows arrow. (3) reduce unnecessary imports.
|
Thanks for your contribution! The pull request is marked to be |
| this._show = false; | ||
| this._longHideTimeout = setTimeout(() => this._longHide = true, 500) as any; | ||
| } |
There was a problem hiding this comment.
I think we still need to set display: 'none' when this._longHide = true. I'm not sure if it will cause other issues like cover the graph and cause interaction issues. if only set the opacity to 0.
Also, should the time 500ms plus user set hideDelay?
There was a problem hiding this comment.
I think we still need to set
display: 'none'whenthis._longHide = true. I'm not sure if it will cause other issues like cover the graph and cause interaction issues. if only set the opacity to0.
Perhaps, you are worried that it will cause interaction issues if only set opacity to 0 for opacity: 0 will still respond to DOM events. I suppose this issue could be solved through setting visibility to hidden. It won't respond to DOM events. So it should not bring interaction issues anymore and in the meanwhile, it can keep the fade transition when the tooltip is hidden. See this commit f038591
Also, should the time
500msplus user sethideDelay?
This logic is now in the hide function, hide is called by hideLater, and hideLater has applied the hideDelay.
So I think no need to plus the specified hideDelay here again.
There was a problem hiding this comment.
I think it's a solution I didn't think of. Looks good! Nice work!
|
The result looks good. I've left my comment. |
…to fix/tooltip
…nding to DOM events.
|
Congratulations! Your PR has been merged. Thanks for your contribution! 👍 |
Brief Information
This pull request is in the type of:
What does this PR do?
(1) [fix]
transformmay not be working in IE. (add vendor prefixes)(2) [feat] disable transition after a long-time hide
(3) [feat] use
opacityto fade out but not hide directly and add opacity transition(3) reduce unnecessary imports.
I've made these changes to the new tooltip according to some personal ideas, please feel free to leave your comment.
Thank you!
Fixed issues
N/A.
Details
Before: What was the problem?
After: How is it fixed in this PR?
Usage
Are there any API changes?
Related test cases or examples to use the new APIs
please refer to
test/new-tooltip.htmlOthers
Merging options
Other information