You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@echarts.apache.org by "plainheart (via GitHub)" <gi...@apache.org> on 2023/08/07 04:03:06 UTC

[GitHub] [echarts] plainheart commented on a diff in pull request #18436: feat(tooltip): add `appendTo` option to allow customized tooltip container

plainheart commented on code in PR #18436:
URL: https://github.com/apache/echarts/pull/18436#discussion_r1285355450


##########
src/component/tooltip/TooltipHTMLContent.ts:
##########
@@ -241,23 +247,25 @@ function makeStyleCoord(out: number[], zr: ZRenderType, appendToBody: boolean, z
 
 interface TooltipContentOption {
     /**
-     * `false`: the DOM element will be inside the container. Default value.
-     * `true`: the DOM element will be appended to HTML body, which avoid
-     *  some overflow clip but intrude outside of the container.
+     * Choose a DOM element which the tooltip element will be located in order to
+     * avoid some overflow clip but intrude outside of the container.
+     *
+     * this config can be either a DomElement, a function to choose a element
+     * or a selector string used by query delector to local a element
      */
-    appendToBody: boolean
+    appendTo: ((chartContainer?: HTMLElement) => HTMLElement | undefined | null) | HTMLElement | string
 }
 
 class TooltipHTMLContent {
 
     el: HTMLDivElement;
 
-    private _container: HTMLElement;
+    private _api: ExtensionAPI;
+    private _container: HTMLElement | undefined | null;

Review Comment:
   I would suggest clearing the `_container` & `el` in the `dispose` function.



##########
src/component/tooltip/TooltipModel.ts:
##########
@@ -61,11 +61,18 @@ export interface TooltipOption extends CommonTooltipOption<TopLevelFormatterPara
     renderMode?: 'auto' | TooltipRenderMode   // TODO richText renamed canvas?
 
     /**
-     * If append popup dom to document.body
-     * Only available when renderMode is html
+     * @deprecated
+     * use appendTo: 'body' instead
      */
     appendToBody?: boolean
 
+    /**
+     * If append popup dom to some other dom element
+     * Only available when renderMode is html
+     */
+    appendTo?: ((chartContainer?: HTMLElement) => HTMLElement | undefined | null) | string | HTMLElement

Review Comment:
   It seems the callback parameter `chartContainer` will never be undefined. Maybe the `?` can be removed.



-- 
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: commits-unsubscribe@echarts.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org