[Infra UI] Replace EUI Charts with Elastic Charts on node detail page #41262
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
simianhacker
added
review
release_note:fix
Feature:Metrics UI
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💚 Build Succeeded |
jasonrhodes
approved these changes
Jul 17, 2019
There was a problem hiding this comment.
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.
|
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. |
10 tasks
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
sgrodzicki
added
the
Team:Infra Monitoring UI - DEPRECATED
|
Pinging @elastic/infra-logs-ui |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
boundsoverride 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.Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportDocumentation was added for features that require explanation or tutorialsUnit or functional tests were updated or added to match the most common scenariosThis was checked for keyboard-only and screenreader accessibility