Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Feb 8, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/demo-app/demo-app/demo-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ import {TableDemo} from '../table/table-demo';
import {ScreenTypeDemo} from '../screen-type/screen-type-demo';
import {LayoutModule} from '@angular/cdk/layout';
import {TableHeaderDemo} from '../table/table-header-demo';
import {FoggyTabContent, RainyTabContent, SunnyTabContent, TabsDemo} from '../tabs/tabs-demo';
import {
FoggyTabContent, RainyTabContent, SunnyTabContent, TabsDemo, Counter
} from '../tabs/tabs-demo';
import {ToolbarDemo} from '../toolbar/toolbar-demo';
import {TooltipDemo} from '../tooltip/tooltip-demo';
import {TypographyDemo} from '../typography/typography-demo';
Expand Down Expand Up @@ -86,6 +88,7 @@ import {DEMO_APP_ROUTES} from './routes';
ExpansionDemo,
FocusOriginDemo,
FoggyTabContent,
Counter,
GesturesDemo,
GridListDemo,
Home,
Expand Down
14 changes: 14 additions & 0 deletions src/demo-app/tabs/tabs-demo.html
Original file line number Diff line number Diff line change
Expand Up @@ -288,3 +288,17 @@ <h1>Tabs with autosize textarea</h1>
</div>
</mat-tab>
</mat-tab-group>

<h1>Lazy Loaded Tabs</h1>
<mat-tab-group>
<mat-tab label="First">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs consistent two-space indentation

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?)

<ng-template matTabContent>
<counter></counter>
</ng-template>
</mat-tab>
<mat-tab label="Second">
<ng-template matTabContent>
<counter></counter>
</ng-template>
</mat-tab>
</mat-tab-group>
13 changes: 13 additions & 0 deletions src/demo-app/tabs/tabs-demo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,16 @@ export class RainyTabContent {}
template: 'This is the routed body of the foggy tab.',
})
export class FoggyTabContent {}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

count variable is not defined

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

})
export class Counter {
count = 0;
ngOnInit() {
this.count++;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

console.log('Counting...', this.count);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
}
1 change: 1 addition & 0 deletions src/lib/tabs/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@ export {MatTabLabelWrapper} from './tab-label-wrapper';
export {MatTab} from './tab';
export {MatTabLabel} from './tab-label';
export {MatTabNav, MatTabLink} from './tab-nav-bar/index';
export {MdTabContent} from './tab-content';
6 changes: 5 additions & 1 deletion src/lib/tabs/tab-body.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
ComponentFactoryResolver,
ViewContainerRef,
forwardRef,
ViewChild,
} from '@angular/core';
import {
trigger,
Expand All @@ -31,7 +32,7 @@ import {
transition,
AnimationEvent,
} from '@angular/animations';
import {TemplatePortal, CdkPortalOutlet} from '@angular/cdk/portal';
import {TemplatePortal, CdkPortalOutlet, PortalHostDirective} from '@angular/cdk/portal';
import {Directionality, Direction} from '@angular/cdk/bidi';
import {Subscription} from 'rxjs/Subscription';

Expand Down Expand Up @@ -153,6 +154,9 @@ export class MatTabBody implements OnInit {
/** Event emitted when the tab completes its animation towards the center. */
@Output() _onCentered: EventEmitter<void> = new EventEmitter<void>(true);

/** The portal host inside of this container into which the tab body content will be loaded. */
@ViewChild(PortalHostDirective) _portalHost: PortalHostDirective;

/** The tab body content to display. */
@Input('content') _content: TemplatePortal<any>;

Expand Down
14 changes: 14 additions & 0 deletions src/lib/tabs/tab-content.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

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

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs some doc describing the use and purpose of this directive

export class MdTabContent {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MatTabContent =P

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some more MdTabContents lying around 😉

constructor(public template: TemplateRef<any>) { }
}
34 changes: 34 additions & 0 deletions src/lib/tabs/tab-group.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ describe('MatTabGroup', () => {
AsyncTabsTestApp,
DisabledTabsTestApp,
TabGroupWithSimpleApi,
TemplateTabs,
],
});

Expand Down Expand Up @@ -375,6 +376,23 @@ describe('MatTabGroup', () => {
});
});

