[popover] Popover lazy loading of content

[knoobie]

Describe your motivation

Creating 100x Popover on the server side with rich content - when only a single one is opened by the user on demand - is really bad for performance.

Describe the solution you'd like

Either extend Popover by e.g. allowing to add callbacks as a way to add components only when the popover is opened once or create a LazyLoadingPopover with callbacks.

P.s. The callback might take some time.. server roundtrip + creation of the components.. The Popover on the client side would need some kind of loading state / loading indicator theme that the user knows something is happening. (e.g. open the popup with 3x3rem + a loading icon like shown here https://github.com/vaadin/web-components/issues/842)

Describe alternatives you've considered

No response

Additional context

No response

[mstahv]

Haven't used Popover yet (I have had the need in the past), but intuitively I'd say the callback based creation of the content (and lazy generation/loading) is the way to go. There can be lot of extra CPU and memory wasted now if the content is generated eagerly (by reading the docs I get this impression).

[mstahv]

Related, recently noticed that Details was missing this kind of API as well. Added to Viritin's version like this:

https://github.com/viritin/flow-viritin/blob/2d2d1989d87be4b161d32e3d7a1f0d6873487634/src/main/java/org/vaadin/firitin/components/details/VDetails.java#L15-L32

[jouni]

This is an obvious issue and there’s a clear need. But isn't this a broader issue/topic than Popover? Shouldn't it be possible to lazily create and render any component? Like Matti said, this is currently missing from Details, and I assume it’s also missing from Accordion, Tabsheet, and Dialog. Perhaps Context Menu should also support it.

Kinda sounds like there could/should be a general-purpose "lazy component renderer" which is rendered in the DOM in place of the actual component, and once that placeholder comes visible in the browser viewport the actual component gets rendered.

[TatuLund]

I think this is API design question. If you prefer a callback function for content producer instead of populating the PopOver/Dialog in opened changed listener event, then it is a "missing feature", but technically even with current API you are able to do this already with the mentioned components using their event listeners. More over, such callback API would be syntactic sugar on top of existing API. So fundamental question is API naming and how it would improve finding the functionality vs. state of affairs today.

[web-padawan]

Related to this, we have an example of lazy loading content in TabSheet where we specifically introduced a spinner part.

[TatuLund]

in TabSheet where we specifically introduced a spinner part.

I noticed that, and I packaged it as component, so that it is easier to re-use it where ever you want.

https://vaadin.com/directory/component/spinner

[rolfsmeds]

I think there's a ticket about it somewhere, but couldn't find it now, but some kind of LazyContainer component has been discussed now and then, that would do this and provide a few different visualizations during loading (e.g. like the skeleton loader component for vuetify https://vuetifyjs.com/en/components/skeleton-loaders/#usage)

[knoobie]

If I remember correctly, we also discussed this two years ago when you introduced TabSheet https://github.com/vaadin/flow-components/issues/3644

[rappenze]

I think this is API design question. If you prefer a callback function for content producer instead of populating the PopOver/Dialog in opened changed listener event, then it is a "missing feature", but technically even with current API you are able to do this already with the mentioned components using their event listeners. More over, such callback API would be syntactic sugar on top of existing API. So fundamental question is API naming and how it would improve finding the functionality vs. state of affairs today.

The current API does not allow to populate popover content dynamically, the positioning and sizing is wrong when you create your content within OpenedChangeListener.

[mstahv]

@rappenze Could you share an example? I didn't yet notice anything (while learning to use this new component and drafting a PoC for the lazy loading API). But my tests are quite simple ATM...

[rappenze]

Here an example which shows that lazy loading currently does not position / size the popup correctly. Important in the example is that the popup can't be shown on initial position and has to be moved. If you change the position in the example to BOTTOM_START so that the popup has enough space, all works nicely.

public class TestView extends Div {
  public TestView() {
    Button button = new Button("Test");
    add(button);

    Popover popover = new Popover();
    popover.setTarget(button);
    popover.setPosition(PopoverPosition.BOTTOM_END);
    popover.addOpenedChangeListener(event -> {
      if (event.isOpened()) {
        Div div = new Div("My popup content");
        div.setHeight("100px");
        div.setWidth("600px");
        popover.add(div);
      } else {
        popover.removeAll();
      }
    });
  }
}

Current state

Expected

[mstahv]

Thanks, easily reproducible 👍 My issue was that I didn't realise the infamously named listener is called once in the beginning, when attached (I didn't have that isOpened() check).

[mstahv]

Bug🤔

[rolfsmeds]

Somewhere in the borderlands between a bug and a missing feature, as we didn't consider someone would populate the Popover in the listener, so supporting it wasn't even considered. But certainly a valid use case in many situations. I'm also not sure how come it doesn't resize itself, since the Popover doesn't actually calculate a size for itself at all – it just natively (as in without javascript or anything) scales to accommodate its contents. (If you modify the contents in the browser's devtools after it has opened, it seems to work as expected.)

[mstahv]

Documenting another "workaround" for lazy init I figured out, that might do it for some cases: Create also Popover instance lazily in a click listener and open the popover programmatically:

    public class OtherActions extends Button {
        Popover popover;

        public OtherActions(Person person) {
            setIcon(VaadinIcon.ELLIPSIS_V.create());
            addClickListener(e -> {
                if (popover == null) {
                    var extraActions = new VVerticalLayout(
                            new ActionButton("Merge.."),
                            new ActionButton("Send email"),
                            new ActionButton("Call"),
                            new ActionButton("SMS")
                            ///...
                    );
                    popover = new Popover(extraActions);
                    popover.setTarget(this);
                    popover.setWidth("30vw");
                    popover.setHeight("45vh"); // optimally Popover would have implicit max height
                    popover.addThemeVariants(PopoverVariant.ARROW);
                    popover.addOpenedChangeListener(evt -> {
                        if (!evt.isOpened()) {
                            popover.removeFromParent();
                            popover = null;
                        }
                    });
                }
                popover.open();
            });
        }
    }

Technically even better resource wise (saving some bytes of the Popover instanses as well), but there is some nasty maintenance logic needed to free memory.