From 9f0e40a6e80e90a90488f8aa1801bf702f3c4fab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Popovics=20Andr=C3=A1s?= Date: Fri, 29 Sep 2017 17:49:01 +0100 Subject: [PATCH] [ADF-1583] Fix change detection error (the quick way) (#2396) * Fix change detection error (the quick way) * Fix it the second time --- .../app/components/files/files.component.html | 2 +- .../app/components/files/files.component.ts | 11 ++++-- .../info-drawer/info-drawer.component.spec.ts | 1 + .../node-permission.directive.spec.ts | 36 ++++++++++++------- .../directives/node-permission.directive.ts | 11 +++--- 5 files changed, 40 insertions(+), 21 deletions(-) diff --git a/demo-shell-ng2/app/components/files/files.component.html b/demo-shell-ng2/app/components/files/files.component.html index 5703dec0da..14450ea3b6 100644 --- a/demo-shell-ng2/app/components/files/files.component.html +++ b/demo-shell-ng2/app/components/files/files.component.html @@ -4,7 +4,7 @@ diff --git a/demo-shell-ng2/app/components/files/files.component.ts b/demo-shell-ng2/app/components/files/files.component.ts index 3fa085b210..d6c40ce37c 100644 --- a/demo-shell-ng2/app/components/files/files.component.ts +++ b/demo-shell-ng2/app/components/files/files.component.ts @@ -29,6 +29,8 @@ import { DocumentListComponent, PermissionStyleModel } from 'ng2-alfresco-docume import { ViewerService } from 'ng2-alfresco-viewer'; +const DEFAULT_FOLDER_TO_SHOW = '-my-'; + @Component({ selector: 'adf-files-component', templateUrl: './files.component.html', @@ -36,7 +38,7 @@ import { ViewerService } from 'ng2-alfresco-viewer'; }) export class FilesComponent implements OnInit { // The identifier of a node. You can also use one of these well-known aliases: -my- | -shared- | -root- - currentFolderId: string = '-my-'; + currentFolderId: string = DEFAULT_FOLDER_TO_SHOW; errorMessage: string = null; fileNodeId: any; @@ -114,6 +116,7 @@ export class FilesComponent implements OnInit { } onFolderChange($event) { + this.currentFolderId = $event.value.id; this.router.navigate(['/files', $event.value.id]); } @@ -220,7 +223,11 @@ export class FilesComponent implements OnInit { } getSiteContent(site: SiteModel) { - this.currentFolderId = site && site.guid ? site.guid : '-my-'; + this.currentFolderId = site && site.guid ? site.guid : DEFAULT_FOLDER_TO_SHOW; + } + + getDocumentListCurrentFolderId() { + return this.documentList.currentFolderId || DEFAULT_FOLDER_TO_SHOW; } hasSelection(selection: Array): boolean { diff --git a/ng2-components/ng2-alfresco-core/src/components/info-drawer/info-drawer.component.spec.ts b/ng2-components/ng2-alfresco-core/src/components/info-drawer/info-drawer.component.spec.ts index 8c836605d9..9d7f676180 100644 --- a/ng2-components/ng2-alfresco-core/src/components/info-drawer/info-drawer.component.spec.ts +++ b/ng2-components/ng2-alfresco-core/src/components/info-drawer/info-drawer.component.spec.ts @@ -14,6 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { DebugElement } from '@angular/core'; import { async, ComponentFixture, TestBed } from '@angular/core/testing'; import { MaterialModule } from '../../material.module'; diff --git a/ng2-components/ng2-alfresco-core/src/directives/node-permission.directive.spec.ts b/ng2-components/ng2-alfresco-core/src/directives/node-permission.directive.spec.ts index d7cd557923..e2e47bac89 100644 --- a/ng2-components/ng2-alfresco-core/src/directives/node-permission.directive.spec.ts +++ b/ng2-components/ng2-alfresco-core/src/directives/node-permission.directive.spec.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { Component, ElementRef, SimpleChange } from '@angular/core'; +import { ChangeDetectorRef, Component, ElementRef, SimpleChange } from '@angular/core'; import { AlfrescoContentService } from './../services/alfresco-content.service'; import { NodePermissionDirective, NodePermissionSubject } from './node-permission.directive'; @@ -28,19 +28,27 @@ class TestComponent implements NodePermissionSubject { describe('NodePermissionDirective', () => { + let changeDetectorMock: ChangeDetectorRef; + + beforeEach(() => { + changeDetectorMock = { detectChanges: () => {} }; + }); + describe('HTML nativeElement as subject', () => { it('updates element once it is loaded', () => { - const directive = new NodePermissionDirective(null, null, null); + const directive = new NodePermissionDirective(null, null, null, changeDetectorMock); spyOn(directive, 'updateElement').and.stub(); - directive.ngAfterViewInit(); + const nodes = [{}, {}]; + const change = new SimpleChange([], nodes, false); + directive.ngOnChanges({ nodes: change }); expect(directive.updateElement).toHaveBeenCalled(); }); it('updates element on nodes change', () => { - const directive = new NodePermissionDirective(null, null, null); + const directive = new NodePermissionDirective(null, null, null, changeDetectorMock); spyOn(directive, 'updateElement').and.stub(); const nodes = [{}, {}]; @@ -51,7 +59,7 @@ describe('NodePermissionDirective', () => { }); it('updates element only on subsequent change', () => { - const directive = new NodePermissionDirective(null, null, null); + const directive = new NodePermissionDirective(null, null, null, changeDetectorMock); spyOn(directive, 'updateElement').and.stub(); const nodes = [{}, {}]; @@ -64,7 +72,7 @@ describe('NodePermissionDirective', () => { it('enables decorated element', () => { const renderer = jasmine.createSpyObj('renderer', ['removeAttribute']); const elementRef = new ElementRef({}); - const directive = new NodePermissionDirective(elementRef, renderer, null); + const directive = new NodePermissionDirective(elementRef, renderer, null, changeDetectorMock); directive.enableElement(); @@ -74,7 +82,7 @@ describe('NodePermissionDirective', () => { it('disables decorated element', () => { const renderer = jasmine.createSpyObj('renderer', ['setAttribute']); const elementRef = new ElementRef({}); - const directive = new NodePermissionDirective(elementRef, renderer, null); + const directive = new NodePermissionDirective(elementRef, renderer, null, changeDetectorMock); directive.disableElement(); @@ -82,7 +90,7 @@ describe('NodePermissionDirective', () => { }); it('disables element when nodes not available', () => { - const directive = new NodePermissionDirective(null, null, null); + const directive = new NodePermissionDirective(null, null, null, changeDetectorMock); spyOn(directive, 'disableElement').and.stub(); directive.nodes = null; @@ -96,7 +104,7 @@ describe('NodePermissionDirective', () => { const contentService = new AlfrescoContentService(null, null, null); spyOn(contentService, 'hasPermission').and.returnValue(true); - const directive = new NodePermissionDirective(null, null, contentService); + const directive = new NodePermissionDirective(null, null, contentService, changeDetectorMock); spyOn(directive, 'enableElement').and.stub(); directive.nodes = [{}, {}]; @@ -109,7 +117,7 @@ describe('NodePermissionDirective', () => { const contentService = new AlfrescoContentService(null, null, null); spyOn(contentService, 'hasPermission').and.returnValue(false); - const directive = new NodePermissionDirective(null, null, contentService); + const directive = new NodePermissionDirective(null, null, contentService, changeDetectorMock); spyOn(directive, 'disableElement').and.stub(); directive.nodes = [{}, {}]; @@ -124,29 +132,33 @@ describe('NodePermissionDirective', () => { it('disables decorated component', () => { const contentService = new AlfrescoContentService(null, null, null); spyOn(contentService, 'hasPermission').and.returnValue(false); + spyOn(changeDetectorMock, 'detectChanges'); let testComponent = new TestComponent(); testComponent.disabled = false; - const directive = new NodePermissionDirective(null, null, contentService, testComponent); + const directive = new NodePermissionDirective(null, null, contentService, changeDetectorMock, testComponent); directive.nodes = [{}, {}]; directive.updateElement(); expect(testComponent.disabled).toBeTruthy(); + expect(changeDetectorMock.detectChanges).toHaveBeenCalledTimes(1); }); it('enables decorated component', () => { const contentService = new AlfrescoContentService(null, null, null); spyOn(contentService, 'hasPermission').and.returnValue(true); + spyOn(changeDetectorMock, 'detectChanges'); let testComponent = new TestComponent(); testComponent.disabled = true; - const directive = new NodePermissionDirective(null, null, contentService, testComponent); + const directive = new NodePermissionDirective(null, null, contentService, changeDetectorMock, testComponent); directive.nodes = [{}, {}]; directive.updateElement(); expect(testComponent.disabled).toBeFalsy(); + expect(changeDetectorMock.detectChanges).toHaveBeenCalledTimes(1); }); }); }); diff --git a/ng2-components/ng2-alfresco-core/src/directives/node-permission.directive.ts b/ng2-components/ng2-alfresco-core/src/directives/node-permission.directive.ts index f707b6a2fa..e5ca7b7ea7 100644 --- a/ng2-components/ng2-alfresco-core/src/directives/node-permission.directive.ts +++ b/ng2-components/ng2-alfresco-core/src/directives/node-permission.directive.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { AfterViewInit, Directive, ElementRef, Host, Inject, Input, OnChanges, Optional, Renderer2, SimpleChanges } from '@angular/core'; +import { ChangeDetectorRef, Directive, ElementRef, Host, Inject, Input, OnChanges, Optional, Renderer2, SimpleChanges } from '@angular/core'; import { MinimalNodeEntity } from 'alfresco-js-api'; import { EXTENDIBLE_COMPONENT } from './../interface/injection.tokens'; import { AlfrescoContentService } from './../services/alfresco-content.service'; @@ -27,7 +27,7 @@ export interface NodePermissionSubject { @Directive({ selector: '[adf-node-permission]' }) -export class NodePermissionDirective implements OnChanges, AfterViewInit { +export class NodePermissionDirective implements OnChanges { @Input('adf-node-permission') permission: string = null; @@ -38,16 +38,13 @@ export class NodePermissionDirective implements OnChanges, AfterViewInit { constructor(private elementRef: ElementRef, private renderer: Renderer2, private contentService: AlfrescoContentService, + private changeDetector: ChangeDetectorRef, @Host() @Optional() @Inject(EXTENDIBLE_COMPONENT) private parentComponent?: NodePermissionSubject) { } - ngAfterViewInit() { - this.updateElement(); - } - ngOnChanges(changes: SimpleChanges) { if (changes.nodes && !changes.nodes.firstChange) { this.updateElement(); @@ -75,6 +72,7 @@ export class NodePermissionDirective implements OnChanges, AfterViewInit { private enable(): void { if (this.parentComponent) { this.parentComponent.disabled = false; + this.changeDetector.detectChanges(); } else { this.enableElement(); } @@ -83,6 +81,7 @@ export class NodePermissionDirective implements OnChanges, AfterViewInit { private disable(): void { if (this.parentComponent) { this.parentComponent.disabled = true; + this.changeDetector.detectChanges(); } else { this.disableElement(); }