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

Remove the padding in Elastic chart #736

Closed
Tracked by #118429
AlonaNadler opened this issue Jul 1, 2020 · 9 comments
Closed
Tracked by #118429

Remove the padding in Elastic chart #736

AlonaNadler opened this issue Jul 1, 2020 · 9 comments
Labels
enhancement New feature or request

Comments

@AlonaNadler
Copy link

In manty operation driven dashboards, the use of the screen real estate is really important. Users are trying to fit as many metrics in the size of the screen. It came to my attention that when using Lens and in Kibana, in general, there is a big waste of space which limit the number of chart and panel that can fit in.
image

image

@AlonaNadler AlonaNadler added the enhancement New feature or request label Jul 1, 2020
@markov00
Copy link
Member

markov00 commented Jul 6, 2020

I think this can be achieved with few options, as also discussed during the roadmap planning:

  • add a slim theme, that reduces margin/sizes and other unused spaces
  • I'm not a fan of axis titles, because most of the time the same concept is already expressed in the title of the chart

Screenshot 2020-07-06 at 15 07 21

In this example, the axes titles can be removed and you can add just a meaning full title like `Daily infections cases`

Screenshot 2020-07-06 at 15 09 38

This is also another example where the titles can be omitted easily, the same happens for all the other metrics on the top left/right (TR, TC, TD can be completely removed and more space can be freed) - we can have also a flag that can be switched on to allow automatic changes on the LOD (Level Of Details) of the chart depending on the screen/panel size, like it's done in the EUI examples https://eui.elastic.co/#/elastic-charts/sizing

@AlonaNadler I've a side question for you: how did you achieved that multiline title? I'm pretty sure this can't be done in elastic-charts 😕
Screenshot 2020-07-06 at 15 02 32

@AlonaNadler
Copy link
Author

This is a TSVB chart and below the Lens chart. The size of the panels is exactly the same
image

The padding in the Lens chart makes the chart itself smaller which is harder to read.

  • between the title and the legend there is an empty space that is not needed.
  • on the right side of the Lens chart there is a an empty space which is not needed.

Besides the padding, there are other differences which I don't get that makes the TSVB easier to read

cc: @markov00 @cchaos @MichaelMarcialis

@MichaelMarcialis
Copy link

MichaelMarcialis commented Oct 20, 2020

Hey, @AlonaNadler! I took a look into this and found a few interesting facts. Before diving in though, I believe in your previous example the top visualization is actually one of the older/standard visualizations (not TSVB), but please correct me if I'm wrong. That said, here's what I found.

Older/Standard Visualizations

  • Padding is split between an outer (div.visualization) and inner (div.visWrapper) container.
  • There is 8px of horizontal and vertical padding on the outer container.
  • There is an additional 8px of vertical padding on the inner container. This is why spacing looks better in the older/standard visualizations when a top legend is present, because the top legend is sandwiched in between the two 8px paddings, rather than just being under one 16px padding.
  • A bit of a tangent, but I also noticed that the axis titles are a light gray, which differs from both the Lens and TSVB visualizations.

Lens Visualizations

  • There is 16px of horizontal and vertical padding styled inline on a single container (div.expExpressionRenderer__expression).

TSVB Visualizations

  • Spacing is split between an outer (div.visualization) and inner (div.tvbVis) container.
  • There is 8px of horizontal and vertical padding on the outer container.
  • There is an 8px transparent border on the inner container. This appears to be used as some kind of faux padding, though I don't know why. Also, the legend is nested in this inner container, so the dividing of the spacing styles doesn't make as much sense here as it does in the older/standard visualizations.

Based on your comments, it sounds like you'd prefer a consistent 8px of horizontal and vertical padding across all three visualization types. Is this correct? If so I believe that could be easily done by having a singular outer container for the visualizations with the 8px of padding applied. Additionally, in situations where there is a top or bottom legend present, that legend's element receives an additional 8px margining at the appropriate position (top or bottom) in order to offset it from touching the chart.

Regarding the extra space on the right-hand side of the Lens chart, I'll leave it to @markov00 to confirm, but I speculate that it's either accounting for the possibility of a right-hand axis and label, or ensuring that the last x-axis label doesn't get cut off when running close to the end of the canvas element.

image

Thoughts?

@nickofthyme
Copy link
Collaborator

nickofthyme commented Oct 20, 2020

@MichaelMarcialis I think you are correct regarding the screenshot being an old (non-elastic chart) TSVB version based on the legend color size difference.

Regarding the extra space on the right-hand side of the Lens chart, I'll leave it to @markov00 to confirm, but I speculate that it's either accounting for the possibility of a right-hand axis and label, or ensuring that the last x-axis label doesn't get cut off when running close to the end of the canvas element.

You are sort of correct, the screenshot @AlonaNadler shared is actually not rendering the last tick (or rather creating the extra space) because the domain end does not exactly align with the tick interval. We have an issue (#397) to fix this via "nicing" the ticks to show better intervals and ending values given the data and domain. However, the image you shared does create extra space to allow for the end value to not be clipped off the edge of the canvas area like you correctly speculated.


One thing I'd like to add is that the TSVB chart is using the elastic-charts standard theme, whereas Lens is using the EUI charts theme via this service, which could effect the padding/margin inside the canvas element. I think whatever changes are made should include adding the eui theme to the TSVB chart to get a more consistent theme across all charts in kibana.

@AlonaNadler
Copy link
Author

AlonaNadler commented Oct 20, 2020

Based on your comments, it sounds like you'd prefer a consistent 8px of horizontal and vertical padding across all three visualization types. Is this correct?

Not exactly, I don't care so much about consistency. I focus this on Lens, we want to allow users to user the screen real estate and panel real estate in a more efficient way. Currently Lens has a lot of needed spaces.

This is a screenshot I got from @gose yesterday that explain a similar pain point:

image

To summarized:

  • Focus on Lens
  • This is not about consistency, it's about efficient visualizations panels in dashboards and the spaces takes a lot of the page

@MichaelMarcialis
Copy link

@AlonaNadler: Sounds good! Is this something you'd like me to cut a PR for, or is this something that should be assigned to an engineer on Dashboard or Lens?

@AlonaNadler
Copy link
Author

@markov00 can you please point out which team can help which section. I understand it is a combination of EUI, ES-chart and Lens

@markov00
Copy link
Member

Yes exactly, we will take care of the padding caused by the chart axis labels, trying to limit and use it only when strictly necessary. Dashboard team and Lens should take care of their respective styles applied to panels.
This should probably replace the current switch in dashboard Use margins between panels with a more global Compact view where margins are removed from the panels together with a more dense theme applied to charts and titles

@markov00
Copy link
Member

markov00 commented Jun 4, 2024

Closing as this should be handled in Kibana and the chart theme could be customized there

@markov00 markov00 closed this as not planned Won't fix, can't repro, duplicate, stale Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants