Skip to content

fix(flamegraph): solve animation regression#1541

Merged
monfera merged 2 commits intoelastic:masterfrom
monfera:fix-animation-regression
Dec 30, 2021
Merged

fix(flamegraph): solve animation regression#1541
monfera merged 2 commits intoelastic:masterfrom
monfera:fix-animation-regression

Conversation

@monfera
Copy link
Contributor

@monfera monfera commented Dec 29, 2021

Summary

Restored tweening (animation) to flamegraph that stopped working at 6db2677

Details

The work that moved numerous config options into the theme turned the previously mandatory animation property optional. This watered up preexisting TS type checks, as optionals often do, and we don't yet have mid-flight VRT for (currently alpha) animations, so it flew under the radar.

The fix restores the mandatory nature of animation and supplies a default zero duration for it.
In the future it might become part of the theme, would need a convo.

Issues

Closes #1540

Checklist

  • The proper chart type label has been added (e.g. :xy, :partition)
  • The proper feature labels have been added (e.g. :interactions, :axis)
  • The :theme label has been added and the @elastic/eui-design team has been pinged when there are Theme API changes
  • All related issues have been linked (i.e. closes #123, fixes #123)
  • New public API exports have been added to packages/charts/src/index.ts
  • Unit tests have been added or updated to match the most common scenarios
  • The proper documentation and/or storybook story has been added or updated
  • The code has been checked for cross-browser compatibility (Chrome, Firefox, Safari, Edge)
  • Visual changes have been tested with all available themes including dark, light, eui-dark & eui-light

@monfera monfera added :interactions Interactions related issue :partition Partition/PieChart/Donut/Sunburst/Treemap chart related :flamegraph Flame/Icicle chart related issues labels Dec 29, 2021
@monfera monfera marked this pull request as draft December 29, 2021 15:55
@monfera monfera marked this pull request as ready for review December 29, 2021 16:19
pickQuads,
outerRadius,
chartDimensions,
animation,
Copy link
Contributor Author

@monfera monfera Dec 29, 2021

Choose a reason for hiding this comment

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

This line is the actual fix, and the other change (restoring mandatory nature of the property) just ensures it can't get lost accidentally

@monfera monfera requested a review from rshen91 December 29, 2021 17:04
Copy link
Contributor

@rshen91 rshen91 left a comment

Choose a reason for hiding this comment

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

Changes LGTM - tested locally on Chrome

Copy link
Collaborator

@markov00 markov00 left a comment

Choose a reason for hiding this comment

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

LGTM

@monfera monfera merged commit 5ec6037 into elastic:master Dec 30, 2021
nickofthyme pushed a commit that referenced this pull request Jan 5, 2022
# [42.0.0](v41.0.1...v42.0.0) (2022-01-05)

### Bug Fixes

* **flamegraph:** solve animation regression occurring with 6db2677 ([#1541](#1541)) ([5ec6037](5ec6037)), closes [#1540](#1540)
* **heatmap:** render empty state ([#1532](#1532)) ([59002df](59002df))
* **waffle:** fix strange 0 text in legend item extra when label is 0 ([#1538](#1538)) ([72224b9](72224b9))

### Features

* **goal:** add valueFormatter for tooltip ([#1529](#1529)) ([8139973](8139973))
* **heatmap:** add axis titles ([#1503](#1503)) ([a87325d](a87325d))
* **types:** improve generic types in specs, and spec prop types ([#1421](#1421)) ([562929e](562929e))

### BREAKING CHANGES

* **types:** The `xAccessor` and `yAccessor` are now required on all xy chart specs. Stronger typing on `data` prop that may cause type errors when using untyped array (i.e. `const arr: never[] = []`). Other minor type changes related to spec types.
* **heatmap:** The heatmap yAxisLabel.padding style type is changed from Pixel | Partial<Padding> to Pixels | Padding. The heatmap axis labels are now correctly subjected to padding calculations and it will result in a slightly different position of labels.

Co-authored-by: Marco Vettorello <vettorello.marco@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:flamegraph Flame/Icicle chart related issues :interactions Interactions related issue :partition Partition/PieChart/Donut/Sunburst/Treemap chart related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Flamegraph] tweening stopped (regression)

3 participants