From 5a2e2fe3ac30803bedcb3453b8ca7453e0de10b3 Mon Sep 17 00:00:00 2001 From: Jon Rimmer Date: Thu, 13 Jul 2017 12:50:23 +0100 Subject: [PATCH 1/3] fix(uiSref): run update when input properties change --- src/directives/uiSref.ts | 37 +++++++++++++--- test/uiSref/uiSref.spec.ts | 91 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 119 insertions(+), 9 deletions(-) diff --git a/src/directives/uiSref.ts b/src/directives/uiSref.ts index 8825b826f..b9db61304 100644 --- a/src/directives/uiSref.ts +++ b/src/directives/uiSref.ts @@ -71,6 +71,7 @@ export class AnchorUISref { host: { '(click)': 'go()' } }) export class UISref { + /** * `@Input('uiSref')` The name of the state to link to * @@ -78,7 +79,14 @@ export class UISref { * Home * ``` */ - @Input('uiSref') state: string; + @Input('uiSref') + get state(): string { + return this._state; + } + set state(val: string) { + this._state = val; + this.update(); + } /** * `@Input('uiParams')` The parameter values to use (as key/values) @@ -87,7 +95,14 @@ export class UISref { * Book {{ book.name }} * ``` */ - @Input('uiParams') params: any; + @Input('uiParams') + get params(): any { + return this._params; + } + set params(val: any) { + this._params = val; + this.update(); + } /** * `@Input('uiOptions')` The transition options @@ -96,7 +111,14 @@ export class UISref { * Book {{ book.name }} * ``` */ - @Input('uiOptions') options: TransitionOptions; + @Input('uiOptions') + get options(): TransitionOptions { + return this._options; + } + set options(val: TransitionOptions) { + this._options = val; + this.update(); + } /** * An observable (ReplaySubject) of the state this UISref is targeting. @@ -108,6 +130,9 @@ export class UISref { /** @internalapi */ private _statesSub: Subscription; /** @internalapi */ private _router: UIRouter; /** @internalapi */ private _anchorUISref: AnchorUISref; + /** @internalapi */ private _state: string; + /** @internalapi */ private _params: any; + /** @internalapi */ private _options: TransitionOptions; /** @internalapi */ public parent: ParentUIViewInject; constructor( @@ -123,11 +148,11 @@ export class UISref { } /** @internalapi */ - set "uiSref"(val: string) { this.state = val; this.update(); } + set "uiSref"(val: string) { this.state = val; } /** @internalapi */ - set "uiParams"(val: Obj) { this.params = val; this.update(); } + set "uiParams"(val: Obj) { this.params = val; } /** @internalapi */ - set "uiOptions"(val: TransitionOptions) { this.options = val; this.update(); } + set "uiOptions"(val: TransitionOptions) { this.options = val; } ngOnInit() { this._emit = true; diff --git a/test/uiSref/uiSref.spec.ts b/test/uiSref/uiSref.spec.ts index 2dde88b73..4ec09cec5 100644 --- a/test/uiSref/uiSref.spec.ts +++ b/test/uiSref/uiSref.spec.ts @@ -1,26 +1,41 @@ -import { Component, DebugElement } from '@angular/core'; +import { Component, DebugElement, ViewChildren, QueryList } from '@angular/core'; import { async, ComponentFixture, TestBed } from '@angular/core/testing'; import { By } from '@angular/platform-browser'; import { UIRouterModule } from '../../src/uiRouterNgModule'; import { UISref } from '../../src/directives/uiSref'; -import { UIRouter } from '@uirouter/core'; +import { UIRouter, TargetState, TransitionOptions } from '@uirouter/core'; import { Subject } from 'rxjs/Subject'; +import { Subscription } from "rxjs/Subscription"; describe('uiSref', () => { @Component({ template: ` - + ` }) class TestComponent { linkA: string; + linkAParams: any; + linkAOptions: TransitionOptions; targetA: string; linkB: string; + @ViewChildren(UISref) srefs: QueryList; + + get linkASref() { + return this.srefs.first; + } + + get linkBSref() { + return this.srefs.toArray()[1]; + } + constructor() { this.linkA = null; + this.linkAParams = null; + this.linkAOptions = null; this.targetA = ''; this.linkB = ''; } @@ -40,6 +55,7 @@ describe('uiSref', () => { des = fixture.debugElement.queryAll(By.directive(UISref)); }); + it('should not bind "null" string to `href`', () => { expect(des[0].nativeElement.hasAttribute('href')).toBeFalsy(); expect(des[1].nativeElement.hasAttribute('href')).toBeFalsy(); @@ -114,6 +130,75 @@ describe('uiSref', () => { }); }); }); + + describe('when the bound values change', () => { + let fixture: ComponentFixture; + let comp: TestComponent; + let logger: TargetState[]; + let subscription: Subscription; + + beforeEach(() => { + fixture = TestBed.configureTestingModule({ + declarations: [TestComponent], + imports: [UIRouterModule.forRoot({ useHash: true })] + }).createComponent(TestComponent); + fixture.detectChanges(); + comp = fixture.componentInstance; + logger = []; + subscription = comp.linkASref.targetState$.subscribe(evt => logger.push(evt)); + }); + + afterEach(() => { + subscription.unsubscribe(); + }); + + describe('when the uiSref is empty', () => { + it('should emit an empty target state event', () =>{ + expect(logger.length).toBe(1); + expect(logger[0].name()).toBeNull(); + }); + }) + + describe('when the target state changes', () => { + beforeEach(() => { + comp.linkA = 'stateA'; + fixture.detectChanges(); + }); + + it('should emit an event', () => { + expect(logger.length).toBe(2); + expect(logger[1].name()).toBe('stateA'); + }); + }); + + describe('when the target params change', () => { + const params = { paramA: 'paramA' }; + + beforeEach(() => { + comp.linkAParams = params; + fixture.detectChanges(); + }); + + it('should emit an event', () => { + expect(logger.length).toBe(2); + expect(logger[1].params()).toEqual(params); + }); + }); + + describe('when the transition options change', () => { + const options: TransitionOptions = { custom: 'custom' }; + + beforeEach(() => { + comp.linkAOptions = options; + fixture.detectChanges(); + }); + + it ('should emit an event', () => { + expect(logger.length).toBe(2); + expect(logger[1].options().custom).toEqual(options.custom); + }); + }) + }); }); }); From b64f14d72e2fb6b71a86a3da99bbde14c80d8d5f Mon Sep 17 00:00:00 2001 From: Jon Rimmer Date: Thu, 13 Jul 2017 13:24:42 +0100 Subject: [PATCH 2/3] [uiSref] use ngOnChanges to run update on input changes --- src/directives/uiSref.ts | 38 +++++++++----------------------------- 1 file changed, 9 insertions(+), 29 deletions(-) diff --git a/src/directives/uiSref.ts b/src/directives/uiSref.ts index b9db61304..37aa05911 100644 --- a/src/directives/uiSref.ts +++ b/src/directives/uiSref.ts @@ -1,7 +1,7 @@ /** @ng2api @module directives */ /** */ import { UIRouter, extend, Obj, TransitionOptions, TargetState } from "@uirouter/core"; -import { Directive, Inject, Input, Optional, ElementRef, Renderer2 } from "@angular/core"; +import { Directive, Inject, Input, Optional, ElementRef, Renderer2, OnChanges, SimpleChanges } from "@angular/core"; import { UIView, ParentUIViewInject } from "./uiView"; import { ReplaySubject } from "rxjs/ReplaySubject"; import { Subscription } from "rxjs/Subscription"; @@ -70,7 +70,7 @@ export class AnchorUISref { selector: '[uiSref]', host: { '(click)': 'go()' } }) -export class UISref { +export class UISref implements OnChanges { /** * `@Input('uiSref')` The name of the state to link to @@ -79,14 +79,7 @@ export class UISref { * Home * ``` */ - @Input('uiSref') - get state(): string { - return this._state; - } - set state(val: string) { - this._state = val; - this.update(); - } + @Input('uiSref') state: string; /** * `@Input('uiParams')` The parameter values to use (as key/values) @@ -95,14 +88,7 @@ export class UISref { * Book {{ book.name }} * ``` */ - @Input('uiParams') - get params(): any { - return this._params; - } - set params(val: any) { - this._params = val; - this.update(); - } + @Input('uiParams') params: any; /** * `@Input('uiOptions')` The transition options @@ -111,14 +97,7 @@ export class UISref { * Book {{ book.name }} * ``` */ - @Input('uiOptions') - get options(): TransitionOptions { - return this._options; - } - set options(val: TransitionOptions) { - this._options = val; - this.update(); - } + @Input('uiOptions') options: TransitionOptions; /** * An observable (ReplaySubject) of the state this UISref is targeting. @@ -130,9 +109,6 @@ export class UISref { /** @internalapi */ private _statesSub: Subscription; /** @internalapi */ private _router: UIRouter; /** @internalapi */ private _anchorUISref: AnchorUISref; - /** @internalapi */ private _state: string; - /** @internalapi */ private _params: any; - /** @internalapi */ private _options: TransitionOptions; /** @internalapi */ public parent: ParentUIViewInject; constructor( @@ -159,6 +135,10 @@ export class UISref { this.update(); } + ngOnChanges(changes: SimpleChanges): void { + this.update(); + } + ngOnDestroy() { this._emit = false; this._statesSub.unsubscribe(); From 763d0fb1d5fba7be533457dc32760d6cce5adcdf Mon Sep 17 00:00:00 2001 From: Jon Rimmer Date: Thu, 13 Jul 2017 13:27:04 +0100 Subject: [PATCH 3/3] [uiSref] add update call back to internal API setters --- src/directives/uiSref.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/directives/uiSref.ts b/src/directives/uiSref.ts index 37aa05911..598f3ad93 100644 --- a/src/directives/uiSref.ts +++ b/src/directives/uiSref.ts @@ -124,11 +124,11 @@ export class UISref implements OnChanges { } /** @internalapi */ - set "uiSref"(val: string) { this.state = val; } + set "uiSref"(val: string) { this.state = val; this.update(); } /** @internalapi */ - set "uiParams"(val: Obj) { this.params = val; } + set "uiParams"(val: Obj) { this.params = val; this.update(); } /** @internalapi */ - set "uiOptions"(val: TransitionOptions) { this.options = val; } + set "uiOptions"(val: TransitionOptions) { this.options = val; this.update(); } ngOnInit() { this._emit = true;