Skip to content

revert: tooltip logic to 5.2.12 #6776

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 15, 2025
Merged

revert: tooltip logic to 5.2.12 #6776

merged 4 commits into from
Apr 15, 2025

Conversation

BQXBQX
Copy link
Contributor

@BQXBQX BQXBQX commented Apr 14, 2025

Checklist
  • npm test passes
  • commit message follows commit guidelines
image
Description of change

@coveralls
Copy link

Pull Request Test Coverage Report for Build 14443876534

Details

  • 16 of 16 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.04%) to 86.751%

Totals Coverage Status
Change from base Build 14441101903: -0.04%
Covered Lines: 10705
Relevant Lines: 11960

💛 - Coveralls

@hustcc
Copy link
Member

hustcc commented Apr 14, 2025

操作方式是直接按顺序 revert 代码吗?又遇到冲突吗?需要二次检查一下:

  1. 列出最近版本 tooltip 相关的 pr。
  2. 涉及文件和行数。
  3. 看看 revert 之后的代码,相关文件和行数对比,是否内容相同。

@BQXBQX
Copy link
Contributor Author

BQXBQX commented Apr 14, 2025

操作方式是直接按顺序 revert 代码吗?又遇到冲突吗?

是找所有含有 tooltip 关键词的 git commit,遇到了一些冲突,但是基本上都是单测文件夹里面的 index.ts 的导出代码的冲突,都已经解决,单测也检测了一遍,没有报错

@hustcc
Copy link
Member

hustcc commented Apr 14, 2025

我记得我也提了一个 pr,这个 pr 需要回滚吗?不确定是不是之前你的 pr 带来的问题。

@hustcc
Copy link
Member

hustcc commented Apr 14, 2025

把相关的 pr 在描述中加入进去吧,这样可以在信息流中关联起来。

@BQXBQX
Copy link
Contributor Author

BQXBQX commented Apr 14, 2025

回滚背景

因为 tooltip 逻辑的贸然修改,导致了后续代码无法正常迭代,出现了 bug 滚雪球的情况,后续 bugfix 不断的在修复之前的 bugfix 导致的坑。特此 revert 5.2.12 - 5.3.0 期间所有的 tooltip 的相关逻辑的修改,从而重新设计最初 issue 的修复思路,维持代码的可持续维护性。

时间线

  1. fix: display abnormally when seriesTooltip #6708
  • issue 描述:因为 filterValid 导致数据项为 null 的 element 的缺失,tooltip 拾取的准则是就近原则导致 tootip 找到了隔壁的 element 来渲染,导致了错误的 tooltip 内容的出现。
  • 当时的修复方法:对就近原则选出的 element 进行判断是否在当前的 domainX 中,如果在则返回。
  1. fix(tooltip): handle single element case in series tooltip #6717
  • issue 描述: 因为 折线图的所有的 element 公用一条 path 导致了单纯的 key 想等判断其实并不准确,导致了折线图的 tooltip 拾取错误。
  • 当时的修复方法:使用 seriesKey.includes 修复。
  1. fix(tooltip): tooltip with one element #6763
  • issue描述:没有兜底逻辑导致 sortedDomain 出现了 undefined 的情况。
  • 当时的修复方法:scaleX.adjustedRange || scaleX.getOptions().range 使用 scaleX.getOptions().range 进行兜底。
  1. fix: tooltip pickup error when width is not equal #6738
  • issue 描述:不同宽度的 bandwidth 并没有正确根据不同的 key 来进行获取,而是都进行简单的直接获取,导致 tooltip 拾取错误。
  • 当时的修复方法:封装公共逻辑,并根据 key 值准确获取不同 element 的 bandwidth

原因

  • tooltip 相关单测较缺失
  • 首次修复时未考虑更多的边缘情况
  • domainX 的判断也并不准确

后续

  • 需要想出更加完全的修复的方式,确保 tooltip 逻辑的可迭代性

@interstellarmt
Copy link
Contributor

这几个遇到的issue后续可以都加入tooltip的单测,覆盖率提高一点

@hustcc
Copy link
Member

hustcc commented Apr 15, 2025

已经对比 https://github.com/antvis/G2/blob/3373ba65a62921db5f5673bde67b55ab5dc755a5/ 的代码,tooltip.ts 文件完全回滚。

@hustcc hustcc merged commit e51c1ad into antvis:v5 Apr 15, 2025
2 checks passed
@hustcc hustcc changed the title chore: revert tooltip logic to 5.2.12 revert: tooltip logic to 5.2.12 Apr 15, 2025
mingcheng pushed a commit that referenced this pull request Apr 21, 2025
* Revert "fix: tooltip pickup error when width is not equal (#6738)"

This reverts commit 1e73769.

* Revert "fix: tooltip with one element (#6763)"

This reverts commit 0318b9d.

* Revert "fix(tooltip): handle single element case in series tooltip (#6717)"

This reverts commit 17c5c41.

* Revert "fix: display abnormally when seriesTooltip (#6708)"

This reverts commit 4566b64.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants