Skip to content

fix: update VennSeries to handle empty keys and filter valid legend keys - #4000

Open
afkdsghk211331 wants to merge 6 commits into
VisActor:developfrom
afkdsghk211331:feat/venn
Open

fix: update VennSeries to handle empty keys and filter valid legend keys#4000
afkdsghk211331 wants to merge 6 commits into
VisActor:developfrom
afkdsghk211331:feat/venn

Conversation

@afkdsghk211331

Copy link
Copy Markdown
Contributor

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Refactoring
  • Update dependency
  • Code style optimization
  • Test Case
  • Branch merge
  • Release
  • Site / documentation update
  • Demo update
  • Workflow
  • Other (about what?)

🔗 Related issue link

#2847

🔗 Related PR link

VisActor/VUtil#239

🐞 Bugserver case id

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English
🇨🇳 Chinese

☑️ Self-Check before Merge

⚠️ Please check all items below before requesting a reviewing. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

🚀 Summary

This pull request refines the logic for handling selected keys in the VennSeries class to improve filtering and ensure proper handling of empty keys. The most important changes focus on enhancing the filtering process and maintaining consistency in the selected keys array.

Improvements to key filtering and handling:

  • Introduced a new emptyKey constant and added logic to check for and handle empty keys ('') in the selectedKeys array. This ensures that empty keys are preserved when appropriate.
  • Refined the filtering process by separating non-empty keys from the selectedKeys array and using only valid keys from originalLegendKeys for further operations. This avoids processing invalid or empty keys.
  • Updated the logic for removing derived keys from the selectedKeys array to operate on the filtered nonEmpty keys, improving clarity and correctness.

🔍 Walkthrough

copilot:walkthrough

@afkdsghk211331
afkdsghk211331 marked this pull request as ready for review May 28, 2025 12:02
@xile611 xile611 added this to the v2.0.0 milestone May 29, 2025
Comment thread packages/vchart/src/series/venn/venn.ts Outdated
derivedDisableKeys.forEach(key => {
selectedKeys.splice(selectedKeys.indexOf(getVennSeriesDataKey(key)), 1);
});
const emptyKey = 'others';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个最好加一个配置项,有的地方可能要中文,不能写死了

…Key handling in VennChart and related components

@xile611 xile611 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@afkdsghk211331 感谢你补充 Venn 图的空集合支持,以及可配置的 emptySetKey。本次基于当前提交 6bc32a9832c7,在 develop 4a51ea6a20d5 上叠加改动验证。默认名称和中文名称在图例、tooltip 中均能正常生效,常规集合筛选、resize 和数据更新也通过了验证。

合并前还需要修正两处筛选问题,详细复现和建议已分别附在行内评论中:

  1. 只选空集合时重复追加选中项['others'] 变成 ['others', 'others'],并改写调用方数组;在两项图例的场景下,会被后续过滤误判为全选,未选中的 A 仍然显示。中文名称“其他”也能复现。
  2. 普通集合名为 others 时发生回归:即使数据中没有空集合,普通 ['others'] 也被排除出交集依赖检查。取消选中它后,其交集被错误保留,进而触发布局异常。同一数据和操作在未叠加 PR 的 develop 上正常。

本地测试结果为 15 项通过、5 项失败,失败测试对应上述两个根因;另有 1 项 develop 对照测试通过。对于 3 个具名集合、空集合及其交集组成的 8 项图例,也枚举了全部 256 种选中组合,其中仅选空集合的组合出现重复键和入参修改。这些补充测试是在评审用隔离副本中运行的,目前不包含在 PR 中。

麻烦优先修正这两处逻辑,并将针对性回归测试纳入 PR:仅选空集合(默认/中文名称)、入参不被修改、普通集合名为 others 时的交集筛选,以及已有的正常筛选和更新场景。新增的 emptySetKey 也建议补充中文配置说明。

本次先提交 Request changes。感谢你的贡献,修正后可以继续复核合并。

});
}
if (hasEmpty) {
selectedKeys.push(emptyKey);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 请避免仅选空集合时重复追加选中项并修改入参

selectedKeys = ['others'] 时,nonEmpty.length === 0,因此不会执行上面的 selectedKeys = nonEmpty.slice()。这里继续向原数组追加 emptyKey,结果和调用方入参都会变为 ['others', 'others']

可在配置了 categoryField: 'sets'valueField: 'value'seriesField: 'sets' 和可见图例的普通 Venn 图中复现:

// data.values
[
  { sets: [], value: 6 },
  { sets: ['A'], value: 8 }
];

// 渲染后仅选中空集合
const selected = ['others'];
chart.setLegendSelectedDataByIndex(0, selected);

预期仅保留空集合,且 selected 仍为 ['others']。实际选中项和入参都有两个 others,过滤后的数据仍包含 A:discreteLegendFilter 将“选中项数量等于全量图例数量”视为全选,直接返回全部数据。将 emptySetKey 配为“其他”也有同样结果。

麻烦在 Venn 筛选器中始终构造独立的结果数组,确保空集合项最多加入一次,并补充两项图例下的单选回归测试。根因是这里生成了重复的选中项,应在此处修正。


if (nonEmpty.length > 0) {
// 过滤出非空的原始图例键
const validKeys = originalLegendKeys.filter(key => getVennSeriesDataKey(key, this._emptySetKey) !== emptyKey);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 请依据原始集合结构识别空集合,避免误伤普通 others 集合

这里通过名称是否等于 emptyKey 过滤原始图例键,会把合法的普通集合 ['others'] 也当作空集合处理,即使数据中根本没有 sets: []

在配置了 categoryField: 'sets'valueField: 'value'seriesField: 'sets' 和可见图例的普通 Venn 图中,使用以下数据:

[
  { sets: ['others'], value: 8 },
  { sets: ['A'], value: 10 },
  { sets: ['others', 'A'], value: 4 }
]

渲染后取消选中 others,对应公共 API 调用:

chart.setLegendSelectedDataByIndex(0, ['A', 'others,A']);

预期隐藏 others 及依赖它的交集,仅保留 A。当前实现将 ['others']validKeys 中排除,导致交集未被禁用。布局收到缺少组成集合的交集后,在 greedyLayout 抛出 TypeError: Cannot read property 'size' of undefined。同一数据和操作在 develop 4a51ea6a20d5 对照中通过,属于本 PR 引入的回归。

麻烦根据原始 sets 是否为空来识别空集合,并区分空集合身份与显示名称,避免把普通集合名 others 隐式变为保留名称。请补充这一场景的回归测试。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants