Skip to content

fix(radar-transformer): fix theme config priority for radar - #3992

Open
Liu-Hong-Wei wants to merge 1 commit into
VisActor:developfrom
Liu-Hong-Wei:fix/radar-label-visible
Open

Liu-Hong-Wei wants to merge 1 commit into
VisActor:developfrom
Liu-Hong-Wei:fix/radar-label-visible

Conversation

@Liu-Hong-Wei

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

#3371

🔗 Related PR link

🐞 Bugserver case id

💡 Background and solution

背景: radius 轴 label 等属性默认不显示, 配置个空对象后才能使用 theme 设置的值

解决方案: 如果 theme 中设置了 visible, 优先使用 theme 的值, 否则默认不显示

📝 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

copilot:summary

🔍 Walkthrough

copilot:walkthrough

@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.

@Liu-Hong-Wei 目前建议修改后再合并。已将本 PR 的提交 f309159 应用到当前 develop 4a51ea6,使用真实 VChart 实例做了渲染验证。

阻塞问题是主题读取接口已经变化:当前 getTheme() 无参返回 undefined,所以本 PR 仍会把半径轴的 label/domainLine/tick.visible 写为 false#3371 的主题配置依然不生效。具体位置见行内意见。

另一个原有需求也尚未完成:默认主题下,省略 label 时隐藏,配置 label: {} 时显示。这不是本 PR 新增的回归,但 issue 已明确要求两者保持一致:#3371 (comment) 。建议在既有主题与 spec 合并路径中统一默认值处理,避免是否传入空对象改变默认可见性。

验证结果:

  • 注册主题并设置三项 visible: true,省略对应轴配置:预期 [true, true, true],实际 [false, false, false]
  • 默认主题下比较省略 labellabel: {}:实际分别为隐藏、显示。
  • 在隔离目录中仅修正为按路径读取 component.axisRadius 后,主题生效、空对象继承自定义主题、显式 spec 优先、默认隐藏这 4 个对照用例通过;这不代表默认主题下空对象一致性问题已解决。未跑全量测试。

请更新到当前 develop,修正主题读取并统一默认值合并逻辑,补充上述回归测试后再申请 review。

if (!axis[configName]) {
axis[configName] = { visible: false };
// 获取 theme 中的配置
const theme = this._option.getTheme?.();

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.

这里使用了旧版 getTheme() 的无参调用契约。当前 develop 中,VChart.getTheme(...keys)keys.length === 0 时直接返回 undefined,而 chart transformer 收到的正是这个方法。因此 themeConfig 始终为空,下面的 ?? false 会继续覆盖用户主题,本 PR 合入当前 develop 后无法修复 #3371。已用注册主题将 label/domainLine/tick.visible 全部设为 true 的真实渲染用例复现,最终三项仍全部为 false。请使用当前按路径读取主题的接口获取 component.axisRadius(例如 this._option.getTheme?.('component', 'axisRadius')),相应调整后续取值,并添加不依赖 mock 的回归测试。当前接口实现:

protected getTheme = (...keys: string[]): any => {
if (!this._currentTheme || !keys || !keys.length) {
return undefined;
}

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants