From 306c074dd7ca3e9bb196042022bd75ad38c248a6 Mon Sep 17 00:00:00 2001 From: Paul Crowder Date: Wed, 1 Feb 2017 13:43:36 -0500 Subject: [PATCH 1/3] Fixed bug where tile was not rendered properly when a parent used OnPush change detection strategy Fixes #324 --- .../tiles/tile-dashboard/fixtures/index.ts | 1 + .../tile-dashboard-fixtures.module.ts | 7 +- ...ile-dashboard-on-push.component.fixture.ts | 74 +++++++++++++++++++ .../fixtures/tile1.component.fixture.html | 2 +- .../fixtures/tile1.component.fixture.ts | 2 + .../tile-dashboard.component.spec.ts | 28 ++++++- .../tile-dashboard/tile-dashboard.service.ts | 4 + 7 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 src/modules/tiles/tile-dashboard/fixtures/tile-dashboard-on-push.component.fixture.ts diff --git a/src/modules/tiles/tile-dashboard/fixtures/index.ts b/src/modules/tiles/tile-dashboard/fixtures/index.ts index 9c8522f38..dd5df1cc7 100644 --- a/src/modules/tiles/tile-dashboard/fixtures/index.ts +++ b/src/modules/tiles/tile-dashboard/fixtures/index.ts @@ -2,6 +2,7 @@ export { MockDragulaService } from './mock-dragula.service'; export { Tile1TestComponent } from './tile1.component.fixture'; export { Tile2TestComponent } from './tile2.component.fixture'; export { TileDashboardTestComponent } from './tile-dashboard.component.fixture'; +export { TileDashboardOnPushTestComponent } from './tile-dashboard-on-push.component.fixture'; export { TileTestContext } from './tile-context.fixture'; export { MockTileDashboardService } from './mock-tile-dashboard.service'; export { SkyTileDashboardFixturesModule } from './tile-dashboard-fixtures.module'; diff --git a/src/modules/tiles/tile-dashboard/fixtures/tile-dashboard-fixtures.module.ts b/src/modules/tiles/tile-dashboard/fixtures/tile-dashboard-fixtures.module.ts index e37e83301..42ce42a10 100644 --- a/src/modules/tiles/tile-dashboard/fixtures/tile-dashboard-fixtures.module.ts +++ b/src/modules/tiles/tile-dashboard/fixtures/tile-dashboard-fixtures.module.ts @@ -5,12 +5,14 @@ import { SkyTilesModule } from '../..'; import { Tile1TestComponent } from './tile1.component.fixture'; import { Tile2TestComponent } from './tile2.component.fixture'; import { TileDashboardTestComponent } from './tile-dashboard.component.fixture'; +import { TileDashboardOnPushTestComponent } from './tile-dashboard-on-push.component.fixture'; @NgModule({ declarations: [ Tile1TestComponent, Tile2TestComponent, - TileDashboardTestComponent + TileDashboardTestComponent, + TileDashboardOnPushTestComponent ], imports: [ CommonModule, @@ -19,7 +21,8 @@ import { TileDashboardTestComponent } from './tile-dashboard.component.fixture'; exports: [ Tile1TestComponent, Tile2TestComponent, - TileDashboardTestComponent + TileDashboardTestComponent, + TileDashboardOnPushTestComponent ], entryComponents: [ Tile1TestComponent, diff --git a/src/modules/tiles/tile-dashboard/fixtures/tile-dashboard-on-push.component.fixture.ts b/src/modules/tiles/tile-dashboard/fixtures/tile-dashboard-on-push.component.fixture.ts new file mode 100644 index 000000000..ca35bd2c1 --- /dev/null +++ b/src/modules/tiles/tile-dashboard/fixtures/tile-dashboard-on-push.component.fixture.ts @@ -0,0 +1,74 @@ +import { ChangeDetectionStrategy, Component, ViewChild } from '@angular/core'; + +import { SkyTileDashboardComponent, SkyTileDashboardConfig } from '../../../tiles'; +import { Tile1TestComponent } from './tile1.component.fixture'; +import { Tile2TestComponent } from './tile2.component.fixture'; + +import { TileTestContext } from './tile-context.fixture'; + +@Component({ + selector: 'sky-demo-app', + templateUrl: './tile-dashboard.component.fixture.html', + changeDetection: ChangeDetectionStrategy.OnPush +}) +export class TileDashboardOnPushTestComponent { + @ViewChild(SkyTileDashboardComponent) + public dashboardComponent: SkyTileDashboardComponent; + + public dashboardConfig: SkyTileDashboardConfig; + + constructor() { + this.dashboardConfig = { + tiles: [ + { + id: 'sky-test-tile-1', + componentType: Tile1TestComponent + }, + { + id: 'sky-test-tile-2', + componentType: Tile2TestComponent, + providers: [ + { + provide: TileTestContext, + useValue: { + id: 3 + } + } + ] + } + ], + layout: { + singleColumn: { + tiles: [ + { + id: 'sky-test-tile-2', + isCollapsed: false + }, + { + id: 'sky-test-tile-1', + isCollapsed: true + } + ] + }, + multiColumn: [ + { + tiles: [ + { + id: 'sky-test-tile-1', + isCollapsed: true + } + ] + }, + { + tiles: [ + { + id: 'sky-test-tile-2', + isCollapsed: false + } + ] + } + ] + } + }; + } +} diff --git a/src/modules/tiles/tile-dashboard/fixtures/tile1.component.fixture.html b/src/modules/tiles/tile-dashboard/fixtures/tile1.component.fixture.html index f646b5658..477d7b01c 100644 --- a/src/modules/tiles/tile-dashboard/fixtures/tile1.component.fixture.html +++ b/src/modules/tiles/tile-dashboard/fixtures/tile1.component.fixture.html @@ -1,4 +1,4 @@ - Title + {{ title }} Content diff --git a/src/modules/tiles/tile-dashboard/fixtures/tile1.component.fixture.ts b/src/modules/tiles/tile-dashboard/fixtures/tile1.component.fixture.ts index 04c88fb9b..38a7a4022 100644 --- a/src/modules/tiles/tile-dashboard/fixtures/tile1.component.fixture.ts +++ b/src/modules/tiles/tile-dashboard/fixtures/tile1.component.fixture.ts @@ -11,5 +11,7 @@ export class Tile1TestComponent { @ViewChild(SkyTileComponent) public tile: SkyTileComponent; + public title = 'Tile 1'; + public tileSettingsClick() { } } diff --git a/src/modules/tiles/tile-dashboard/tile-dashboard.component.spec.ts b/src/modules/tiles/tile-dashboard/tile-dashboard.component.spec.ts index 7695620a1..d97f37212 100644 --- a/src/modules/tiles/tile-dashboard/tile-dashboard.component.spec.ts +++ b/src/modules/tiles/tile-dashboard/tile-dashboard.component.spec.ts @@ -17,7 +17,8 @@ import { SkyTileDashboardFixturesModule, Tile1TestComponent, Tile2TestComponent, - TileDashboardTestComponent + TileDashboardTestComponent, + TileDashboardOnPushTestComponent } from './fixtures'; describe('Tile dashboard component', () => { @@ -309,4 +310,29 @@ describe('Tile dashboard component', () => { expect(tileComponentRef.instance.context.id).toBe(3); }) ); + + fit( + `should render tiles properly when the parent component's change detection strategy is OnPush`, + fakeAsync(() => { + let fixture = TestBed.createComponent(TileDashboardOnPushTestComponent); + + fixture.detectChanges(); + tick(); + + // For some reason we have to run change detection twice for the tile to actually render. + fixture.detectChanges(); + tick(); + + let cmp = fixture.componentInstance; + + let tileComponentRef = cmp + .dashboardComponent + .dashboardService + .getTileComponent('sky-test-tile-1'); + + let tileEl = tileComponentRef.location.nativeElement; + + expect(tileEl.querySelector('.sky-tile-title').innerText).toBe('Tile 1'); + }) + ); }); diff --git a/src/modules/tiles/tile-dashboard/tile-dashboard.service.ts b/src/modules/tiles/tile-dashboard/tile-dashboard.service.ts index 2e257bf61..9b21f98de 100644 --- a/src/modules/tiles/tile-dashboard/tile-dashboard.service.ts +++ b/src/modules/tiles/tile-dashboard/tile-dashboard.service.ts @@ -215,6 +215,10 @@ export class SkyTileDashboardService { let componentRef = column.content.createComponent(factory, undefined, injector); this.addTileComponent(layoutTile, componentRef); + + // Make sure the component is marked for changes in case the parent component uses + // the OnPush change detection strategy. + componentRef.changeDetectorRef.markForCheck(); } private moveTilesToSingleColumn() { From d0332faa5fe757bf1b6b048e390696dcdff8e242 Mon Sep 17 00:00:00 2001 From: Paul Crowder Date: Wed, 1 Feb 2017 14:07:07 -0500 Subject: [PATCH 2/3] Improved expect messages. --- src/modules/testing/matchers.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/modules/testing/matchers.ts b/src/modules/testing/matchers.ts index 0fe8c7bd4..1ae06bc24 100644 --- a/src/modules/testing/matchers.ts +++ b/src/modules/testing/matchers.ts @@ -17,8 +17,8 @@ let skyMatchers: jasmine.CustomMatcherFactories = { result.pass = getComputedStyle(el).display !== 'none'; result.message = result.pass ? - 'Expected element to not be visible' : - 'Expected element to be visible'; + 'Expected element to not be visible.' : + 'Expected element to be visible.'; return result; } @@ -42,8 +42,8 @@ let skyMatchers: jasmine.CustomMatcherFactories = { result.pass = actualText === expectedText; result.message = result.pass ? - 'Expected element\'s inner text not to be ' + expectedText : - 'Expected element\'s inner text to be ' + expectedText; + `Expected element's inner text not to be '${expectedText}'.` : + `Expected element's inner text '${actualText}' to be '${expectedText}'.`; return result; } @@ -65,8 +65,8 @@ let skyMatchers: jasmine.CustomMatcherFactories = { result.pass = el.classList.contains(expectedCls); result.message = result.pass ? - 'Expected element not to have CSS class ' + expectedCls : - 'Expected element to have CSS class ' + expectedCls; + `Expected element not to have CSS class ${expectedCls}.` : + `Expected element to have CSS class ${expectedCls}.`; return result; } @@ -91,8 +91,8 @@ let skyMatchers: jasmine.CustomMatcherFactories = { } result.message += result.pass ? - 'Expected element not to have CSS style ' + p + ': ' + expectedStyle : - 'Expected element to have CSS style ' + p + ': ' + expectedStyle; + `Expected element not to have CSS style '${p}: ${expectedStyle}'.` : + `Expected element to have CSS style '${p}: ${expectedStyle}'.`; } } } @@ -113,8 +113,8 @@ let skyMatchers: jasmine.CustomMatcherFactories = { result.pass = !!el; result.message = result.pass ? - 'Expected element not to exist' : - 'Expected element to exist'; + 'Expected element not to exist.' : + 'Expected element to exist.'; return result; } From 3de234fdb4bd229715c3c6088faba935849e90d4 Mon Sep 17 00:00:00 2001 From: Paul Crowder Date: Wed, 1 Feb 2017 14:07:21 -0500 Subject: [PATCH 3/3] Fixed unit tests. --- .../tiles/tile-dashboard/tile-dashboard.component.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/modules/tiles/tile-dashboard/tile-dashboard.component.spec.ts b/src/modules/tiles/tile-dashboard/tile-dashboard.component.spec.ts index d97f37212..28fda401d 100644 --- a/src/modules/tiles/tile-dashboard/tile-dashboard.component.spec.ts +++ b/src/modules/tiles/tile-dashboard/tile-dashboard.component.spec.ts @@ -311,7 +311,7 @@ describe('Tile dashboard component', () => { }) ); - fit( + it( `should render tiles properly when the parent component's change detection strategy is OnPush`, fakeAsync(() => { let fixture = TestBed.createComponent(TileDashboardOnPushTestComponent); @@ -332,7 +332,7 @@ describe('Tile dashboard component', () => { let tileEl = tileComponentRef.location.nativeElement; - expect(tileEl.querySelector('.sky-tile-title').innerText).toBe('Tile 1'); + expect(tileEl.querySelector('.sky-tile-title')).toHaveText('Tile 1'); }) ); });