plainheart commented on code in PR #18436:
URL: https://github.com/apache/echarts/pull/18436#discussion_r1155520937
##########
src/component/tooltip/TooltipHTMLContent.ts:
##########
@@ -291,17 +292,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);
+ const appendTo = opt.appendTo;
+ const container: HTMLElement = (
+ isString(appendTo)
+ ? document.querySelector(appendTo)
+ : isDom(appendTo)
+ ? appendTo
+ : isFunction(appendTo) && appendTo(api.getDom())
+ );
Review Comment:
I think the fallback is required. The default container is just
`api.getDom()`. It will be undefined if no fallback here.
##########
src/component/tooltip/TooltipHTMLContent.ts:
##########
@@ -350,7 +361,7 @@ class TooltipHTMLContent {
update(tooltipModel: Model<TooltipOption>) {
// FIXME
// Move this logic to ec main?
- const container = this._container;
+ const container = this._api.getDom();
Review Comment:
Agree with you. The tooltip probably can't position correctly if its
container `position` isn't `relative`, but the user should be responsible for
this risk when they decide to append the tooltip element to a customized
container that is uncontrolled by the chart.
So, maybe we can set the `position` only when the customized container is
not specified?
``` ts
const container = this._container;
if (container === this._api.getDom()) {
// ...
}
```
##########
src/component/tooltip/TooltipHTMLContent.ts:
##########
@@ -212,14 +212,14 @@ function assembleCssText(tooltipModel:
Model<TooltipOption>, enableTransition?:
}
// If not able to make, do not modify the input `out`.
-function makeStyleCoord(out: number[], zr: ZRenderType, appendToBody: boolean,
zrX: number, zrY: number) {
+function makeStyleCoord(out: number[], zr: ZRenderType, container: HTMLElement
| null, zrX: number, zrY: number) {
const zrPainter = zr && zr.painter;
- if (appendToBody) {
+ if (container) {
const zrViewportRoot = zrPainter && zrPainter.getViewportRoot();
if (zrViewportRoot) {
// Some APPs might use scale on body, so we support CSS transform
here.
- transformLocalCoord(out, zrViewportRoot, document.body, zrX, zrY);
+ transformLocalCoord(out, zrViewportRoot, container, zrX, zrY);
Review Comment:
I'm also not sure here. As per the context, it seems to be used to handle
the case where `document.body` applies the CSS transform. @100pah @Ovilia Could
you please provide some information about this?
--
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]
