Skip to content

Commit

Permalink
Fixes firing onValueChanged event several times on change value option (
Browse files Browse the repository at this point in the history
#419)

* Fixes firing onValueChanged event several times on change value option

* Fix code style
  • Loading branch information
ovchinnikov committed Apr 4, 2017
1 parent b57f3cb commit 255001a
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 14 deletions.
21 changes: 18 additions & 3 deletions src/core/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export abstract class DxComponent implements INestedOptionContainer, ICollection
eventHelper: EmitterHelper;
templates: DxTemplateDirective[];
instance: any;
changedOptions = {};

protected _events: { subscribe?: string, emit: string }[];

Expand All @@ -44,10 +45,19 @@ export abstract class DxComponent implements INestedOptionContainer, ICollection
}
private _initEvents() {
this.instance.on('optionChanged', e => {
let changeEventName = e.name + 'Change';
this.eventHelper.fireNgEvent(changeEventName, [e.value]);
this.changedOptions[e.name] = e.value;
this.eventHelper.fireNgEvent(e.name + 'Change', [e.value]);
});
}
_shouldOptionChange(name: string, value: any) {
if (this.changedOptions.hasOwnProperty(name)) {
const prevValue = this.changedOptions[name];
delete this.changedOptions[name];

return value !== prevValue;
}
return true;
}
protected _getOption(name: string) {
if (this.instance) {
return this.instance.option(name);
Expand All @@ -57,11 +67,16 @@ export abstract class DxComponent implements INestedOptionContainer, ICollection
}
protected _setOption(name: string, value: any) {
if (this.instance) {
this.instance.option(name, value);
this._updateOption(name, value);
} else {
this._initialOptions[name] = value;
}
}
protected _updateOption(name: string, value: any) {
if (this._shouldOptionChange(name, value)) {
this.instance.option(name, value);
};
}
protected abstract _createInstance(element, options)
protected _createWidget(element: any) {
this._initTemplates();
Expand Down
30 changes: 20 additions & 10 deletions src/core/iterable-differ-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,33 @@ export class IterableDifferHelper {
}

setup(prop: string, changes: SimpleChanges) {
if (prop in changes) {
if (prop in changes) {
const value = changes[prop].currentValue;
if (value && Array.isArray(value)) {
if (!this._propertyDiffers[prop]) {
try {
this._propertyDiffers[prop] = this._differs.find(value).create(null);
} catch (e) { }
}
} else {
delete this._propertyDiffers[prop];
this.setupSingle(prop, value);
}
}

setupSingle(prop: string, value: any) {
if (value && Array.isArray(value)) {
if (!this._propertyDiffers[prop]) {
try {
this._propertyDiffers[prop] = this._differs.find(value).create(null);
} catch (e) { }
}
} else {
delete this._propertyDiffers[prop];
}
}

getChanges(prop: string, value: any) {
if (this._propertyDiffers[prop]) {
return this._propertyDiffers[prop].diff(value);
}
}

doCheck(prop: string) {
if (this._propertyDiffers[prop]) {
const changes = this._propertyDiffers[prop].diff(this._host[prop]);
const changes = this.getChanges(prop, this._host[prop]);
if (changes && this._host.instance) {
this._host.instance.option(prop, this._host[prop]);
}
Expand Down
9 changes: 9 additions & 0 deletions templates/component.tst
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,15 @@ export class <#= it.className #>Component extends <#= baseClass #> <#? implement
ngDoCheck() {<#~ collectionProperties :prop:i #>
this._idh.doCheck('<#= prop #>');<#~#>
this._watcherHelper.checkWatchers();
}

_updateOption(name: string, value: any) {
if (Array.isArray(value)) {
this._idh.setupSingle(name, value);
this._idh.getChanges(name, value);
}

super._updateOption(name, value);
}<#?#>
<#? !it.isExtension #>
ngAfterViewInit() {
Expand Down
3 changes: 2 additions & 1 deletion tests/src/ui/list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,8 @@ describe('DxList', () => {
testComponent.complexItems[0].text = 'Changed';
fixture.detectChanges();

expect(instance.option).toHaveBeenCalledTimes(0);
expect(instance.option).toHaveBeenCalledTimes(1);
expect(instance.option.calls.allArgs().length).toBe(1);
expect(instance.option('items').length).toBe(2);
expect(instance.element().find('.dx-item-content').length).toBe(2);
expect(instance.element().find('.dx-item-content').eq(0).text()).toBe('Changed');
Expand Down
61 changes: 61 additions & 0 deletions tests/src/ui/tag-box.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/* tslint:disable:component-selector */

import {
Component,
ViewChild
} from '@angular/core';

import {
TestBed,
async
} from '@angular/core/testing';

import {
DxTagBoxModule,
DxTagBoxComponent
} from '../../../dist';

@Component({
selector: 'test-container-component',
template: ''
})
class TestContainerComponent {
@ViewChild(DxTagBoxComponent) tagBox: DxTagBoxComponent;

value: number[] = [];
testMethod() {}
}

describe('DxTagBox', () => {

beforeEach(() => {
TestBed.configureTestingModule(
{
declarations: [TestContainerComponent],
imports: [DxTagBoxModule]
});
});

// spec
it('value change should be fired once', async(() => {
let testSpy = spyOn(TestContainerComponent.prototype, 'testMethod');
TestBed.overrideComponent(TestContainerComponent, {
set: {
template: `
<dx-tag-box [items]="[1, 2, 3]" [(value)]="value" (onValueChanged)="testMethod()">
</dx-tag-box>
`
}
});
let fixture = TestBed.createComponent(TestContainerComponent);
fixture.detectChanges();

expect(testSpy).toHaveBeenCalledTimes(0);

let instance: any = fixture.componentInstance.tagBox.instance;
instance.option('value', [2]);

fixture.detectChanges();
expect(testSpy).toHaveBeenCalledTimes(1);
}));
});

0 comments on commit 255001a

Please sign in to comment.