fix(tooltip): 修复 tooltip transition 不生效以及线上兼容问题

[Eve-Sama]

Checklist

• npm test passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

Description of change

为了减少大家的思考复杂度, 在简述部分不多说了, 直接看下方留言的小作文就好.

[Eve-Sama]

如果对这块还是没理解我这么改的原因, 我可以再详细、完整地解释一遍. 可能确实不太好review, 这边历史悠久, 牵涉的东西也不少. 有问题可以留言或者随时钉钉找我.

[Eve-Sama]

目前我在component、G2repo上跑过单测, 没问题. 但我通过yalc打包component给G2使用. 再打包G2给G2Plot使用, 在G2Plot上单测出问题了(因为改法的原因, 这是必然的, 修改单测用例就好了), 但是debug时感觉单测读的包不是yalc的包(因为容器名明显不是我设置的这个). 但是npm start的表现却又是yalc的包. 我也有点懵了.

我能想到的就是先发一版, 然后G2Plot再引入依赖, debug完再提一个针对单测的PR给G2Plot.

或者, 你们有什么更好的办法也可以提出来.

[visiky]

不建议先发版，再测试。
可以考虑下 link 到 G2Plot 或者 G2 上应该都是可行的，然后跑 start 看官网回归

[Eve-Sama]

嗯 我试试

[Eve-Sama]

忘掉之前所有的改法与说明, 以本PR与本段解析为准.

业务背景:

官方之前实现customContent的思路, 在方向上有些不太合适, 使用了replaceChild来实现刷新内容, 而导致transition效果丢失. 原始实现思路如下图所示(绿色框为函数名).

修复思路

那么, 想要实现transition的效果, 最核心的就是保持容器始终是不变的. 因此, 在使用customContent的场景下, 我选择由component创建一个容器, 这个容器是始终保持不变的. 而它的类名, 不可以是g2-tooltip. 因为通过上面的流程图可得知, component最终会遍历整个容器, 子元素中如果存在g2-tooltip, 则也会添加一堆效果. 那么如果我们使用了这个类名, 而用户返回的容器也是这个类名, 那么就会出现2份g2-tooltip, 单单box-shadow的效果就加重了, 与线上效果不符.

考虑到线上用户的情况, 我选择新创建一个类名g2-tooltip-custom. 该类名只负责定位与transition, 不负责任何其他UI效果. 在这样的设计下, 假如用户返回了一个g2-tooltip, 也完全没有关系, 因为我们套上的容器只负责定位与transition. 没有任何视觉效果, 原先是啥样还是啥样. 只是需要注意, 此时的g2-tooltip不应该再设置position与visibility, 因为此时的g2-tooltip已经不是最外层容器了.

如果用户返回的元素类名既不是g2-tooltip也不是其他预设的类名, 则完全没关系. 因为在修复之前, 他应该就是没样式的, 那么现在还是没样式. 如果没有使用customContent, 那么这一切逻辑都不会执行, 最外层容器和之前一样, 还是g2-tooltip. 综上所述, 整体的思路如下图所示.

其实和前面没什么太大的区别, 核心点就是关注如何保持最外层容器不变

单元测试

跑了3个仓库(component, G2, G2Plot)的单测, 发现几个现象.
1. jest并不稳定. 在不修改代码的情况下, 多跑几次单测, 经常出现失败用例前后不一.
2. 在本次修改之前, 也就是目前的线上版本master分支, 就存在单测稳定失败的情况.

这就导致如果我本地单测失败, 有可能是本来就失败的, 有可能是jest不稳定导致的, 也有可能是我的改动引起的场景兼容问题, 还有可能是需要修改单测用例. 为了方便对比, 我在本地, 对3个仓库都设置了对照组, 也就是clone了2份, 一份用于链接到我自己的修改. 另一份则是和线上保持一致. 同时跑单测, 对比结果. 为了方便描述, 我称与线上一致的项目为old, 我本地链接了新修改内容的仓库为new, 旁边的数字为失败单测用例数目. 以下是他们跑单测的情况描述.

component

old: 1
new: 1
备注: 双方失败测试用例均为同一个测试用例

