Allow popover content to be lazily instantiated

[rosslavery]

Hey!

I've noticed that if you have a component as your sat-popover content, that the component is actually initialized, even though the popover isn't opened yet.

At first I thought this was a regression based on this comment you made from a previous conversation we had, but I was playing around on StackBlitz and rolled back a few versions and saw the same behaviour regardless.

Demo of issue: https://stackblitz.com/edit/sat-popover-issues-vicsu1

You can see that the ngOnInit of the popover-content component is called even before the button is clicked to display it.

This is an issue for us because our popover content component makes an http request. So the request is being made even before the user has shown any interest in viewing that content.

We can easily work around it by using the isOpen() method combined with *ngIf, but I thought I'd mention the issue since it seems like from your previous comment this behaviour may not be intended.

[willshowell]

Ah yep, this is not intentional - thanks for the report! I'm afraid it'll have to bump a major release since it seems like a bug folks could accidentally be relying on.

FWIW, if your "content component" is specific enough, you could inject the SatPopover and subscribe to the first opened event before making your ajax request. A workaround still, but potentially more encapsulated.

[willshowell]

@rosslavery I take that back haha. The popover doesn't exist in the DOM until opened, but is instantiate and lives in memory once declared. That's consistent with what I was getting at in https://github.com/ncstate-sat/popover/issues/23#issuecomment-345470165.

https://stackblitz.com/edit/sat-popover-issues-qfddyj

Regardless, I agree there should be a lazy way to instantiate it. I'll look into it as time permits!

[rosslavery]

Great thanks. I think material2 just added lazy instantiation for mat-tabs as well as for mat-menu, might wanna take a look at their implementation for inspiration.

[tarekbazine]

... 2 years later. @rosslavery @willshowell any update about this, I am using the popover in a dynamic built table in which I have all the cells as inline editable, in which I use the popover to show the appropriate form/picker, the problem is that all the popover are loaded ( rows nb * cells nb !!) which makes the table consuming too much resource memory/cpu.

thanks

[julianobrasil]

If you allow the edition of only one cell at a time, you could use just one popover for all the cells and one anchor per cell.

[tarekbazine]

@julianobrasil I tried sharing one popover and then passing the anchor to the cell but the popover was showing but displaying positions was not right, I end up hacking it as suggested in top with *ngIf popover.isOpen() .

[julianobrasil]

I have opened an issue about the wrong position thing a few months ago. Fortunately, there's a workaround for it: https://github.com/ncstate-sat/popover/issues/167

[isaackehle]

@julianobrasil There are a number of open issues here and in PRs. I'd be glad to get them resolved if folks can help with the testing.

[julianobrasil]

@pgkehle, do you mean the update of the .spec files?

[julianobrasil]

Or just trying the code of the PRs?

[isaackehle]

No, I mean I tried building locally from scratch some of the different PRs, some things not working. The workaround you mentioned talked about some of the code being merged w/o proper tests, so it seems there's an opportunity to add another PR to fix a couple of these issues. Looking for some help with that.

[julianobrasil]

I'll take a look at them and give it a try. I'll come back to you in case of trouble with the overall process.