elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.71k stars 8.13k forks source link

[Lens] workspace panel dimensions by chart type #136993

Closed drewdaemon closed 8 months ago

drewdaemon commented 2 years ago

An idea from https://github.com/elastic/kibana/issues/134242 . We could allow visualization types to impose constraints on the dimensions of the workspace panel. These could be specific measurements or just an aspect ratio.

Potential benefits

Example from the metric vis:

Screen Shot 2022-07-22 at 10 24 49 AM
elasticmachine commented 2 years ago

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

elasticmachine commented 2 years ago

Pinging @elastic/datavis (Team:DataVis)

MichaelMarcialis commented 2 years ago

Responded to the thread in Slack, but adding here for the sake of visibility. Personally, Iā€™d prefer to get rid of the white shadowed panel/container for the visualization altogether in Lens, and simply make the center content area (containing toolbar visualization and suggestions) all on a white background. I think it would help reduce visual noise and avoid the box-in-a-box looks that a lot of areas suffer from.

drewdaemon commented 2 years ago

Another option suggested by @ThomThomson is to retain the border, but remove the differences in background color between the workspace and the sidebars.

Screen Shot 2022-09-14 at 9 38 03 AM
markov00 commented 1 year ago

@ThomThomson great idea, we love such clean up: this box in box UI pattern start popping up a bit too much around our products, I think is safe and genuine to start cleaning it up a bit more

stratoula commented 1 year ago

Another idea mentioned in our weekly syc, do not change the workspace size but add a shadow instead to distinguish between the visualization and the panel. We should evaluate each one of them

stratoula commented 1 year ago

cc @gvnmagni

gvnmagni commented 1 year ago

@MichaelMarcialis I would love to discuss with you about this since we have few implications that are bigger than this single chart

MichaelMarcialis commented 1 year ago

@MichaelMarcialis I would love to discuss with you about this since we have few implications that are bigger than this single chart

Yes, I'll put something on the calendar for us to sync up. The biggest question I have (assuming we wish to allow the white workspace panel to shrink/hug certain visualization types) is how will the workspace's DND drop zone be affected? Should the drop zone also shrink? Or should it always maintain the same full width/height dimensions? Something for us to discuss. Stay tuned.

MichaelMarcialis commented 1 year ago

Hey, gang. @gvnmagni and I met up to discuss this issue. After discussing possible solutions, we ultimately landed on the same general direction that @drewdaemon described earlier. This includes:

If we're all in agreement with this direction, let me know and I'll plan to proceed with explorations on the drop zone question when I get a moment.

stratoula commented 1 year ago

@MichaelMarcialis yes this sounds great! Looking forward to the designs :)

MichaelMarcialis commented 1 year ago

@stratoula: As promised, I've explored some possible directions on how to proceed with the workspace drop zones when we're dealing with a Lens visualization type that is restricted to a maximum width, maximum height, and/or an aspect ratio.

Ultimately, I think I much prefer the panel-only direction out of the two explorations shown below. While the full workspace drop zone example does benefit from a consistent and very large size, it doesn't look too great on the metric example and looks like a visual bug on the time series example (with the drop zone appearing flush horizontally, but overflowing vertically). On the other hand, the panel-only direction looks nicer and intentionally placed in all examples. The only downside to this approach is that it will present users with a smaller drop zone for some visualization types, which will ask that they drag fields a bit further from the field list, but that's a tradeoff I'm willing to accept for the time being (and one we can monitor for feedback).

CCing @gvnmagni in case he has any additional thoughts.

Panel-Only Drop Zone (My Preferred Direction)

Metric Example

Panel Only

Time Series Example

Panel Only

Full Workspace Drop Zone

Metric Example

Full Workspace

Time Series Example

Full Workspace
gvnmagni commented 1 year ago

I love how clean and elegant it looks in the first option, I would prefer that as well. We can eventually enlarge that if users have troubles with it, but I doubt that it would be a problem šŸ˜Š

stratoula commented 1 year ago

@MichaelMarcialis thanks so much! The panel drop zone was my personal preference so happy that we agree!

drewdaemon commented 11 months ago

A few learnings:

Aspect ratios are easy(-ish)

Defining an aspect ratio without worrying about the actual dimensions is straightforward (e.g. line charts should be 16:9). Can use flexbox with the CSS aspect-ratio property.

Predicting metric dimensions

Metric dimensions are supposed to follow this logic

In the small multiples case, numMetrics can sometimes be known before receiving data. In other cases, we can't know until we receive data.

Breakdown function can know tile count before data
Top values Yes
Filters Yes
Intervals No
Date histogram No

So, width can always be known based on configuration, but height sometimes can't be known until we have data.

Timing WRT to chart render

The workspace panel should be resized in the same render cycle as the chart is drawn.

Resizing too soon

Chart render is always after data is received. So, if the workspace is resized as soon as we know what size it should be (sometimes before data, and always before chart render) the previous chart will be displayed distorted before the new chart is rendered.

Screenshot 2023-10-19 at 9 04 21 AM

Panel is resized before chart updates.

Screenshot 2023-10-19 at 9 04 26 AM

New chart is eventually rendered into the panel.

Resizing too late

However, if we wait to set the dimensions until the charts library notifies of a completed render, the chart will be drawn in an unsized dimension panel which is then sized, thereby triggering a second chart render.

https://github.com/elastic/kibana/assets/315764/626e396c-e3c8-4d3a-bab5-cd183cb2886d

Conclusion

The existing charts notifications need to be extended with a "will-render" event that notifies the consumer that the chart will be rendered on the next cycle. That way, we can cue up the dimension change to coincide with the chart's render.

@nickofthyme does this sound reasonable?

Where dimensions should be computed

There seem to be two viable options as to where we compute the dimensions

  1. the visualization class
  2. the expression renderer

With 1, we get the flexibility of knowing the panel dimensions before data has been received in some cases. I don't foresee much use for this but I guess it could be useful for https://github.com/elastic/kibana/issues/136995 if the user was to create a Lens visualization from dashboard and then click Save and return before the render is complete. But it feels like an unlikely case perhaps?

With 2, we don't need to move as much code and it makes sense to me that the expression renderer would request optimal dimensions/aspect ratio depending on the information it has.

So, I lean toward 2, but I'm looking for feedback (cc @stratoula )

nickofthyme commented 11 months ago

Great analysis here!

However, if we wait to set the dimensions until the charts library notifies of a completed render, the chart will be drawn in an unsized dimension panel which is then sized, thereby triggering a second chart render.

Is this second render ever noticeable by eye?

The existing charts notifications need to be extended with a "will-render" event that notifies the consumer that the chart will be rendered on the next cycle. That way, we can cue up the dimension change to coincide with the chart's render.

Yeah I think this sounds like a possible solution and I think pretty easy to implement from the charts size, just need to find where/when to fire the event to have the right affect.

drewdaemon commented 11 months ago

However, if we wait to set the dimensions until the charts library notifies of a completed render, the chart will be drawn in an unsized dimension panel which is then sized, thereby triggering a second chart render.

Is this second render ever noticeable by eye?

Good question. At least on my computer, it looks more like a flurry of activity. I don't think I'd notice the distinct stages if I didn't know what to look for. If you slow the video above down to .5 speed, you can see the stages.

Screenshot 2023-10-19 at 10 17 49 AM

Full workspace render

Screenshot 2023-10-19 at 10 18 02 AM

After resize but before re-render

Screenshot 2023-10-19 at 10 18 11 AM

After resize and re-render

But, when I throttle the CPU to simulate a slower machine, it becomes even more noticeable.

nickofthyme commented 11 months ago

Yeah that's not good. Also thanks for sharing the playback speed option, didn't know that existed! šŸŽ‰

stratoula commented 10 months ago

Yes this is not good, even without slowing the video I can see the flickering so I like the idea of the consumer notification.

With 2, we don't need to move as much code and it makes sense to me that the expression renderer would request optimal dimensions/aspect ratio depending on the information it has.

I agree with option 2 Drew, it makes more sense to be on the expression renderer side. This is what I would expect as a developer šŸ‘

dej611 commented 10 months ago

Breakdown function can know tile count before data Top values Yes Filters Yes Intervals No Date histogram No

For Intervals you can find the number of buckets in two ways:

drewdaemon commented 10 months ago

Another twist. Even after the willRender hook is in charts, we have a problem.

  1. charts library plans chart dimensions based on current container size
  2. charts library schedules a render and notifies consumer
  3. consumer schedules a dimensions update
  4. browser updates container dimensions and draws chart simultaneously
  5. charts library detects the discrepancy
  6. charts library redraws chart

Or visually:

Screenshot 2023-10-23 at 2 14 50 PM

charts library uses existing container dimensions to plan render

Screenshot 2023-10-23 at 2 15 01 PM

render and container dimension update occur simultaneously

Screenshot 2023-10-23 at 2 15 10 PM

charts library resizes chart to new dimensions

drewdaemon commented 10 months ago

Blocked on https://github.com/elastic/elastic-charts/issues/2221

drewdaemon commented 10 months ago

Blocked on https://github.com/elastic/elastic-charts/issues/2238

drewdaemon commented 10 months ago

Decided synchronously not to block this effort on https://github.com/elastic/elastic-charts/issues/2238

ninoslavmiskovic commented 9 months ago

Is the status on this still blocked ? @drewdaemon + @stratoula

stratoula commented 9 months ago

We are just waiting for this https://github.com/elastic/kibana/pull/170914 to be merged