plainheart commented on a change in pull request #13398:
URL:
https://github.com/apache/incubator-echarts/pull/13398#discussion_r502756112
##########
File path: src/component/tooltip/TooltipHTMLContent.ts
##########
@@ -386,8 +392,9 @@ class TooltipHTMLContent {
}
hide() {
- this.el.style.display = 'none';
+ this.el.style.opacity = '0';
this._show = false;
+ this._longHideTimeout = setTimeout(() => this._longHide = true, 500)
as any;
}
Review 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`.
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. See
this commit f038591383293467900c791865f4cb284c315acf
> Also, should the time `500ms` plus user set `hideDelay`?
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.
##########
File path: src/component/tooltip/TooltipHTMLContent.ts
##########
@@ -386,8 +392,9 @@ class TooltipHTMLContent {
}
hide() {
- this.el.style.display = 'none';
+ this.el.style.opacity = '0';
this._show = false;
+ this._longHideTimeout = setTimeout(() => this._longHide = true, 500)
as any;
}
Review 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`.
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 f038591383293467900c791865f4cb284c315acf
> Also, should the time `500ms` plus user set `hideDelay`?
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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@echarts.apache.org
For additional commands, e-mail: commits-h...@echarts.apache.org
