100pah commented on pull request #12947: URL: https://github.com/apache/incubator-echarts/pull/12947#issuecomment-661650537
@susiwen8 Do you mind upgrade about these below: When performing regression testing on the existing test cases (`test/*tooltip*`) and new test case (`test/new-tooltip.html`), I found some defects: ## Tooltip position issue The position is not appropriate in some case: `test/tooltip-appendToBody.html`  The tooltip is far out of the current chart instance, too high. (That is not about the css 3d) `test/new-tooltip.html`  ## Arrow position issue `test/new-tooltip.html` Use the first demo as an exmaple: Set the `grid.bottom: 0`, and set `tooltip.confine: true`, which confine the tooltip not overflow the DOM of the current chart instance. The arrow becoming wrong.  Aslo, set `grid.right: 10`, and set `tooltip.confine: true`, the arrow is also incorrect.  `tooltip.confine` is useful when there are some `overflow:hidden` setting on the any of the ancestors of the main DOM. So I think it should better to add this test case to `test/new-tooltip.html`. Is indeed an issue that the location of the tooltip body might makes the "center-vertical" arrow impossible to point to the target point. In this case, I think there probably these solutions: (A) do not show arrow. (recommended) Simple but not consistent (but I think that result can be accepted). (B) make a "oblique" arrow. Not easy to implement, especially consider the different customized style of the tooltip. I think not need to bring that complex.  (C) make a "oblique line" instead of arrow. Easier than (B) but still not consistent.  What's your opinion @Wdingding @susiwen8 @pissang @Ovilia ? ## Do not show tooltip when return `null`/`undefined` Previously, then `tooltip.formatter` return `null`/`undefined`, tooltip will not be displayed any more. But currently, it is displayed as:  I think we should better not do that break change. ## Content layout issue In these cases in `test/new-tooltip.html`: The text is not at the `vertical middle` of the tooltip, which is not good.    In this case below, the gap is too small. (The upper texts and the lower texts are from different series, should have a larger gap than the title and data in a single series)  Because that is a "default style" of tooltip, it could be used in most of the product scenario. So it is checked carefully. ( @Wdingding what's your opinion about that? ) ## About the setting to show arrow In the current commit: > When trigger was item and position is top | bottom | left | right, Tooltip will show arrow and it points to current series But I doubt that in most cases, users will not set `tooltip.position` as top | bottom | left | right . Thus in most cases, arrow will not be displayed. @Wdingding @pissang is it OK for that? ---------------------------------------------------------------- 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: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
