Try to fix Plotly layout issues

[falkoschindler]

This PR tries to solve #2917 by reverting PR #2516 to avoid Plotly issue plotly/plotly.js#4856 (see /test1 below).
But reverting this PR doesn't seem to be enough, since demo plots on our documentation page still have a wrong height, which I reproduced with /test2 below. It turned out that we need to partly revert 328fbab as well to avoid this problem.

from random import random
import plotly.graph_objects as go
from nicegui import ui

@ui.page('/test1')
def test1():
    fig = go.Figure()
    plot = ui.plotly(fig)

    def add_trace():
        fig.add_trace(go.Scatter(x=[1, 2, 3], y=[random(), random(), random()]))
        plot.update()

    ui.button('Add trace', on_click=add_trace)

@ui.page('/test2')
def test2():
    ui.timer(0.5, lambda: ui.plotly({}).classes('h-60'), once=True)

I'm still not very happy with this solution because it re-introduces unnecessary payload. So consider it as work in progress.

[bmaranville]

Even if the behavior is reverted to use Plotly.newPlot on first draw, it will presumably still use Plotly.react for subsequent draws, so won't the issue in #2917 still occur?

Also I'm having trouble reproducing the initial height issue on Firefox and Chromium using the current nicegui development branch (main) - can you provide a minimal reproduction? If I run this code in python with my browser resized to have a height < 450px it opens a new browser tab and displays properly. Does it only fail in an iframe?

import plotly.graph_objects as go
from nicegui import ui

fig = go.Figure(go.Scatter(x=[1, 2, 3, 4], y=[1, 2, 3, 2.5]))
fig.update_layout(margin=dict(l=0, r=0, t=0, b=0))
ui.plotly(fig).classes('w-full h-40')

ui.run()

Result:

[falkoschindler]

Even if the behavior is reverted to use Plotly.newPlot on first draw, it will presumably still use Plotly.react for subsequent draws, so won't the issue in #2917 still occur?

As far as I know the issue doesn't occur when using Plotly.react for updates, only when using it for the initial creation.

The height issue seems to occur only if the creation of the Plotly element is slightly delayed:

@ui.page('/test2')
def test2():
    ui.timer(0.5, lambda: ui.plotly({}).classes('h-60'), once=True)

The plot should be squeezed like this

but grows to 450px like this

[bmaranville]

One issue might be that the js-plotly-plot class is actually added during the initial render of the plot, and is not present on the div element before then. This change fixes the problem, but it's worth testing to see if it causes any additional problems (the presence of the class is used internally by Plotly to determine if there's already a plot there, but it also checks if the div is empty, so I don't think it will break anything)

diff --git a/nicegui/elements/plotly.py b/nicegui/elements/plotly.py
index cc912581..5c4b7294 100644
--- a/nicegui/elements/plotly.py
+++ b/nicegui/elements/plotly.py
@@ -35,8 +35,8 @@ class Plotly(Element, component='plotly.vue', libraries=['lib/plotly/plotly.min.
         super().__init__()
 
         self.figure = figure
+        self.classes('js-plotly-plot')
         self.update()
-        self._classes.append('js-plotly-plot')
 
     def update_figure(self, figure: Union[Dict, go.Figure]):
         """Overrides figure instance of this Plotly chart and updates chart on client side."""

...actually now that I think about it I'm not sure why this fixes the issue. The h-60 class is still not applied until after the call to ui.plotly({}), but it's interesting.

[bmaranville]

Is there a race condition between the application of the classes (like h-60) and the rendering of the widget? Seems like there could be.

[bmaranville]

I think the only way to control the initial height of the plotly plot in a stable way is to either

• set the height of the div into which the plot will be rendered before rendering is started (add optional height kwarg to the ui.plotly command? add optional class list kwarg that gets applied to the target div before rendering?)
• explicitly tell plotly what height you want, by adding it to the layout object, e.g. ui.plotly({"layout": {"height": 260}}), but this is awkward because plotly seems to only accept numerical values for height (not em or rem).

[bmaranville]

There is another race condition: after creating a div with class h-60 in a nicegui component, I am timing that it takes > 150ms before the height (as measured by style.getPropertyValue('height') becomes non-zero, which I'm guessing comes from a delay in loading the Tailwind css rules.

[bmaranville]

I see that tailwind is being used client-side as a script rather than a CSS static file - I think that explains the delay (the script is analyzing the page content to dynamically insert style rules?)

It might be nice performance-wise to compile all the styles that would result from executing every combination in tailwind_types into a static CSS artifact that could be used in the client. It would certainly avoid delays like these.

[falkoschindler]

That's a very good point, @bmaranville. Quasar classes are generated via JavaScript, probably after the DOM has been created. But I still don't understand and would like to find out why the original plotly.vue (before PR #2917) now requires these changes in element.py. And why exactly text, style, slots and events - nothing more, nothing less?

We might end up setting an initial height somewhere via CSS or Plotly option. But it worked before PR #2917 and this PR fixes it again, so I'm not convinced that inducing another height value is necessary. I just don't understand and like the current solution, because it doesn't make much sense to me and it increases the payload again.

[bmaranville]

The Quasar classes are not what I was referring to - it's the use of Tailwind as a script at runtime rather than using the Tailwind API to generate static CSS at build time. The use of the script version is not recommended for production in the Tailwind documentation: https://tailwindcss.com/docs/installation/play-cdn

When using the Tailwind script there is automatically a race as an inline CSS is dynamically generated by a script that is reading the DOM and updating the CSS in real time, so after you add a class like h-60, it will have no immediate effect. At some later time (> 150ms) the inline stylesheet will be updated, then the style of the class will change.

Most of the time you don't care, because everything just updates pretty fast, but if you have a library that is trying to read the style of a container before rendering itself, it won't work. The times that it appears to work are probably because an additional update() is called, which then triggers the auto-resize (if {config: {responsive: true}} is set)

I noticed that in the demo code for reproducing the problem, update() was called twice (once by the mounted() function, once by the updated() function), in quick succession. There seem to be multiple potential race conditions here then.

I still suggest that the only (stable) way to make sure the first render of the plot is sized to the container is to size the container before rendering. Sometimes other approaches will work because of race conditions.

[falkoschindler]

After an exciting debugging session with @codingpaula we finally seem to understand what is happening when we simply revert PR #2516:

1. Vue receives a Plotly element with undefined attributes like text, style, slots or events, which are automatically set to null, [] or {}.
2. The plot is initially rendered using Plotly.newPlot.
3. Tailwind notices a new class "h-60" and adds it to the CSS definition in <head>, causing the Plotly container to shrink.
4. The updated text attribute causes a second rendering using Plotly.react, which is buggy and doesn't resize correctly.

We fixed it by replacing undefined element attributes before rendering anything. This not only avoids the buggy resizing of Plotly.react (see plotly/plotly.js#4856), it also should reduce the computation for other elements as well. Because we don't change the elements dictionary anymore while rendering, Vue doesn't try to re-render everything.