Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Infra UI] Replace EUI Charts with Elastic Charts on node detail page #41262

Merged
merged 9 commits into from
Jul 18, 2019

Conversation

simianhacker
Copy link
Member

@simianhacker simianhacker commented Jul 16, 2019

Summary

This PR fixes #29135 by replacing the EUI Charts with Elastic Charts on the node detail page. I also to this opportunity to clean up the ChartSection component a bit. I removed the bounds override because was only used on the CPU chart and I felt it look better without it. Also Elastic Charts doesn't have support for synchronized tooltips so I removed that feature for now.

image

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@jasonrhodes jasonrhodes requested review from jasonrhodes and removed request for a team July 17, 2019 02:29
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.

Some of the Elastic Charts design elements look a little broken to me but I checked their docs and I think it's just how they implement things (like the colored rings that appear on hover but only when you hover right over the element, or how those rings are the same color as the area chart so they disappear half or all the way a lot of the time, etc.)

Other than that, this appears to work. @snide will be pleased.

@snide
Copy link
Contributor

snide commented Jul 17, 2019

Awesome! If you have issues with the actual charts I'd bring it up with @markov00 and add an issue on their repo. I agree there's room for improvement and we've been helping them style stuff up.

@simianhacker simianhacker merged commit 879dfad into elastic:master Jul 18, 2019
simianhacker added a commit to simianhacker/kibana that referenced this pull request Jul 21, 2019
…elastic#41262)

* Remove EUICharts from node detail replace with Elastic Charts

* Moving stream check inside onChangeTimerange check

* Fixing typo

* Adding error message back in

* Removing exception from getMaxMinTimestamp()

* Checking for valid data

* Adjusting i18n names
simianhacker added a commit that referenced this pull request Jul 22, 2019
…#41262) (#41642)

* Remove EUICharts from node detail replace with Elastic Charts

* Moving stream check inside onChangeTimerange check

* Fixing typo

* Adding error message back in

* Removing exception from getMaxMinTimestamp()

* Checking for valid data

* Adjusting i18n names
@sgrodzicki sgrodzicki added the Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services label Jul 29, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-logs-ui

@simianhacker simianhacker deleted the fixes-29135 branch April 17, 2024 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Metrics UI Metrics UI feature release_note:fix review Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.4.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Infra UI] Change charts on metric detail from EUI Charts to Elastic-Charts
5 participants