feat(tabs): add ability to lazy load tab content #8921

amcdnl · 2017-12-10T19:52:23Z

This PR adds the ability to lazy load tabs, it is a new cut of #6832 on latest code.

donroyco · 2017-12-11T07:36:44Z

src/lib/tabs/tab.ts

@@ -24,6 +24,7 @@ import {
 import {CanDisable, mixinDisabled} from '@angular/material/core';
 import {Subject} from 'rxjs/Subject';
 import {MatTabLabel} from './tab-label';
+import { MdTabContent } from './tab-content';


No spaces between brackets?

josephperrott

LGTM

jelbourn · 2017-12-12T17:10:27Z

I'd like @andrewseguin to take a look at this as well

andrewseguin · 2017-12-12T23:04:21Z

src/demo-app/tabs/tabs-demo.ts

+  count = 0;
+  ngOnInit() {
+    this.count++;
+    console.log('Counting...', this.count);


Can you provide some comments about why we are console.loging and what the use of this Counter is?

andrewseguin · 2017-12-12T23:05:01Z

src/demo-app/tabs/tabs-demo.html

+
+<h1>Lazy Loaded Tabs</h1>
+ <mat-tab-group>
+  <mat-tab label="First">


Needs consistent two-space indentation

andrewseguin · 2017-12-12T23:07:21Z

src/lib/tabs/tabs.md

@@ -80,6 +80,30 @@ The `tab-nav-bar` is not tied to any particular router; it works with normal `<a
 the `active` property to determine which tab is currently active. The corresponding
 `<router-outlet>` can be placed anywhere in the view.

+## Lazy Loading
+By default, the tab contents are eagerly loaded. Eagerly loaded tabs


Should we mention here about how lazy-loading resolves issues with components that expect to be in the DOM while they setup? e.g. expansion panels and nested ink bars.

I suspect we'll want a place to point people to as we continue to get issues about this

Preferably yes. I'm waiting on a fix to #5269, to render expansion panels within tabs....as are others.

I'll probably use lazy loading in combination with a the Progress Spinner component since I render many expansion panels within a tab (see EDIT view on JSONSchema.Net).

Eagerly awaiting this feature :)

@andrewseguin - Added, let me know what u think?

andrewseguin · 2017-12-12T23:08:38Z

src/lib/tabs/tab-content.ts

+
+import {Directive, TemplateRef} from '@angular/core';
+
+@Directive({selector: '[matTabContent]'})


Needs some doc describing the use and purpose of this directive

andrewseguin · 2017-12-12T23:12:27Z

src/lib/tabs/tab-content.ts

+import {Directive, TemplateRef} from '@angular/core';
+
+@Directive({selector: '[matTabContent]'})
+export class MdTabContent {


MatTabContent =P

There are some more MdTabContents lying around 😉

andrewseguin · 2017-12-12T23:13:26Z

src/lib/tabs/tab-group.spec.ts

+      let fixture = TestBed.createComponent(TemplateTabs);
+      fixture.detectChanges();
+
+      let tabLabel = fixture.debugElement.queryAll(By.css('.mat-tab-label'))[1];


Rename to secondTabLabel to make it more obvious what we are doing here

andrewseguin · 2017-12-12T23:18:59Z

src/lib/tabs/tab.ts

@@ -45,8 +46,11 @@ export class MatTab extends _MatTabMixinBase implements OnInit, CanDisable, OnCh
  /** Content for the tab label given by <ng-template mat-tab-label>. */
  @ContentChild(MatTabLabel) templateLabel: MatTabLabel;

+  /** User provided template that we are going to use instead of implicitContent template */


"Template provided in the tab content that will be used if present, used to enable lazy-loading"

donroyco · 2017-12-13T07:53:15Z

src/lib/tabs/tabs.md

+declaring the body in a `ng-template` with the `matTabContent` attribute.
+
+```html
+<md-tab-group>


Md or Mat?

amcdnl · 2017-12-16T21:57:55Z

@andrewseguin - ready for round 2!

amcdnl · 2018-01-21T17:17:07Z

@josephperrott - ready for merge.

jelbourn · 2018-01-22T19:35:33Z

@andrewseguin this PR still needs your approval

andrewseguin

LGTM, thanks for your patience on the review. I'm excited to have this in as a solution for a lot of our issues open on tabs.

Add the merge ready label once the comments are addressed. Just minor nits that shouldn't affect the actual logic

andrewseguin · 2018-01-22T20:07:42Z

src/demo-app/tabs/tabs-demo.ts

+ export class Counter {
+  count = 0;
+  ngOnInit() {
+    this.count++;


Not totally sure what the count represents, shouldn't this be 1 for any component regardless of how it is loaded?

To be honest, Its been so long I forgot what I was using it for too. I removed it.

andrewseguin · 2018-01-22T20:09:36Z

src/lib/tabs/tabs.md

+
+If the tab contains several complex child components or the tab's contents
+rely on DOM calculations during initialization, it is advised
+to lazy load the tab's content. Tab contents can be lazy loaded by


I think this statement should stand out a bit more, even just by making it a new paragraph:

"Tab contents can be lazy loaded by declaring the body in a ng-template with the matTabContent attribute."

andrewseguin · 2018-01-22T20:12:15Z

src/demo-app/tabs/tabs-demo.html

+
+<h1>Lazy Loaded Tabs</h1>
+<mat-tab-group>
+  <mat-tab label="First">


Would be neat to see a third tab that doesn't use lazy-loading so we can see the difference somehow (e.g. the content of Counter component should have the time it was loaded?)

aalbericio · 2018-01-26T12:42:01Z

This PR is a must for our application 👍

Will it make it for next version?

amcdnl · 2018-01-26T12:53:43Z

@andrewseguin - resolved all comments.

andrewseguin · 2018-01-26T17:53:48Z

src/demo-app/tabs/tabs-demo.ts

+ })
+ export class Counter {
+  ngOnInit() {
+    console.log('Tab Loaded:');


Smallest nit ever: remove the colon ;)

tinayuangao · 2018-01-30T19:01:53Z

src/demo-app/tabs/tabs-demo.ts

+@Component({
+  moduleId: module.id,
+  selector: 'counter',
+  template: `<span>{{count}}</span>`


count variable is not defined

@amcdnl this is a presubmit error in Google, so if you can fix that it should unblock moving forward

* feat(tabs): add ability to lazy load tab content * fix: lint * chore: nit * chore: pr feedback * fix: presubmit error

martinsafsten · 2018-02-14T15:22:57Z

Just tried this out on the 6.0.0-beta-0 version. It seems to load the content as expected.

The problem I'm having is that the content is being recreated every time you navigate to the tab. I've seen this exact behavior before when using a ng-template in combination with a *ngIf to toggle it. Seems like a similar situation here.

I would want it to lazy load it when the tab is created, not when navigated to...

aalbericio · 2018-02-14T15:54:53Z

I'd prefer to set that behaviour externally: something like a "creationPolicy" param with values like:

CREATE_ALL
CREATE_CURRENT
RECYCLE
...

Best.

julianobrasil · 2018-02-14T16:31:28Z

@Marfyy and @aalbericio, the original issue (referring to #5269) was that to get animations to work inside a hidden tab some hacking was needed. IMO, this is fulfilled, as no hacking is necessary anymore.

Maybe starting another issue asking to improve the lazy load feature is the best you can do now. This issue (again, referring to #5269) is likely to be closed as soon as 6.0.0 is officially released.

aalbericio · 2018-02-14T16:33:43Z

Of course! That's more like a feature request. You're right! ;)

martinsafsten · 2018-02-14T16:34:25Z

agreed :)

waterskier2007 · 2018-05-08T14:18:07Z

What version is this available in? Any 5.x.x versions or not until 6?

julianobrasil · 2018-05-09T10:42:14Z

Not until 6.

angular-automatic-lock-bot · 2019-09-08T22:02:03Z

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.}

feat(tabs): add ability to lazy load tab content

4b19c58

amcdnl self-assigned this Dec 10, 2017

amcdnl requested review from jelbourn and josephperrott December 10, 2017 19:52

amcdnl requested a review from andrewseguin as a code owner December 10, 2017 19:52

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

amcdnl mentioned this pull request Dec 10, 2017

feat(tabs): adds ability for lazy loaded tabs #6832

Closed

donroyco reviewed Dec 11, 2017

View reviewed changes

fix: lint

17715ec

josephperrott approved these changes Dec 11, 2017

View reviewed changes

josephperrott added pr: lgtm action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release labels Dec 11, 2017

jelbourn removed the action: merge The PR is ready for merge by the caretaker label Dec 12, 2017

andrewseguin mentioned this pull request Dec 12, 2017

SortHeader indicator in an inactive MatTab seems broken #8856

Closed

andrewseguin reviewed Dec 12, 2017

View reviewed changes

donroyco reviewed Dec 13, 2017

View reviewed changes

amcdnl added 2 commits December 16, 2017 15:54

chore: nit

36f2dd2

Merge branch 'master' into lazy-tabs

02241d9

Merge branch 'master' into lazy-tabs

3b0e4e8

andrewseguin approved these changes Jan 22, 2018

View reviewed changes

amcdnl added 2 commits January 26, 2018 06:52

chore: pr feedback

b66b2d7

Merge branch 'master' into lazy-tabs

fc450ed

julianobrasil mentioned this pull request Jan 26, 2018

[Tabs] Hidden tabs don't render expansion panels correctly #5269

Closed

andrewseguin reviewed Jan 26, 2018

View reviewed changes

andrewseguin added the action: merge The PR is ready for merge by the caretaker label Jan 26, 2018

tinayuangao reviewed Jan 30, 2018

View reviewed changes

andrewseguin mentioned this pull request Jan 30, 2018

[Table] Sort arrows are not rendered properly in tabs #7291

Closed

fix: presubmit error

65e423b

This was referenced Feb 6, 2018

Mat-Vertical-Stepper incompatible with Mat-Tab #9814

Closed

Mat Drawer in unopened Mat Tab #9804

Closed

JustACodeMonkey mentioned this pull request Feb 7, 2018

[Sort] Arrow does not display correctly inside a menu item #9758

Closed

mmalerba merged commit 6feaf62 into angular:master Feb 8, 2018

tinayuangao pushed a commit that referenced this pull request Feb 9, 2018

feat(tabs): add ability to lazy load tab content (#8921)

00a7ec5

* feat(tabs): add ability to lazy load tab content * fix: lint * chore: nit * chore: pr feedback * fix: presubmit error

amcdnl deleted the lazy-tabs branch February 10, 2018 15:14

josephperrott mentioned this pull request Feb 13, 2018

Expansion panels in tabs not working properly #9834

Closed

amcdnl mentioned this pull request Feb 18, 2018

mat-tab-group doesn't show styles for default tab selected #9540

Closed

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


		import {Directive, TemplateRef} from '@angular/core';

		@Directive({selector: '[matTabContent]'})

feat(tabs): add ability to lazy load tab content #8921

feat(tabs): add ability to lazy load tab content #8921

Conversation

amcdnl commented Dec 10, 2017

Choose a reason for hiding this comment

josephperrott left a comment

Choose a reason for hiding this comment

jelbourn commented Dec 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amcdnl commented Dec 16, 2017

amcdnl commented Jan 21, 2018

jelbourn commented Jan 22, 2018

andrewseguin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aalbericio commented Jan 26, 2018

amcdnl commented Jan 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinsafsten commented Feb 14, 2018

aalbericio commented Feb 14, 2018

julianobrasil commented Feb 14, 2018 • edited Loading

aalbericio commented Feb 14, 2018

martinsafsten commented Feb 14, 2018

waterskier2007 commented May 8, 2018

julianobrasil commented May 9, 2018

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

julianobrasil commented Feb 14, 2018 •

edited

Loading