In our current effort to replace our existing charts with Elastic Charts, I've discovered a few issues that we should take a look at before shipping the replacement.
I've divided the issues into blockers and polish in order to help prioritization a bit before FF.
Clickable buckets regardless of height
Aggregate value in legend
โ ๏ธ Moved to issue https://github.com/elastic/kibana/issues/84478
Aggregate value in legend for errors
โ ๏ธ Moved to issue https://github.com/elastic/kibana/issues/84478
Both issues above are already described as part of https://github.com/elastic/kibana/pull/80298 - Charts issue https://github.com/elastic/elastic-charts/issues/561
Tooltip displayed on top of menu
๐ Charts team have opened an enhancement issue for this https://github.com/elastic/elastic-charts/issues/922
maxValue
in the ticks. Problematic when the graphs go beyond the upper tick. ๐ด๐ด๐ด๐ดโช๏ธ โ ๏ธ Moved to https://github.com/elastic/kibana/issues/84482
๐ View screenshot
โ ๏ธ Moved to enhancement issue https://github.com/elastic/kibana/issues/84481
๐ View screenshot
- [ ] Increase the number of legend items we display in the legend to 4 or 5. We currently reserve too much space for variable content.
๐ View screenshot
[x] Y-axis ticks don't show maxValue or any other ticks for percentages
๐ View screenshot
[x] Chart tooltips should be made more legible. Reduce the timestamp to only hh:mm:ss instead of showing the absolute (this is also the case in our existing charts, I would just prefer to change it)
๐ View screenshot
Pinging @elastic/apm-ui (Team:apm)
Chart tooltips should be made more legible. Reduce the timestamp to only hh:mm:ss instead of showing the absolute (this is also the case in our existing charts, I would just prefer to change it)
@formgeist The date format, in this case, is not absolute. It's dynamically calculated based on the difference between the dates. This Function is used to calculate it
And this is the possible output:
* | Difference | Format |
* | -------------- |------------------------------------------------|
* | >= 5 years | YYYY - YYYY |
* | >= 5 months | MMM YYYY - MMM YYYY |
* | > 1 day | MMM D, YYYY - MMM D, YYYY |
* | >= 5 hours | MMM D, YYYY, HH:mm - HH:mm (UTC) |
* | >= 5 minutes | MMM D, YYYY, HH:mm:ss - HH:mm:ss (UTC) |
* | default | MMM D, YYYY, HH:mm:ss.SSS - HH:mm:ss.SSS (UTC) |
In your case:
The difference is less than 5 minutes, that's why it formates as milliseconds
.
I can see if it's possible to increase the width of the tooltip to have it on a single line. WDYT?
@cauemarcondes Thanks for explaining - I'm surprised that we opt to show milliseconds for anything below 5 minutes. That's still a really long time, and the issue I see is that those labels are really hard to process for a user even if we make the tooltip wider to accommodate the longer labels. I would much prefer us to reconsider the formats so we're presenting rounded values instead of what I refer to as "absolute". If we take the common case, using our default range, the user will see this;
I don't think it's necessary to have the milliseconds in there unless the difference is perhaps less than a second. I understand this is pretty aggressive formatting of time, but I think it's for the benefit of improved legibility of the labels. cc @sqren thoughts?
Hey Team, I'm not 100% sure but from the screenshots, it looks like you are not using the EUI Chart theme, that is the default theme to use for charts inside Kibana.
There is a service that you should use to get the correct theme (it also allows you to get the right one if you are in darkmode)
https://github.com/elastic/kibana/blob/master/src/plugins/charts/public/services/theme/README.md
EUI decided design and manage this theme outside the elastic-charts library, so unfortunately at the moment, you have to use it from the services mentioned above and not from the one coming with the library.
Hey Team, I'm not 100% sure but from the screenshots, it looks like you are not using the EUI Chart theme, that is the default theme to use for charts inside Kibana.
There is a service that you should use to get the correct theme (it also allows you to get the right one if you are in darkmode)
hey @markov00 thanks for your comment, I'm going to look into it.
@cauemarcondes I added a new polish issue in the issue description where the chart tooltips are showing on top of the Kibana header.
Hey Team, I'm not 100% sure but from the screenshots, it looks like you are not using the EUI Chart theme, that is the default theme to use for charts inside Kibana.
@markov00 can you tell me how you can tell we are not using the theme correctly based on the images?
@cauemarcondes from the last shared set of images (the one with the tooltips) seems that you are using the EUI theme. I've wrote that comment after seeing the image shared here: https://github.com/elastic/kibana/issues/84178#issuecomment-733652626The tick lines on the axis are not enabled by default on the EUI theme
@cauemarcondes I added a new polish issue in the issue description where the chart tooltips are showing on top of the Kibana header.
@formgeist for this issue you can use an option calledboundary
in the tooltip configuration that allows you to specify a DOM element to contain tooltip. @nickofthyme can provide more details. You can also check this story for an example usage https://elastic.github.io/elastic-charts/?path=/story/bar-chart--test-tooltip-and-rotation
for this issue you can use an option called boundary in the tooltip configuration that allows you to specify a DOM element to contain tooltip
That's for the tip @markov00
for this issue you can use an option called boundary in the tooltip configuration that allows you to specify a DOM element to contain tooltip. @nickofthyme can provide more details
@markov00 @nickofthyme About the tooltip boundary, I create a simple example here to reproduce the problem, which is the tooltip being shown on top of the menu.
I believe only adding boundary
is not enough, because the chart is still visible, but behind the menu, I think the real problem is that the z-index
of the Header is set to 1000
and the tooltip is set to 1000000
, making it to show on top of all elements.
~@cauemarcondes I don't understand the issue based on the example you posted. The tooltip appears to function as expected.~
Nvm, I see this issue now. Checking...
The boundary
prop is the element in which the tooltip will _attempt to contain itself within_, this is dependent on the placement
and fallbackPlacements
. The default value for the boundary
is the first parent scroll container on the dom tree. If you pass the string 'chart'
it will use the chart element itself as the boundary
, but again there is no guarantee that the tooltip will be inside of the chart depending on the tooltip and chart dimensions, the placement
and fallbackPlacements
.
If you are trying to place a menu element above the chart via some absolute positioning, the boundary
will not help.
@nickofthyme This is what I what to avoid:
The tooltip showing on top of the menu. As you mentioned I think it's not possible to fix it with boundary
, am I correct?
Oh I see, yeah that's tough I don't think the boundary
is not gonna help you when the menu being position: fixed
, or absolute
for that matter, and the containing chart element is a scroll container. Because the scroll container dimensions are moving with the scroll so it will never block the placement.
Still looking to see if there is some type of hack to make it work.
Update:
So it looks like setting the tooltip z-index
is the best option. We can add the option to our theme and set the value to be under the kibana menus.
Update:
So it looks like setting the tooltip z-index is the best option. We can add the option to our theme and set the value to be under the kibana menus.
Thanks @nickofthyme , I see that you've already created the issue. ๐
@formgeist we just released a new version of charts that includes the ability to add a listener to a bucket click: 24.2.0
Please refer to the PR description for a general overview: https://github.com/elastic/elastic-charts/pull/913
and the storybook example for a quick look into that https://elastic.github.io/elastic-charts/?path=/story/interactions--bar-clicks-and-hovers (the github page didn't pick up yet the update, but please check again later)
We also have a PR to update the package on Kibana but it's currently on hold due to an issue with renovatebot.
https://github.com/elastic/kibana/pull/84223 We will update and merge that PR as soon as that issue is solved
@markov00 Fantastic! Thanks for the quick turn around on this ๐
I don't think it's necessary to have the milliseconds in there unless the difference is perhaps less than a second. I understand this is pretty aggressive formatting of time, but I think it's for the benefit of improved legibility of the labels. cc @sqren thoughts?
We could definitely re-think the time bounders of that function. @sqren WDYT?
I agree, having milliseconds is quite noisy and it's better only to show it for small durations (less than 10 seconds or so).
I've opened a number of follow up issues in order to close this one out. There's a list of references available above. Considering most of the critical blockers have been resolved by a first pass PR and help from the charts team I'll close this issue.
Most helpful comment
@formgeist we just released a new version of charts that includes the ability to add a listener to a bucket click:
24.2.0
Please refer to the PR description for a general overview: https://github.com/elastic/elastic-charts/pull/913
and the storybook example for a quick look into that https://elastic.github.io/elastic-charts/?path=/story/interactions--bar-clicks-and-hovers (the github page didn't pick up yet the update, but please check again later)
We also have a PR to update the package on Kibana but it's currently on hold due to an issue with renovatebot.
https://github.com/elastic/kibana/pull/84223 We will update and merge that PR as soon as that issue is solved