Skip to content

Update dependency @elastic/charts to v45 (main)#126729

Merged
markov00 merged 14 commits intomainfrom
renovate/main-@elasticcharts
Mar 16, 2022
Merged

Update dependency @elastic/charts to v45 (main)#126729
markov00 merged 14 commits intomainfrom
renovate/main-@elasticcharts

Conversation

@renovate
Copy link
Contributor

@renovate renovate bot commented Mar 2, 2022 •
edited
Loading

WhiteSource Renovate

This PR contains the following updates:

Package Change Age Adoption Passing Confidence
@elastic/charts 43.1.1 -> 45.0.0 age adoption passing confidence

Release Notes

elastic/elastic-charts

v45.0.0

Compare Source

Features
BREAKING CHANGES
  • styles: chart color and style changes

v44.0.0

Compare Source

Bug Fixes
  • axis: correct tick alignment in ordinal scale with numeric values (#​1609) (915349d)
  • legend: width with duplicate nested pie slice labels (#​1585) (1073231)
  • partition: consider legend extras when computing the legend size (#​1611) (2078f3d)
  • xy: dataIndex keeps original data order on small multiples (#​1597) (9e2566c)
Features
BREAKING CHANGES
  • heatmap: width, align, and baseline style properties are removed from the xAxisLabels and yAxisLabels style of the Heatmap theme.

Co-authored-by: Marco Vettorello vettorello.marco@gmail.com

43.1.1 (2022-01-26)

Bug Fixes

Configuration

📅 Schedule: At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

♻ Rebasing: Renovate will not automatically rebase this PR, because other commits have been found.

🔕 Ignore: Close this PR and you won't be reminded about this update again.

  • If you want to rebase/retry this PR, click this checkbox.

This PR has been generated by WhiteSource Renovate. View repository job log here.

@renovate renovate bot added auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:DataVis v7.17.0 v8.1.0 labels Mar 2, 2022
@renovate renovate bot force-pushed the renovate/main-@elasticcharts branch from 1b2e89e to 1249b7d Compare March 4, 2022 04:28
@renovate renovate bot changed the title Update dependency @elastic/charts to v44 (main) Update dependency @elastic/charts to v45 (main) Mar 4, 2022
@markov00
Copy link
Contributor

markov00 commented Mar 4, 2022 •
edited
Loading

To ML team

The charts changes improved a bit the styles of the heatmap, as shown in the screenshot:

  • there is no more right-overflow
  • Each x axis label is now aligned to the right of the tick (previously it was centered) to reflect the fact that the tick and the label represent the beginning of the bucket.
  • the labels are aligned to the beginning of each bucket and the label overlap now follows a different approach: we hide labels that overlaps with a fixed cadence. This produce ticks at even spaces. We will improve that in the near future [Heatmap] Improve tick cadence with time scale elastic-charts#1606
  • I've applied a small fix to the Annotations chart, vertically aligning the label to the middle of the line
  • I've also fixed the missing padding on the bottom of the tooltip (that was cutting the color bar on the tooltip list)

Screenshot 2022-03-04 at 10 22 13

Additional notes:
I saw that you are using d3to render the annotations chart. If you are ok I can convert it, in a different PR, from d3 to react and SVG with the same result.

A functional test case expected result was fixed: in particular the Overall heatmap line, due to the currently configured props, was configured to not show the x-axis labels, but due to a wrong logic in charts the x-axis was rendered and the test was testing a wrong assumption. Please le me know if we should enable the x-axis on the overall heatmap

@markov00
Copy link
Contributor

markov00 commented Mar 4, 2022 •
edited
Loading

To each pinged team

EUI and elastic-charts are two different projects but they should work in symbiosis within Kibana. In particular, elastic-charts should use the provided EUI theme when used in Kibana (that is aligned with fonts, colors, sizes to the EUI component system) but, unfortunately, this doesn't currently work out of the box.
We have a Kibana plugin called charts that provides an handful set of functionalities to support this and we hope we will improve that soon (more info here).

In this PR we don't want you to force changes to the EUI theme your charts, but I've taken the opportunity of pinging all of you in this new major release of @elastic/charts and to mark every non-EUI-themed chart on the code so we can easily find them later.
In this PR we just want to notify you about that adding a small TODO comment on your code and the following major change: the @elastic/charts version 45 we released a breaking change related to the visualization colors in the out-of-the-box theme: We aligned the bundled set of colors with the one coming from the EUI theme.
The differences are minimal as you can see on these images, but this aligns with the EUI theme.

from
Screenshot 2022-03-03 at 15 04 57
to
Screenshot 2022-03-03 at 15 05 04

All charts that are not using the EUI theme and that are relying on the out-of-the-box provided visualization colors will look slightly different. Please pull down this PR and check locally (with your designers if possible) if you are ok with that see of color changes. Please ping us @elastic/datavis if you disagree with that change and we can provide you with the original set of colors previously used.

This change is another step toward unifying the chart styles across Kibana.

@elastic elastic deleted a comment from kibana-ci Mar 4, 2022
@markov00 markov00 marked this pull request as ready for review March 8, 2022 09:00
@markov00 markov00 requested review from a team as code owners March 8, 2022 09:00
@elasticmachine
Copy link
Contributor

elasticmachine commented Mar 9, 2022

Pinging @elastic/uptime (Team:uptime)

ymao1
ymao1 approved these changes Mar 9, 2022
View reviewed changes
Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

Response Ops changes LGTM

@markov00
Copy link
Contributor

markov00 commented Mar 10, 2022

@markov00 I like this strategy! Is there a link that would be helpful to explain what "use EUI theme" means? Maybe this? I'd love to add that to the comment to help us find our way.

yes @jasonrhodes that's the link to the how to, I will add it on the comments

darnautov
darnautov approved these changes Mar 14, 2022
View reviewed changes
Copy link
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

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

ML changes LGTM, also tested locally

jasonrhodes
jasonrhodes approved these changes Mar 14, 2022
View reviewed changes
Copy link
Member

@jasonrhodes jasonrhodes left a comment

Choose a reason for hiding this comment

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

Comments are a great idea, and the new default colors are fine by me. @katrin-freihofner just making sure you're aware of this change too, but I'm approving on behalf of obs infra UI :)

shahzad31
shahzad31 approved these changes Mar 14, 2022
View reviewed changes
Copy link
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

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

Uptime changes LGTM !!

YulNaumenko
YulNaumenko approved these changes Mar 14, 2022
View reviewed changes
Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM

@markov00
Copy link
Contributor

markov00 commented Mar 15, 2022

@elasticmachine run elasticsearch-ci/docs

@markov00 markov00 enabled auto-merge (squash) March 15, 2022 11:06
@kibana-ci
Copy link

kibana-ci commented Mar 16, 2022

💛 Build succeeded, but was flaky

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
ml 3.5MB 3.5MB +16.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
core 295.1KB 295.3KB +206.0B
kbnUiSharedDeps-npmDll 4.8MB 4.8MB +2.7KB
total +2.9KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@markov00
Copy link
Contributor

markov00 commented Mar 16, 2022

@elasticmachine run elasticsearch-ci/docs

@markov00 markov00 merged commit ffebaed into main Mar 16, 2022
@markov00 markov00 deleted the renovate/main-@elasticcharts branch March 16, 2022 15:47
@kibanamachine
Copy link
Contributor

kibanamachine commented Mar 16, 2022

⚪ Backport skipped

The pull request was not backported as there were no branches to backport to. If this is a mistake, please apply the desired version labels or run the backport tool manually.

Manual backport

To create the backport manually run:

node scripts/backport --pr 126729

Questions ?

Please refer to the Backport tool documentation

maksimkovalev pushed a commit to maksimkovalev/kibana that referenced this pull request Mar 18, 2022
@renovate @markov00 @maksimkovalev
Update dependency @elastic/charts to v45 (main) (elastic#126729)
53bedbe 
* Fix chart breaking changes in Heatmap and small stylistic fixes
* Ping every team with non EUI themed chart
* Update charts to 45.0.1
* Updated donut chart snapshot
* Fix swimlane functional tests
* Fix new swimlane height and test x offset
* Add link to EUI theme README.md

Co-authored-by: Marco Vettorello <marco.vettorello@elastic.co>
@kibanamachine
Copy link
Contributor

kibanamachine commented Mar 18, 2022

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 126729 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 18, 2022
@markov00 markov00 removed backport missing Added to PRs automatically when the are determined to be missing a backport. auto-backport Deprecated - use backport:version if exact versions are needed labels Mar 21, 2022
@kibanamachine
Copy link
Contributor

kibanamachine commented Mar 21, 2022

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 126729 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 21, 2022
@markov00 markov00 added backport:skip This PR does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Mar 21, 2022
@JordanSh JordanSh mentioned this pull request Jun 20, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasonrhodes jasonrhodes jasonrhodes approved these changes

@ryankeairns ryankeairns ryankeairns approved these changes

@shahzad31 shahzad31 shahzad31 approved these changes

@darnautov darnautov darnautov approved these changes

@ymao1 ymao1 ymao1 approved these changes

@JordanSh JordanSh JordanSh approved these changes

@YulNaumenko YulNaumenko YulNaumenko approved these changes

Assignees

No one assigned

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.2.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.