plainheart commented on code in PR #18436:
URL: https://github.com/apache/echarts/pull/18436#discussion_r1153993746
##########
src/component/tooltip/TooltipHTMLContent.ts:
##########
@@ -291,14 +291,21 @@ class TooltipHTMLContent {
(el as any).domBelongToZr = true;
this.el = el;
const zr = this._zr = api.getZr();
- const appendToBody = this._appendToBody = opt && opt.appendToBody;
- makeStyleCoord(this._styleCoord, zr, appendToBody, api.getWidth() / 2,
api.getHeight() / 2);
-
- if (appendToBody) {
- document.body.appendChild(el);
+ let customContainer: HTMLElement | null
+ if (opt && opt.appendToBody) {
+ customContainer = this._customContainer = document.body
+ } else if (opt && opt.getAppendElement) {
Review Comment:
> I would suggest providing an option when init:
>
> ```
> echarts.init(dom, theme, {
> tooltipContainer: '#tooltip-container'
> })
> ```
@Ovilia I prefer directly adding it in the `tooltip` option, just like
current `appendToBody`. And `appendToBody` can be replaced by `teleport:
document.body` after it's supported. Also, it can allow the developers to
update the container if they need, which is impossible when specifying in the
`init` option.
As for the naming of tooltip container option, besides `teleport`
(figurative, the same as Vue's teleport), `container` / `tooltipContainer` /
`appendTo` (more intutive) looks good to me.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
