fix: remove double rendering

[markov00]

Summary

The chart_resizer component sends the first action on mount and then activate the resize observer that subsequentially sends another chart size update.

The PR fixes this behavior letting the ResizeObserver doing his job.

fix #690

[codecov-commenter]

Codecov Report

Merging #693 into master will increase coverage by 1.99%.
The diff coverage is 78.73%.

@@            Coverage Diff             @@
##           master     #693      +/-   ##
==========================================
+ Coverage   73.13%   75.13%   +1.99%     
==========================================
  Files         271      271              
  Lines        8784     8802      +18     
  Branches     1747     1735      -12     
==========================================
+ Hits         6424     6613     +189     
+ Misses       2309     2132     -177     
- Partials       51       57       +6     

Impacted Files	Coverage Δ
...art_types/goal_chart/layout/viewmodel/viewmodel.ts	3.12% <0.00%> (ø)
.../chart_types/goal_chart/state/selectors/tooltip.ts	41.66% <ø> (ø)
src/chart_types/index.ts	100.00% <ø> (ø)
..._types/partition_chart/layout/circline_geometry.ts	98.43% <0.00%> (ø)
src/chart_types/partition_chart/layout/geometry.ts	77.77% <ø> (ø)
...types/partition_chart/layout/types/config_types.ts	100.00% <ø> (ø)
...es/partition_chart/layout/types/viewmodel_types.ts	83.33% <ø> (ø)
...c/chart_types/partition_chart/layout/utils/math.ts	100.00% <ø> (ø)
...hart_types/partition_chart/layout/utils/measure.ts	100.00% <ø> (ø)
...tion_chart/layout/viewmodel/hierarchy_of_arrays.ts	88.88% <ø> (ø)
... and 338 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5581c3c...588bea8. Read the comment docs.

[monfera]

Thanks @markov00 the PR is effective in that there's no double rendering, I checked it in storybook mode and playground mode.

As a side effect, the way eg. the viewModel is invoked in partition charts (and maybe in Cartesians) seems to have changed a bit, at least when tested via the playground:

1. viewmodel generation (viewModel.ts main function) is invoked twice (tbh not sure if master does it or not); the first invocation have zeroish width/height so it bails (and the resulting identity viewmodel - emptiness - causes that there's no rendering)
2. however, now the first invocation not only doesn't have width/height; it also doesn't seem to have the chart spec data

While point 2 is not apparent - because it bails anyway due to zero width/height - I'm mentioning this as you might know of some unintended consequences to this. Also, it's unrelated to the PR but ideally there wouldn't be an initial round of rendering with zero dimensions but I'm not sure if it's accidental and we don't care about the dual pass, or there's some hard async problem for which it's the optimal or developer time efficient solution

Approving the PR as it addresses the issue, and leaving it to your judgment if the apparently different zero-width/height invocation is cause for concern or not; @nickofthyme I see you've made commits in this part of the file, maybe know of something?

[monfera]

Jenkins test this

[monfera]

unrelated but could be changed when this file needs an update, containerRef could be read only

[nickofthyme]

Looks to affect the positioning of annotations

[markov00]

Looks to affect the positioning of annotations

@nickofthyme fixed 2b0c472

[markov00]

Thanks @markov00 the PR is effective in that there's no double rendering, I checked it in storybook mode and playground mode.

As a side effect, the way eg. the viewModel is invoked in partition charts (and maybe in Cartesians) seems to have changed a bit, at least when tested via the playground:

1. viewmodel generation (viewModel.ts main function) is invoked twice (tbh not sure if master does it or not); the first invocation have zeroish width/height so it bails (and the resulting identity viewmodel - emptiness - causes that there's no rendering)
2. however, now the first invocation not only doesn't have width/height; it also doesn't seem to have the chart spec data

While point 2 is not apparent - because it bails anyway due to zero width/height - I'm mentioning this as you might know of some unintended consequences to this. Also, it's unrelated to the PR but ideally there wouldn't be an initial round of rendering with zero dimensions but I'm not sure if it's accidental and we don't care about the dual pass, or there's some hard async problem for which it's the optimal or developer time efficient solution

Approving the PR as it addresses the issue, and leaving it to your judgment if the apparently different zero-width/height invocation is cause for concern or not; @nickofthyme I see you've made commits in this part of the file, maybe know of something?

@monfera fixed here 72b6c36
I've basically added the check on the size of the parent before doing any computation

[nickofthyme]

LGTM, seems like the connection between the legend unit test needs to be updated. But the legend stories appear to render fine.

[nickofthyme]

Code changes look good to me. Tested double render via redux and Chart#render, both show single call. Tested legend changes, all looks unaffected.

[monfera]

Great work, I also see a single rendering - and a single change that doubles performance across the board 🎉

[markov00]

🎉 This PR is included in version 19.5.0 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