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

[APM] Fix Transaction duration distribution barchart clickarea #84394

Merged

Conversation

cauemarcondes
Copy link
Contributor

@cauemarcondes cauemarcondes commented Nov 26, 2020

This is related to #84178

  • Transaction duration distribution barchart clickarea for the selection of bucket is only on the bar itself. Quite a big usability issue in this. If this is missing option in the charts, I would propose not to replace the distribution chart for this release. Open charts issue Enable click on the whole bucket elastic-charts#846 🔴🔴🔴🔴🔴

bucket click

This PR also fixed the margin problem around the charts.
Screenshot 2020-12-02 at 15 38 22

@cauemarcondes cauemarcondes force-pushed the apm-distribution-chart-click branch from f2a795a to 45de2e0 Compare December 2, 2020 14:41
@cauemarcondes cauemarcondes added release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Dec 2, 2020
@cauemarcondes cauemarcondes marked this pull request as ready for review December 2, 2020 14:42
@cauemarcondes cauemarcondes requested a review from a team as a code owner December 2, 2020 14:42
Copy link
Member

@sorenlouv sorenlouv left a comment

Choose a reason for hiding this comment

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

Nice!!

@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Dec 2, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

@sorenlouv
Copy link
Member

Nit: when hovering over the buckets, the cursor is on the right side of the bucket. Is it possible to center it?

@sorenlouv
Copy link
Member

jenkins run the e2e

@formgeist
Copy link
Contributor

Minor visual improvement; would it be possible to change the focus border around the selected bucket to euiColorPrimary? It would separate it from the visualization palette a bit, which is useful when you have large buckets where the selection is not visible. Using the primary color is subtle, but more consistent with our selected elements style.

Screenshot 2020-12-03 at 08 53 48

@cauemarcondes
Copy link
Contributor Author

Minor visual improvement; would it be possible to change the focus border around the selected bucket to euiColorPrimary

new color
Screenshot 2020-12-03 at 15 57 52

@cauemarcondes
Copy link
Contributor Author

Nit: when hovering over the buckets, the cursor is on the right side of the bucket. Is it possible to center it?

I asked the Elastic Charts team if it is possible to set the tooltip to follow the cursor.

@cauemarcondes
Copy link
Contributor Author

@sqren currently there's no way to make the tooltip to follow the cursor.

@sorenlouv
Copy link
Member

I asked the Elastic Charts team if it is possible to set the tooltip to follow the cursor.

I didn't mean for it to follow the cursor. But the tooltip should be centered on the hovered bucket instead of being on the right edge of it.
If it's not possible yet that's fine. Perhaps you can open an issue about it?

@cauemarcondes
Copy link
Contributor Author

I didn't mean for it to follow the cursor. But the tooltip should be centered on the hovered bucket instead of being on the right edge of it.
If it's not possible yet that's fine. Perhaps you can open an issue about it?

@sqren That's what I meant, but apparently, I haven't expressed myself clearly when I said "follow", the charts team also didn't get that I wanted to show it inside 😅. But that's possible 👇🏻

Screenshot 2020-12-04 at 09 39 39

@sorenlouv
Copy link
Member

That's what I meant, but apparently, I haven't expressed myself clearly when I said "follow", the charts team also didn't get that I wanted to show it inside 😅. But that's possible

@cauemarcondes , sorry, this time I didn't express myself clearly :D I didn't mean the tooltip - I mean the vertical marker/cursor (very light grey) that currently (also in your screenshot) sits on the right edge of the bucket. It should be centered.

@cauemarcondes
Copy link
Contributor Author

@elasticmachine merge upstream

@sorenlouv
Copy link
Member

jenkins run the e2e

@cauemarcondes
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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
apm 5.4MB 5.4MB -28.0B

Distributable file count

id before after diff
default 47129 47889 +760

Page load bundle

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

id before after diff
observability 75.1KB 75.1KB +49.0B

History

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

@cauemarcondes cauemarcondes merged commit 7a5c3b4 into elastic:master Dec 14, 2020
@cauemarcondes cauemarcondes deleted the apm-distribution-chart-click branch December 14, 2020 17:29
cauemarcondes added a commit to cauemarcondes/kibana that referenced this pull request Dec 14, 2020
…ic#84394)

* [APM] select transaction distribution by clicking on the entire bucket

* fixing margins and bucket click

* changing annotation color

* adding tooltip placement bottom

* addressing pr comments

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
cauemarcondes added a commit that referenced this pull request Dec 15, 2020
… (#85816)

* [APM] select transaction distribution by clicking on the entire bucket

* fixing margins and bucket click

* changing annotation color

* adding tooltip placement bottom

* addressing pr comments

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@cauemarcondes cauemarcondes self-assigned this Dec 28, 2020
@cauemarcondes
Copy link
Contributor Author

Tests OK

@cauemarcondes cauemarcondes added the apm:test-plan-done Pull request that was successfully tested during the test plan label Dec 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:test-plan-7.11.0 apm:test-plan-done Pull request that was successfully tested during the test plan release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support v7.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants