Skip to content

Fix/fix tooltip theme not work#4564

Merged
xuefei1313 merged 2 commits intodevelopfrom
fix/fix-tooltip-theme-not-work
Apr 22, 2026
Merged

Fix/fix tooltip theme not work#4564
xuefei1313 merged 2 commits intodevelopfrom
fix/fix-tooltip-theme-not-work

Conversation

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

close #4550 #4562

🔗 Related PR link

🐞 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

copilot:summary

🔍 Walkthrough

copilot:walkthrough

The radar chart transformer copied a visible:false default into the
series default spec before series themes were merged.

That caused chart-level radar themes to lose area.visible:true whenever
the chart spec provided area config without its own visible flag.

Use the current radar area theme visibility as the series default and
add a regression test for the style-only chart-area case.

Constraint: Chart transformers downcast chart-level radar area config before series theme merge
Rejected: Remove the radar area default entirely | changes no-theme area-style behavior
Confidence: medium
Scope-risk: narrow
Reversibility: clean
Directive: Keep radar chart defaults aligned with later series-theme precedence
Tested: npm test -- --runInBand __tests__/unit/theme/radar.test.ts
Tested: npm test -- --runInBand __tests__/unit/theme/line.test.ts __tests__/unit/theme/radar.test.ts
Tested: npm run compile
Not-tested: Full package eslint clean run; repo has pre-existing lint errors and warnings
Not-tested: Browser runtime smoke for radar theme switching
Common-chart series interactions were merged by region and trigger type only.
That collapsed distinct element-active configs into one trigger and let
series with different trigger events interfere with each other.

Group triggers by full config within a region.
Add regression coverage for mixed trigger setups.
Guard element-active reset when no stated graphics have been recorded yet.

Constraint: Common chart initializes series interactions centrally in BaseChart
Rejected: Keep type-only grouping
Rejected: Special-case element-active | leaves other mixed-trigger cases broken
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Merge interaction triggers only when their effective configs match
Directive: Do not merge by trigger type alone
Tested: npm test -- --runInBand __tests__/unit/core/vchart-event.test.ts
Tested: npm run compile
Not-tested: Full package lint sweep
Not-tested: Repository has pre-existing lint noise outside this change
Related: issue1 common-chart element-active isolation
@xuefei1313
Copy link
Copy Markdown
Contributor Author

Summary

This PR fixes a couple of related theme/interaction edge cases:

  1. Interaction trigger merging in common charts: triggers were previously merged only by regionId + trigger.type, which could accidentally merge different trigger configurations (e.g. element-active with click vs pointerover). This can lead to unexpected active-state / interaction behavior (and indirectly affect tooltip-related interactions).
  2. Radar area theme default: radar series area.visible was hardcoded to false as a default, which could override theme defaults when the chart spec only provides area.style.

Overall the direction looks correct and the added tests are helpful.

Notes on the implementation

  • BaseChart._initInteractions: grouping triggers by isEqual(trigger) (per region) makes sense and should prevent cross-series config pollution. Keeping a separate config snapshot is also a nice touch to avoid later marks mutation affecting comparisons.
  • ElementActive: the statedGraphics?.includes(g) change prevents a potential runtime crash when getStatedGraphics() returns undefined.

Suggestions

  • In _initInteractions, consider comparing only semantic trigger fields (e.g. type, trigger, state, etc.) instead of the whole object, in case the trigger object ever contains ephemeral properties. A stable key (or a pick(...) + isEqual) would reduce the risk of missing merges unintentionally.
  • The .vscode/launch.json changes look unrelated to the bug fix; I’d recommend dropping them or moving them into a separate PR to keep the change focused.
  • The new test getActiveTriggerCount reaches into private _interaction internals. It’s OK for a regression test, but if there’s a small public/testing helper we can use instead, it would make the test less brittle across refactors.

Verification

  • The new unit tests cover both the trigger isolation case and the radar theme behavior well. If the original reported issue was specifically about tooltip theme, it might be worth adding a minimal regression around tooltip styling/config too (so we don’t rely on indirect coverage).

@xuefei1313 xuefei1313 merged commit 851fb71 into develop Apr 22, 2026
6 of 9 checks passed
@xuefei1313 xuefei1313 deleted the fix/fix-tooltip-theme-not-work branch April 22, 2026 02:33
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.

[Bug] 柱图使用 interactions.type = 'element-active' & trigger = 'pointerover' 时会失效

2 participants