Skip to content

Fix/map bug#4565

Merged
skie1997 merged 5 commits intodevelopfrom
fix/map-bug
Apr 22, 2026
Merged

Fix/map bug#4565
skie1997 merged 5 commits intodevelopfrom
fix/map-bug

Conversation

@skie1997
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 #4549
close #4547

🔗 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

@xuefei1313
Copy link
Copy Markdown
Contributor

Thanks @skie1997 for the fix!

Summary

  • VChart.reLayout() now guards against invalid state and tries to trigger a full evaluate cycle again (plus a unit test 👍).
  • Roam handling for map / scatter-map series is adjusted to address “label not following on drag/zoom”.

Review comments / suggestions

1) VChart.reLayout() – avoid @ts-ignore and call onEvaluateEnd with a typed arg

In packages/vchart/src/core/vchart.ts, onEvaluateEnd is part of IChart and expects IChartEvaluateOption.
That interface is currently empty, so we can keep type-safety by calling it with {} and remove the @ts-ignore:

this._chart.onEvaluateEnd({});

Also consider guarding the method at runtime if there are any non-BaseChart implementations (just in case).

2) Map series label update – please double-check behavior after removing label transform

In packages/vchart/src/series/map/map.ts, the old code:

  • re-rendered the label component,
  • forced an evaluate,
  • and additionally applied the same postMatrix transform to the label group.

The new code only calls renderInner().

If the root cause was double-applying the roam transform (projection already updated, while label group was also scaled/translated), then removing the extra label transform makes sense.

However, I’m slightly concerned about:

  • calling renderInner() without checking the method exists (label?.renderInner?.() is safer), and
  • dropping the explicit evaluate(null, null) step — if renderInner() doesn’t guarantee re-evaluation in all cases, labels might still lag behind.

Could you confirm this was tested for both zoom and drag/pan on map?

3) Scatter series roam handling – position update vs label layout

In packages/vchart/src/series/scatter/scatter.ts:

  • _updateSymbolGraphicPosition() now updates only _symbolMark graphics, while the previous implementation updated all marks from getMarksWithoutRoot().
    • If scatter-map has other marks that should also follow roam (e.g. additional symbol marks, background marks), they may become stale.
  • Switching from translateTo(...) to setAttributes({x, y}) can be correct, but please ensure there isn’t any leftover translate/dx/dy transform state on the graphics that could accumulate across interactions.
  • Label handling now applies scale/translate to labelGraphic without re-running label layout.
    • If label placement relies on overlap-avoid/collision or any evaluation step, a pure transform might keep labels in a sub-optimal state.

4) Changesets

  • Both changeset JSON files are missing a trailing newline.
  • For user-facing bug fixes, should type be patch instead of none (depending on your release policy)?
  • Consider merging the two changesets into one, unless the repo expects one-per-issue.

Overall this looks like a good direction and it’s great to see a unit test added for reLayout(). With the small type-safety cleanup + a quick confirmation on map/scatter label behavior, I think it will be in good shape to merge.

@skie1997 skie1997 merged commit db7019b into develop Apr 22, 2026
7 of 10 checks passed
@skie1997 skie1997 deleted the fix/map-bug branch April 22, 2026 05:54
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] map error when handler drag

2 participants