feat(expansion-panel): allow for content to be rendered lazily #8243

crisbeto · 2017-11-06T09:25:29Z

Currently the expansion panel renders all of its content on load which can be expensive, considering that all of it won't be visible. These changes allow for some of the content to be rendered lazily via the matExpansionPanelContent directive.

Fixes #8230.

josephperrott · 2017-11-06T17:23:46Z

src/lib/expansion/expansion-panel.ts

+      }
+    });
+
+    // Remove the content once the panel is closed and the closing animation is done.


What is the reasoning behind removing the content on closing? Since the issue we are looking to address is lazy loading, is there a reason to detach and reattach all the time?

I was a bit on the fence on this one as well, but it could help if you have some really massive panels with lots of elements. I'm open to not doing this though.

Could this be configured perhaps via an Input?
I'm developing (for learning purposes) an RSS feed reader. On desktop, each item is placed inside an expansion panel (for fancyness (?)); inspired by Google Inbox interface.

Removing the content inside the panel after been read, could be something I'd like to happen. Being that as more and more items are been loaded, more resources will be consumed. (O course I shall do something about this regardless of this specific feature).

However, also thinking about other Google products like GSuite admin console: where some settings are loaded asynchronously the first time, but then kept. (I don't know if they're actually removed from DOM, but I wouldn't remove them in such scenario).

I think it might be best at this point to not remove the content once it is placed and we can determine if it is something that we want to allow later, potentially as @bardiel mentioned via @Input

josephperrott · 2017-11-06T17:30:10Z

src/lib/expansion/expansion-panel.ts

@@ -127,12 +147,31 @@ export class MatExpansionPanel extends _MatExpansionPanelMixinBase
    return this.expanded ? 'expanded' : 'collapsed';
  }

+  ngAfterViewInit() {


We have had a number of issues with content changed after checked for these situations, are we going to run into the same issue here?

Probably not, those exceptions are usually if data bindings change. This is the same way we're doing it for tabs.

For tabs we actually are having to change it because of these issues #7266

We are creating a custom template portal and having the template portal itself be responsible for attachment because of issues with ChangeAfterContentChecked. I just want to be sure we don't accidentally create an issue further down the road.

jelbourn · 2017-11-06T21:12:07Z

src/lib/expansion/expansion-panel.html

  <div class="mat-expansion-panel-body">
-    <ng-content></ng-content>
+    <ng-template><ng-content></ng-content></ng-template>


This doesn't stop Angular from initializing stuff; we can make this change to stop stuff from making it into the DOM, but we'll ultimately need an alternate API where users provide an ng-template

Ah, I was under the impression that this was the reason we do it the way we do for the tabs. In that case I'll rework it to use a @ContentChild to find an ng-template and instantiate it.

@crisbeto - Look at what I did for #6832

crisbeto · 2017-11-12T11:34:29Z

Rebased and reworked the approach to take an ng-template with matExpansionPanelContent instead. Can you take another look @jelbourn @josephperrott?

jelbourn · 2017-11-14T02:06:48Z

src/lib/expansion/expansion-panel.ts

+    if (this._lazyContent) {
+      // Render the content as soon as the panel becomes open.
+      this.opened.pipe(
+        startWith(null!),


Why does this start with never?

Because the opened stream won't emit if the panel is open by default. This ensures that we render any panels that are open on load.

jelbourn · 2017-11-14T02:09:06Z

src/lib/expansion/expansion.md

+### Lazy rendering
+
+If you want some content not to be rendered until its expansion panel is opened, you can wrap it in
+an `ng-template` using the `matExpansionPanelContent` directive.


How about

By default, the expansion panel content will be initialized even when the panel is closed. To instead defer initialization until the panel is open, the content should be provided as an `ng-template`: ```html <mat-expansion-panel> <mat-expansion-panel-header> This is the expansion title </mat-expansion-panel-header> <ng-template matExpansionPanelContent> Some deferred content </ng-template> </mat-expansion-panel> ```

crisbeto · 2017-11-14T18:06:14Z

Addressed the feedback @jelbourn.

jelbourn

LGTM

rafaelss95 · 2017-11-17T01:25:20Z

src/lib/expansion/expansion-panel-content.ts

+  selector: 'ng-template[matExpansionPanelContent]'
+})
+export class MatExpansionPanelContent {
+  constructor(public _template: TemplateRef<any>) {}


Just a quick question: Don't you use _ for private variables only?

They also use it for "internal use only" properties

Currently the expansion panel renders all of its content on load which can be expensive, considering that all of it won't be visible. These changes allow for some of the content to be rendered lazily via the `matExpansionPanelContent` directive. Fixes angular#8230.

angular-automatic-lock-bot · 2019-09-07T23:01:27Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

crisbeto added the pr: needs review label Nov 6, 2017

crisbeto assigned josephperrott Nov 6, 2017

crisbeto requested review from devversion, jelbourn and josephperrott as code owners November 6, 2017 09:25

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Nov 6, 2017

josephperrott reviewed Nov 6, 2017

View reviewed changes

jelbourn reviewed Nov 6, 2017

View reviewed changes

willshowell mentioned this pull request Nov 8, 2017

[Accordion] Rendering many expansion panels is slow #8287

Open

crisbeto force-pushed the 8230/expansion-panel-lazy-load branch from 9dd9c2c to ac73028 Compare November 12, 2017 11:31

crisbeto changed the title ~~perf(expansion-panel): render content lazily~~ feat(expansion-panel): allow for content to be rendered lazily Nov 12, 2017

crisbeto force-pushed the 8230/expansion-panel-lazy-load branch 2 times, most recently from 8d4354c to 932c58e Compare November 12, 2017 11:33

jelbourn reviewed Nov 14, 2017

View reviewed changes

crisbeto force-pushed the 8230/expansion-panel-lazy-load branch from 932c58e to 5677639 Compare November 14, 2017 18:05

jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Nov 14, 2017

jelbourn approved these changes Nov 14, 2017

View reviewed changes

rafaelss95 reviewed Nov 17, 2017

View reviewed changes

crisbeto force-pushed the 8230/expansion-panel-lazy-load branch 2 times, most recently from f6ef924 to 1d2c440 Compare November 20, 2017 20:31

crisbeto force-pushed the 8230/expansion-panel-lazy-load branch from 1d2c440 to 65a3a85 Compare November 23, 2017 22:03

crisbeto added the target: minor This PR is targeted for the next minor release label Dec 6, 2017

andrewseguin merged commit 60ba0a7 into angular:master Dec 13, 2017

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019

feat(expansion-panel): allow for content to be rendered lazily #8243

feat(expansion-panel): allow for content to be rendered lazily #8243

Uh oh!

Conversation

crisbeto commented Nov 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bardiel Nov 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crisbeto commented Nov 12, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crisbeto commented Nov 14, 2017

Uh oh!

jelbourn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

angular-automatic-lock-bot bot commented Sep 7, 2019

Uh oh!

Uh oh!

crisbeto commented Nov 6, 2017 •

edited

Loading

bardiel Nov 6, 2017 •

edited

Loading