查看失败测试用例

 FAIL  tests/unit/tooltip/html-spec.ts
  ● Test suite failed to run

      ● test tooltip › region limit › update position auto

        expect(received).toBe(expected) // Object.is equality

        Expected: 317
        Received: 410

          261 |
          262 |       bbox = tooltip.getBBox();
        > 263 |       expect(bbox.x).toBe(400 - offset - bbox.width);
              |                      ^
          264 |       expect(bbox.y).toBe(300 - offset - bbox.height);
          265 |
          266 |       tooltip.update({

      at Object.<anonymous> (tests/unit/tooltip/html-spec.ts:263:22)

G2

old: 3
new: 3
备注: 双方失败测试用例均为同样的测试用例

查看失败测试用例

 FAIL  tests/unit/component/axis-line-spec.ts
  ● Test suite failed to run

      ● line position › line position right

        expect(received).toBeGreaterThan(expected)

        Expected: > 500
        Received:   105.9840087890625

          66 |
          67 |     // y 轴在右侧
        > 68 |     expect(y.component.get('start').x).toBeGreaterThan(500);
             |                                        ^
          69 |   });
          70 | });
          71 |

      at Object.<anonymous> (tests/unit/component/axis-line-spec.ts:68:40)

 FAIL  tests/bugs/2433-spec.ts
  ● Test suite failed to run

      ● 2433 › 2433

        expect(received).toBe(expected) // Object.is equality

        Expected: false
        Received: true

          61 |
          62 |     // 没有重叠
        > 63 |     expect(legendBBox.collide(sliderBBox)).toBe(false);
             |                                            ^
          64 |   });
          65 | });
          66 |

      at tests/bugs/2433-spec.ts:63:44
      at step (node_modules/tslib/tslib.js:144:27)
      at Object.next (node_modules/tslib/tslib.js:125:57)
      at fulfilled (node_modules/tslib/tslib.js:115:62)

 FAIL  tests/bugs/2173-spec.ts
  ● Test suite failed to run

      ● #2173 › legend should not be overlap

        expect(received).toBe(expected) // Object.is equality

        Expected: 14
        Received: 11

          47 |   it('legend should not be overlap', () => {
          48 |     const [l1, l2] = chart.getComponents().filter((co) => co.type === COMPONENT_TYPE.LEGEND);
        > 49 |     expect(Math.abs(l1.component.getBBox().y - l2.component.getBBox().y)).toBe(14);
             |                                                                           ^
          50 |   });
          51 | });
          52 |

      at Object.<anonymous> (tests/bugs/2173-spec.ts:49:75)

G2Plot

old: 0
new: 0
备注: 双方单测均全部通过

[Eve-Sama]

我这边要改的, 需要考虑的场景啥的, 都尽可能考虑了, 单测这边也没什么问题, 本地开UI效果也没发现问题, 你们review下, 有问题随时留言 or 钉钉联系.

[xiaoiver]

另外在 debug 过程中我发现 G2 的 Tooltip 在设计上存在一些问题，新版可以加以改进。

从 @antv/component 的 Tooltip 实现上看，它提供了以下生命周期和 API，看得出是遵循了 SOLID 原则的。包括从 html-spec 这样的单测上也能看出原作者期望的使用方式：

• init
• render 首次创建 DOM 结构
• update 后续更新标题、内容
• show 仅控制可见性
• hide 同上
• setLocation 仅控制位置

但到了上层 G2 使用时，

• 生命周期有些混乱 init / render 都是空方法，并没有调用 render，首次渲染也调用了 update
• show / hideTooltip = setLocation + show / hide
• API 命名问题 show/hideTooltip，show / hide 即可

最后到了自定义 Tooltip 内容这块儿，如果是我来设计会 严格限制自定义的范围。自定义内容应该以 插槽 形式嵌入预定容器 .g2-tooltip 内，一方面保证了容器 DOM 结构的稳定（上述 transition 问题），另一方面也不需要在组件内部判断用户传入的到底是容器还是内容（正如各位在目前代码中看到的 replaceChild / appendChild 判断）。值得一提的是，在 Web Components 中也是通过 slot 来限定自定义内容的，换句话说，容器以及容器内我们不希望用户感知的内容对其就是不可见的：

<div class="g2-tooltip" style="...">
  <slot></slot>
</div>

如果容器不可见，那如何改变容器样式呢？我觉得这也是目前支持用户传入用于替换 .g2-tooltip 容器的原因：

<div class="g2-tooltip my-custom-classname" style="...">
  // ...Blah, blah
</div>

但仔细想想，用户想改变的并不是容器的 DOM 结构，他只是想改样式而已。类似的思路在 antd 中随处可见，例如下拉菜单不会允许对于 overlay 结构的任意修改，而是提供 overlayClassName 供用户修改默认样式，这就足够了：https://ant.design/components/dropdown-cn/#API

因此我认为完全没必要支持以下替换容器的写法，而且这也并不灵活，毕竟还得要求用户写上 .g2-tooltip 呢。customContent 应当只接受预定插槽的内容：

// before
customContent: (title: string, data: any[]) => {
  return `
    <div class="g2-tooltip custom-html-tooltip">
      <my-content />
    </div>
  `;
}

// after
tooltipClassName: 'custom-html-tooltip',
customContent: (title: string, data: any[]) => {
  return `
      <my-content />
  `;
}

[Eve-Sama]

你说的没错. 我发现customContent返回的DOM, 必须携带g2-tooltip, 或者自己写死定位. 如 https://g2plot.antv.vision/zh/examples/case/customize#customize-tooltip 不过这一点应该和G2没关系吧. 这应该就是component的问题.

这一点是非常违和的. 可以说, 如果不看用例, 单单只看API, 用户都会觉得我只需要返回一个DOM即可, 渲染是图表库的事情. 根本不会往定位上去想.

但仔细想想，用户想改变的并不是容器的 DOM 结构，他只是想改样式而已, 这说的非常对. 也正因此, 我选择套了一个容器, 这个容器只负责定位. 没有任何样式. 如果用户想要修改样式, 那么他在返回的容器上操作就可以了.

你说的tooltipClassName的设计, 的确不错. 关于overlayClassName的设计我在antd里之前也看到过. 但这并不能解决问题. 这是2个问题, 你说的问题是, 如何控制最外层容器的样式. 而这个bug的问题是, 最外层容器反复被replace而引起的transition不生效.

[xiaoiver]

你说的tooltipClassName的设计, 的确不错. 关于overlayClassName的设计我在antd里之前也看到过. 但这并不能解决问题. 这是2个问题, 你说的问题是, 如何控制最外层容器的样式. 而这个bug的问题是, 最外层容器反复被replace而引起的transition不生效.

是的，所以我认为就不应该出现使用 replaceChild 替换整个儿容器的情况。之前这么做的唯一理由大概就是想控制容器样式，而这一点完全可以通过 tooltipClassName 实现。

[xiaoiver]

然后我本地测试这种互相依赖的项目会使用 yarn，用以验证修改后的效果，yarn link 还是挺好用的。

[Eve-Sama]

我用的yalc, 这个其实没问题. 前文之所以说怀疑没有link成功, 是因为他的一个场景和我的认知冲突了, 先使用默认tooltip, 再手动执行customContent, 而导致初始化时容器还是g2-tooltip. 而后面手动执行后, 拿到的容器就还是g2-tooltip. 而当时我认为存在customContent的话外容器应该是g2-tooltip-custom才对, 所以才怀疑没link成功. 算是他这个单测用例起作用了. 所以后来增加了这个场景的兼容写法.

顺便一提, 像G2这种生态架构, monorepo会不会是更好的选择? 至少能npm run test:all. 现在我只是跑了3个仓库, 但是我并不确定到底有多少仓库依赖了component

[hustcc]

@Eve-Sama 处理的原始 issue 是这个吗？

antvis/G2#3722
antvis/G2Plot#2969

[Eve-Sama]

嗯 是一个东西.

[Eve-Sama]

https://codesandbox.io/s/determined-kapitsa-tu258h?file=/index.ts
刚刚在G2群里有人提了这个bug. 虽然这个bug和我是没关系, 但是不确定这个多tooltip的场景, 是否能够在我的这套解决方案下正常运行. 我晚上看下.

[Eve-Sama]

https://codesandbox.io/s/determined-kapitsa-tu258h?file=/index.ts 刚刚在G2群里有人提了这个bug. 虽然这个bug和我是没关系, 但是不确定这个多tooltip的场景, 是否能够在我的这套解决方案下正常运行. 我晚上看下.

看了下, 没问题. 因为tooltip的容器使用的绝对定位, 所以有多个tooltip, 也不会互相干预的.

[xiaoiver]

已在 G2 中验证，同样可以解决这个问题。
ant-design/ant-design-charts#1508

[visiky]

目前 0.8.32 版本有这个问题，我这边先废弃版本就好。具体修复方案，可以看下，或者考虑外置新的 tooltip 插件，而不是使用 antv/component 了。

• position: 'unset'
  

• 类似这些，都挺好用的：https://atomiks.github.io/tippyjs/v6/getting-started/