describe('lazy loaded tabs', () => {
it('should lazy load the second tab', async () => {
let fixture = TestBed.createComponent(TemplateTabs);
fixture.detectChanges();

let tabLabel = fixture.debugElement.queryAll(By.css('.mat-tab-label'))[1];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

tabLabel.nativeElement.click();
fixture.detectChanges();

fixture.whenStable().then(() => {
fixture.detectChanges();
let child = fixture.debugElement.query(By.css('.child'));
expect(child.nativeElement).toBeDefined();
});
});
});

/**
* Checks that the `selectedIndex` has been updated; checks that the label and body have their
* respective `active` classes
Expand Down Expand Up @@ -602,3 +620,19 @@ class TabGroupWithSimpleApi {
})
class NestedTabs {}

@Component({
selector: 'template-tabs',
template: `
<mat-tab-group>
<mat-tab label="One">
Eager
</mat-tab>
<mat-tab label="Two">
<ng-template matTabContent>
<div class="child">Hi</div>
</ng-template>
</mat-tab>
</mat-tab-group>
`,
})
class TemplateTabs {}
9 changes: 7 additions & 2 deletions src/lib/tabs/tab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No spaces between brackets?



// Boilerplate for applying mixins to MatTab.
Expand All @@ -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 */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@ContentChild(MdTabContent, {read: TemplateRef}) _explicitContent: TemplateRef<any>;

/** Template inside the MatTab view that contains an <ng-content>. */
@ViewChild(TemplateRef) _content: TemplateRef<any>;
@ViewChild(TemplateRef) _implicitContent: TemplateRef<any>;

/** The plain text label for the tab, used when there is no template label. */
@Input('label') textLabel: string = '';
Expand Down Expand Up @@ -102,6 +106,7 @@ export class MatTab extends _MatTabMixinBase implements OnInit, CanDisable, OnCh
}

ngOnInit(): void {
this._contentPortal = new TemplatePortal(this._content, this._viewContainerRef);
this._contentPortal = new TemplatePortal(
this._explicitContent || this._implicitContent, this._viewContainerRef);
}
}
5 changes: 4 additions & 1 deletion src/lib/tabs/tabs-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {MatTabHeader} from './tab-header';
import {MatTabLabel} from './tab-label';
import {MatTabLabelWrapper} from './tab-label-wrapper';
import {MatTabLink, MatTabNav} from './tab-nav-bar/tab-nav-bar';
import {MdTabContent} from './tab-content';


@NgModule({
Expand All @@ -39,6 +40,7 @@ import {MatTabLink, MatTabNav} from './tab-nav-bar/tab-nav-bar';
MatTab,
MatTabNav,
MatTabLink,
MdTabContent,
],
declarations: [
MatTabGroup,
Expand All @@ -50,7 +52,8 @@ import {MatTabLink, MatTabNav} from './tab-nav-bar/tab-nav-bar';
MatTabLink,
MatTabBody,
MatTabBodyPortal,
MatTabHeader
MatTabHeader,
MdTabContent,
],
providers: [VIEWPORT_RULER_PROVIDER],
})
Expand Down
24 changes: 24 additions & 0 deletions src/lib/tabs/tabs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

will initalize the child components but not inject them into the DOM
until the tab is activated.

If the tab contains several complex child components, it is advised
to lazy load the tab's content. Tab contents can be lazy loaded by
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

declaring the body in a `ng-template` with the `matTabContent` attribute.

```html
<md-tab-group>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Md or Mat?

<md-tab label="First">
<ng-template matTabContent>
The First Content
</ng-template>
</md-tab>
<md-tab label="Second">
<ng-template matTabContent>
The Second Content
</ng-template>
</md-tab>
</md-tab-group>
```

### Accessibility
Tabs without text or labels should be given a meaningful label via `aria-label` or
`aria-labelledby`. For `MatTabNav`, the `<nav>` element should have a label as well.
Expand Down