From fd43ec562e1bfecd926e1ac7f4fad00bf8a2032d Mon Sep 17 00:00:00 2001 From: Wendell Date: Fri, 17 May 2019 23:51:55 +0800 Subject: [PATCH] fix(module:breadcrumb): fix input boolean and router event not caught error (#3185) * fix(module:breadcrumb): fix input boolean and lazy module slash docs: add link fix: fix breadcrumb not updated fix: typo * test: add test * chore: cleanup code * chore: rollback * chore: remove vscode close #3186 --- .gitignore | 4 -- components/breadcrumb/demo/auto.ts | 2 +- components/breadcrumb/doc/index.en-US.md | 12 ++++ components/breadcrumb/doc/index.zh-CN.md | 14 +++- .../nz-breadcrumb-item.component.html | 2 +- .../breadcrumb/nz-breadcrumb.component.ts | 42 +++++++---- components/breadcrumb/nz-breadcrumb.spec.ts | 71 ++++++++++++------- .../icon/testing/nz-icon-test.module.ts | 5 +- 8 files changed, 103 insertions(+), 49 deletions(-) diff --git a/.gitignore b/.gitignore index 7ef42b05af7..2e9d0e40ed1 100644 --- a/.gitignore +++ b/.gitignore @@ -31,10 +31,6 @@ yarn.lock # IDE - VSCode .vscode/* -!.vscode/settings.json -!.vscode/tasks.json -!.vscode/launch.json -!.vscode/extensions.json # misc /.sass-cache diff --git a/components/breadcrumb/demo/auto.ts b/components/breadcrumb/demo/auto.ts index 2c9bcf996d8..3ab6aec05bb 100644 --- a/components/breadcrumb/demo/auto.ts +++ b/components/breadcrumb/demo/auto.ts @@ -4,7 +4,7 @@ import { Component } from '@angular/core'; selector: 'nz-demo-breadcrumb-auto', template: ` - Please refer to StackBlitz demo. + Please refer to StackBlitz demo at https://stackblitz.com/edit/ng-zorro-breadcrumb-auto ` }) diff --git a/components/breadcrumb/doc/index.en-US.md b/components/breadcrumb/doc/index.en-US.md index 9d18ca74791..ca8c30228c6 100755 --- a/components/breadcrumb/doc/index.en-US.md +++ b/components/breadcrumb/doc/index.en-US.md @@ -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' + }, +} +``` diff --git a/components/breadcrumb/doc/index.zh-CN.md b/components/breadcrumb/doc/index.zh-CN.md index e95b45f1937..e2ad059e88f 100755 --- a/components/breadcrumb/doc/index.zh-CN.md +++ b/components/breadcrumb/doc/index.zh-CN.md @@ -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' + }, +} +``` diff --git a/components/breadcrumb/nz-breadcrumb-item.component.html b/components/breadcrumb/nz-breadcrumb-item.component.html index 4da7dc3112b..f86c9a73ab9 100644 --- a/components/breadcrumb/nz-breadcrumb-item.component.html +++ b/components/breadcrumb/nz-breadcrumb-item.component.html @@ -5,4 +5,4 @@ {{ nzBreadCrumbComponent.nzSeparator }} - \ No newline at end of file + diff --git a/components/breadcrumb/nz-breadcrumb.component.ts b/components/breadcrumb/nz-breadcrumb.component.ts index 152e6cf25cd..0945577f8f7 100755 --- a/components/breadcrumb/nz-breadcrumb.component.ts +++ b/components/breadcrumb/nz-breadcrumb.component.ts @@ -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'; @@ -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 ) { @@ -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(); } } @@ -88,6 +81,7 @@ export class NzBreadCrumbComponent implements OnInit, OnDestroy { navigate(url: string, e: MouseEvent): void { e.preventDefault(); + this.ngZone .run(() => this.injector @@ -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 = '', @@ -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 }; diff --git a/components/breadcrumb/nz-breadcrumb.spec.ts b/components/breadcrumb/nz-breadcrumb.spec.ts index a88aa3c1b1e..dc32f8c8540 100644 --- a/components/breadcrumb/nz-breadcrumb.spec.ts +++ b/components/breadcrumb/nz-breadcrumb.spec.ts @@ -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'; @@ -84,53 +84,52 @@ describe('breadcrumb', () => { let fixture: ComponentFixture; 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); @@ -140,6 +139,13 @@ describe('breadcrumb', () => { }); }); +// tslint:disable-next-line no-any +function flushFixture(fixture: ComponentFixture): void { + fixture.detectChanges(); + flush(); + fixture.detectChanges(); +} + @Component({ selector: 'nz-breadcrumb-auto-generate-demo', template: ` @@ -157,8 +163,8 @@ class NzBreadcrumbAutoGenerateDemoComponent {} class NzBreadcrumbAutoGenerateErrorDemoComponent {} @Component({ - selector: 'nz-breadcrumb-null', - template: '' + selector: 'nz-breadcrumb-empty', + template: 'empty' }) class NzBreadcrumbNullComponent {} @@ -166,25 +172,36 @@ 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', diff --git a/components/icon/testing/nz-icon-test.module.ts b/components/icon/testing/nz-icon-test.module.ts index 22f5cea4297..e1031540f45 100644 --- a/components/icon/testing/nz-icon-test.module.ts +++ b/components/icon/testing/nz-icon-test.module.ts @@ -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`.