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

fix(module:breadcrumb): fix input boolean and router event not caught error #3185

Merged
merged 5 commits into from
May 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 0 additions & 4 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ yarn.lock

# IDE - VSCode
.vscode/*
!.vscode/settings.json
!.vscode/tasks.json
!.vscode/launch.json
!.vscode/extensions.json

# misc
/.sass-cache
Expand Down
2 changes: 1 addition & 1 deletion components/breadcrumb/demo/auto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Component } from '@angular/core';
selector: 'nz-demo-breadcrumb-auto',
template: `
<nz-breadcrumb [nzAutoGenerate]="true">
Please refer to StackBlitz demo.
Please refer to StackBlitz demo at https://stackblitz.com/edit/ng-zorro-breadcrumb-auto
</nz-breadcrumb>
`
})
Expand Down
12 changes: 12 additions & 0 deletions components/breadcrumb/doc/index.en-US.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,15 @@ Using `[nzAutoGenerate]` by configuring `data` like this:
}
}
```

For lazy loading moduels, you should write `data` in parent module like this:

```ts
{
path: 'first',
loadChildren: './first/first.module#FirstModule',
data: {
breadcrumb: 'First'
},
}
```
14 changes: 13 additions & 1 deletion components/breadcrumb/doc/index.zh-CN.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,22 @@ import { NzBreadCrumbModule } from 'ng-zorro-antd';

```ts
{
path: '/path',
path: 'path',
component: SomeComponent,
data: {
breadcrumb: 'Display Name'
}
}
```

对于懒加载路由,应该在父层路由写 `data`:

```ts
{
path: 'first',
loadChildren: './first/first.module#FirstModule',
data: {
breadcrumb: 'First'
},
}
```
2 changes: 1 addition & 1 deletion components/breadcrumb/nz-breadcrumb-item.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
<ng-container *nzStringTemplateOutlet="nzBreadCrumbComponent.nzSeparator">
{{ nzBreadCrumbComponent.nzSeparator }}
</ng-container>
</span>
</span>
42 changes: 28 additions & 14 deletions components/breadcrumb/nz-breadcrumb.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ import {
TemplateRef,
ViewEncapsulation
} from '@angular/core';
import { ActivatedRoute, Params, PRIMARY_OUTLET, Router } from '@angular/router';
import { ActivatedRoute, NavigationEnd, Params, PRIMARY_OUTLET, Router } from '@angular/router';
import { Subject } from 'rxjs';
import { takeUntil } from 'rxjs/operators';
import { startWith } from 'rxjs/internal/operators/startWith';
import { filter, takeUntil } from 'rxjs/operators';

import { InputBoolean } from 'ng-zorro-antd/core';

Expand Down Expand Up @@ -60,7 +61,7 @@ export class NzBreadCrumbComponent implements OnInit, OnDestroy {
constructor(
private injector: Injector,
private ngZone: NgZone,
private cd: ChangeDetectorRef,
private cdr: ChangeDetectorRef,
elementRef: ElementRef,
renderer: Renderer2
) {
Expand All @@ -69,15 +70,7 @@ export class NzBreadCrumbComponent implements OnInit, OnDestroy {

ngOnInit(): void {
if (this.nzAutoGenerate) {
try {
const activatedRoute = this.injector.get(ActivatedRoute);
activatedRoute.url.pipe(takeUntil(this.destroy$)).subscribe(() => {
this.breadcrumbs = this.getBreadcrumbs(activatedRoute.root);
this.cd.markForCheck();
});
} catch (e) {
throw new Error('[NG-ZORRO] You should import RouterModule if you want to use NzAutoGenerate');
}
this.registerRouterChange();
}
}

Expand All @@ -88,6 +81,7 @@ export class NzBreadCrumbComponent implements OnInit, OnDestroy {

navigate(url: string, e: MouseEvent): void {
e.preventDefault();

this.ngZone
.run(() =>
this.injector
Expand All @@ -98,6 +92,25 @@ export class NzBreadCrumbComponent implements OnInit, OnDestroy {
.then();
}

private registerRouterChange(): void {
try {
const router = this.injector.get(Router);
const activatedRoute = this.injector.get(ActivatedRoute);
router.events
.pipe(
filter(e => e instanceof NavigationEnd),
takeUntil(this.destroy$),
startWith(true) // Trigger initial render.
)
.subscribe(() => {
this.breadcrumbs = this.getBreadcrumbs(activatedRoute.root);
this.cdr.markForCheck();
});
} catch (e) {
throw new Error('[NG-ZORRO] You should import RouterModule if you want to use `NzAutoGenerate`');
}
}

private getBreadcrumbs(
route: ActivatedRoute,
url: string = '',
Expand All @@ -114,10 +127,11 @@ export class NzBreadCrumbComponent implements OnInit, OnDestroy {
// Parse this layer and generate a breadcrumb item.
const routeURL: string = child.snapshot.url.map(segment => segment.path).join('/');
const nextUrl = url + `/${routeURL}`;
const breadcrumbLabel = child.snapshot.data[NZ_ROUTE_DATA_BREADCRUMB];
// If have data, go to generate a breadcrumb for it.
if (child.snapshot.data.hasOwnProperty(NZ_ROUTE_DATA_BREADCRUMB)) {
if (routeURL && breadcrumbLabel) {
const breadcrumb: BreadcrumbOption = {
label: child.snapshot.data[NZ_ROUTE_DATA_BREADCRUMB] || 'Breadcrumb',
label: breadcrumbLabel,
params: child.snapshot.params,
url: nextUrl
};
Expand Down
71 changes: 44 additions & 27 deletions components/breadcrumb/nz-breadcrumb.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { By } from '@angular/platform-browser';
import { Router, Routes } from '@angular/router';
import { RouterTestingModule } from '@angular/router/testing';

import { dispatchMouseEvent } from 'ng-zorro-antd/core';
// import { dispatchMouseEvent } from 'ng-zorro-antd/core';
import { NzIconTestModule } from 'ng-zorro-antd/icon/testing';

import { NzDemoBreadcrumbBasicComponent } from './demo/basic';
Expand Down Expand Up @@ -84,53 +84,52 @@ describe('breadcrumb', () => {
let fixture: ComponentFixture<NzBreadcrumbAutoGenerateDemoComponent>;
let router: Router;
let breadcrumb: DebugElement;
let items: DebugElement[];

// TODO: pending this test because of Angular's bug: https://github.com/angular/angular/issues/25837
xit('should auto generating work', fakeAsync(() => {
it('should auto generating work', fakeAsync(() => {
TestBed.configureTestingModule({
imports: [CommonModule, NzBreadCrumbModule, RouterTestingModule.withRoutes(routes)],
declarations: [NzBreadcrumbAutoGenerateDemoComponent, NzBreadcrumbNullComponent]
}).compileComponents();

fixture = TestBed.createComponent(NzBreadcrumbAutoGenerateDemoComponent);
breadcrumb = fixture.debugElement.query(By.directive(NzBreadCrumbComponent));

fixture.ngZone!.run(() => {
router = TestBed.get(Router);
router.initialNavigation();
// Generate breadcrumb items.
router.navigate(['one', 'two', 'three', 'four']);
fixture.detectChanges();
flush();
fixture.detectChanges();
items = fixture.debugElement.queryAll(By.directive(NzBreadCrumbItemComponent));

// Should generate 2 breadcrumbs when reaching out of the `data` scope.
router.navigate(['one', 'two', 'three', 'four']);
flushFixture(fixture);
expect(breadcrumb.componentInstance.breadcrumbs.length).toBe(2);
dispatchMouseEvent(items[1].nativeElement.querySelector('a'), 'click');

// TODO: pending this test because of Angular's bug: https://github.com/angular/angular/issues/25837
// const items = fixture.debugElement.queryAll(By.directive(NzBreadCrumbItemComponent));
// dispatchMouseEvent(items[1].nativeElement.querySelector('a'), 'click');
// flushFixture(fixture);
// expect(breadcrumb.componentInstance.breadcrumbs.length).toBe(1);

// Should generate breadcrumbs correctly.
router.navigate(['one', 'two', 'three']);
fixture.detectChanges();
flush();
fixture.detectChanges();
flushFixture(fixture);
expect(breadcrumb.componentInstance.breadcrumbs.length).toBe(2);
router.navigate(['one', 'two']);
fixture.detectChanges();
flush();
fixture.detectChanges();
flushFixture(fixture);
expect(breadcrumb.componentInstance.breadcrumbs.length).toBe(1);
router.navigate(['one']);
fixture.detectChanges();
flush();
fixture.detectChanges();

// Shouldn't generate breadcrumb at all.
router.navigate(['one']);
flushFixture(fixture);
expect(breadcrumb.componentInstance.breadcrumbs.length).toBe(0);
});
}));

it('should raise error when RouterModule is not included', fakeAsync(() => {
TestBed.configureTestingModule({
imports: [NzBreadCrumbModule], // no RouterTestingModule
imports: [NzBreadCrumbModule],
declarations: [NzBreadcrumbAutoGenerateErrorDemoComponent]
});

expect(() => {
TestBed.compileComponents();
fixture = TestBed.createComponent(NzBreadcrumbAutoGenerateErrorDemoComponent);
Expand All @@ -140,6 +139,13 @@ describe('breadcrumb', () => {
});
});

// tslint:disable-next-line no-any
function flushFixture(fixture: ComponentFixture<any>): void {
fixture.detectChanges();
flush();
fixture.detectChanges();
}

@Component({
selector: 'nz-breadcrumb-auto-generate-demo',
template: `
Expand All @@ -157,34 +163,45 @@ class NzBreadcrumbAutoGenerateDemoComponent {}
class NzBreadcrumbAutoGenerateErrorDemoComponent {}

@Component({
selector: 'nz-breadcrumb-null',
template: ''
selector: 'nz-breadcrumb-empty',
template: 'empty'
})
class NzBreadcrumbNullComponent {}

const routes: Routes = [
{
path: 'one',
component: NzBreadcrumbAutoGenerateDemoComponent,
data: {
breadcrumb: ''
},
children: [
{
path: 'two',
component: NzBreadcrumbNullComponent,
data: { breadcrumb: 'Layer 2' },
data: {
breadcrumb: 'Layer 2'
},
children: [
{
path: 'three',
component: NzBreadcrumbNullComponent,
data: { breadcrumb: '' },
data: {
breadcrumb: 'Layer 3'
},
children: [
{
path: 'four',
component: NzBreadcrumbNullComponent
component: NzBreadcrumbNullComponent,
data: {
breadcrumb: ''
}
}
]
}
]
},
// Should only work for the primary outlet.
{
path: 'two',
outlet: 'notprimary',
Expand Down
5 changes: 4 additions & 1 deletion components/icon/testing/nz-icon-test.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ const antDesignIcons = AllIcons as {
[key: string]: IconDefinition;
};

const icons: IconDefinition[] = Object.keys(antDesignIcons).map(key => antDesignIcons[key]);
const icons: IconDefinition[] = Object.keys(antDesignIcons).map(key => {
const i = antDesignIcons[key];
return i;
});

/**
* Include this module in every testing spec, except `nz-icon.spec.ts`.
Expand Down