-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[TSVB] Fix series rendering order #53488
[TSVB] Fix series rendering order #53488
Conversation
Pinging @elastic/kibana-app (Team:KibanaApp) |
@elasticmachine merge upstream |
} | ||
)} | ||
) | ||
.reverse()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a comment here, why we should revert the array here.
This is not so obvious, and definitely in the future someone will try to remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! TSVB part LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch @kertal Let me check what we are missing |
@elasticmachine merge upstream |
…0/kibana into 2019-12-17_fix_tsvb_series_order
💔 Build FailedHistory
To update your PR or re-run it, just comment with: |
Closing and waiting for a fix in elastic-chart |
Summary
We switched the implementation of TSVB rendering library between 7.4 and 7.5. Unfortunately the rendering order of the series wasn't fully respected in this refactoring.
This PR fix the current rendering order of the series, going back to the original order of theused in 7.4:
This issue was already raised during the refactor PR #33558
fix #52846
For reviewers
it's easier to see the changes when hiding the whitespaces from the diff ->
Hide whitespace changes
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.[ ] This was checked for cross-browser compatibility, including a check against IE11[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support[ ] Documentation was added for features that require explanation or tutorials[ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers
[ ] This was checked for breaking API changes and was labeled appropriately[ ] This includes a feature addition or change that requires a release note and was labeled appropriately