Skip to content

Commit 6fd9c84

Browse files
ovchinnikovanna-zhernovkova
ovchinnikov
authored and
anna-zhernovkova
committed
Rework the strategy of applying options (#626)
* Rework the strategy of applying options * Use expect.toThrow for two way binding test * Fix typo in variable name * Fix test case * Add implementation to OnChanges interface * Rename variables * Get rid of using of private field * Return condition to iterableDifferHelper * Rewrite test for T545977
1 parent c73a29b commit 6fd9c84

File tree

6 files changed

+274
-78
lines changed

6 files changed

+274
-78
lines changed

‎src/core/component.ts‎

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ import {
33
NgZone,
44
QueryList,
55
AfterViewInit,
6-
AfterViewChecked
6+
DoCheck,
7+
OnChanges,
8+
AfterContentChecked,
9+
SimpleChanges
710
} from '@angular/core';
811

912
import { DxTemplateDirective } from './template';
@@ -19,9 +22,10 @@ import {
1922
CollectionNestedOptionContainerImpl
2023
} from './nested-option';
2124

22-
export abstract class DxComponent implements AfterViewInit,
23-
INestedOptionContainer, ICollectionNestedOptionContainer, IDxTemplateHost, AfterViewChecked {
25+
export abstract class DxComponent implements AfterViewInit, DoCheck, OnChanges, AfterContentChecked,
26+
INestedOptionContainer, ICollectionNestedOptionContainer, IDxTemplateHost {
2427
private _initialOptions: any = {};
28+
protected _optionsToUpdate: any = {};
2529
private _collectionContainerImpl: ICollectionNestedOptionContainer;
2630
eventHelper: EmitterHelper;
2731
templates: DxTemplateDirective[];
@@ -65,24 +69,24 @@ export abstract class DxComponent implements AfterViewInit,
6569
this.instance.option(name) :
6670
this._initialOptions[name];
6771
}
68-
protected _setOption(name: string, value: any) {
69-
if (this.instance) {
70-
this._updateOption(name, value);
71-
} else {
72-
this._initialOptions[name] = value;
73-
}
74-
}
7572
lockWidgetUpdate() {
76-
if (!this.widgetUpdateLocked) {
73+
if (!this.widgetUpdateLocked && this.instance) {
7774
this.instance.beginUpdate();
7875
this.widgetUpdateLocked = true;
7976
}
8077
}
81-
protected _updateOption(name: string, value: any) {
78+
protected _setOption(name: string, value: any) {
8279
this.lockWidgetUpdate();
83-
if (this._shouldOptionChange(name, value)) {
80+
81+
if (!this._shouldOptionChange(name, value)) {
82+
return;
83+
}
84+
85+
if (this.instance) {
8486
this.instance.option(name, value);
85-
};
87+
} else {
88+
this._initialOptions[name] = value;
89+
}
8690
}
8791
protected abstract _createInstance(element, options)
8892
protected _createWidget(element: any) {
@@ -94,7 +98,7 @@ export abstract class DxComponent implements AfterViewInit,
9498
this.eventHelper.rememberEvent(e.name);
9599
};
96100

97-
this._initialOptions.onInitializing = function() {
101+
this._initialOptions.onInitializing = function () {
98102
this.on('optionChanged', optionChangeHandler);
99103
};
100104
this.instance = this._createInstance(element, this._initialOptions);
@@ -120,15 +124,35 @@ export abstract class DxComponent implements AfterViewInit,
120124
this._collectionContainerImpl = new CollectionNestedOptionContainerImpl(this._setOption.bind(this));
121125
this.eventHelper = new EmitterHelper(this.ngZone, this);
122126
}
123-
ngAfterViewChecked() {
127+
ngAfterViewInit() {
128+
if (this.renderOnViewInit) {
129+
this._createWidget(this.element.nativeElement);
130+
}
131+
}
132+
ngAfterContentChecked() {
133+
this.applyOptions();
124134
if (this.widgetUpdateLocked) {
125135
this.widgetUpdateLocked = false;
126136
this.instance.endUpdate();
127137
}
128138
}
129-
ngAfterViewInit() {
130-
if (this.renderOnViewInit) {
131-
this._createWidget(this.element.nativeElement);
139+
ngOnChanges(changes: SimpleChanges) {
140+
for (let key in changes) {
141+
let change = changes[key];
142+
if (change.currentValue !== this[key]) {
143+
this._optionsToUpdate[key] = changes[key].currentValue;
144+
}
145+
}
146+
}
147+
ngDoCheck() {
148+
this.applyOptions();
149+
}
150+
applyOptions() {
151+
if (Object.keys(this._optionsToUpdate).length) {
152+
if (this.instance) {
153+
this.instance.option(this._optionsToUpdate);
154+
}
155+
this._optionsToUpdate = {};
132156
}
133157
}
134158
setTemplate(template: DxTemplateDirective) {

‎templates/component.tst‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,21 +160,29 @@ export class <#= it.className #>Component extends <#= baseClass #> <#? implement
160160
this._destroyWidget();
161161
}
162162
<#? collectionProperties.length #>
163-
ngOnChanges(changes: SimpleChanges) {<#~ collectionProperties :prop:i #>
164-
this._idh.setup('<#= prop #>', changes);<#~#>
163+
ngOnChanges(changes: SimpleChanges) {
164+
super.ngOnChanges(changes);<#~ collectionProperties :prop:i #>
165+
this.setupChanges('<#= prop #>', changes);<#~#>
166+
}
167+
168+
setupChanges(prop: string, changes: SimpleChanges) {
169+
if (!(prop in this._optionsToUpdate)) {
170+
this._idh.setup(prop, changes);
171+
}
165172
}
166173

167174
ngDoCheck() {<#~ collectionProperties :prop:i #>
168175
this._idh.doCheck('<#= prop #>');<#~#>
169176
this._watcherHelper.checkWatchers();
177+
super.ngDoCheck();
170178
}
171179

172-
_updateOption(name: string, value: any) {
180+
_setOption(name: string, value: any) {
173181
let isSetup = this._idh.setupSingle(name, value);
174182
let isChanged = this._idh.getChanges(name, value) !== null;
175183

176184
if (isSetup || isChanged) {
177-
super._updateOption(name, value);
185+
super._setOption(name, value);
178186
}
179187
}<#?#>
180188
<#? it.isEditor #>

‎tests/src/ui/data-grid.spec.ts‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import DxDataGrid from 'devextreme/ui/data_grid';
2323
})
2424
class TestContainerComponent {
2525
dataSource = [{
26+
id: 1,
2627
string: 'String',
2728
date: new Date(),
2829
dateString: '1995/01/15',
@@ -38,9 +39,16 @@ class TestContainerComponent {
3839
];
3940
dataSourceWithUndefined = [{ obj: { field: undefined }}];
4041

42+
columsChanged = 0;
4143
@ViewChildren(DxDataGridComponent) innerWidgets: QueryList<DxDataGridComponent>;
4244

4345
testMethod() {}
46+
47+
onOptionChanged(e) {
48+
if (e.name === 'columns') {
49+
this.columsChanged++;
50+
}
51+
}
4452
}
4553

4654

@@ -179,3 +187,96 @@ describe('DxDataGrid', () => {
179187
jasmine.clock().uninstall();
180188
});
181189
});
190+
191+
describe('Nested DxDataGrid', () => {
192+
let originalTimeout;
193+
194+
beforeEach(() => {
195+
TestBed.configureTestingModule(
196+
{
197+
declarations: [TestContainerComponent],
198+
imports: [DxDataGridModule]
199+
});
200+
201+
originalTimeout = jasmine.DEFAULT_TIMEOUT_INTERVAL;
202+
jasmine.DEFAULT_TIMEOUT_INTERVAL = 3000;
203+
});
204+
205+
afterEach(function() {
206+
jasmine.DEFAULT_TIMEOUT_INTERVAL = originalTimeout;
207+
});
208+
209+
// NOTE: https://github.com/angular/angular/issues/18997
210+
it('should not update parent DxDataGrid with 30 dxi-column (T545977)', (done) => {
211+
TestBed.overrideComponent(TestContainerComponent, {
212+
set: {
213+
template: `
214+
<dx-data-grid
215+
[dataSource]="dataSource"
216+
keyExpr="id"
217+
[masterDetail]="{ enabled: true, template: 'detail' }"
218+
(onOptionChanged)="onOptionChanged($event)">
219+
220+
<dxi-column dataField="string"></dxi-column>
221+
<dxi-column dataField="string"></dxi-column>
222+
<dxi-column dataField="string"></dxi-column>
223+
<dxi-column dataField="string"></dxi-column>
224+
<dxi-column dataField="string"></dxi-column>
225+
<dxi-column dataField="string"></dxi-column>
226+
<dxi-column dataField="string"></dxi-column>
227+
<dxi-column dataField="string"></dxi-column>
228+
<dxi-column dataField="string"></dxi-column>
229+
<dxi-column dataField="string"></dxi-column>
230+
<dxi-column dataField="string"></dxi-column>
231+
<dxi-column dataField="string"></dxi-column>
232+
<dxi-column dataField="string"></dxi-column>
233+
<dxi-column dataField="string"></dxi-column>
234+
<dxi-column dataField="string"></dxi-column>
235+
236+
<div *dxTemplate="let data of 'detail'">
237+
<dx-data-grid [dataSource]="dataSource">
238+
<dxi-column dataField="number"></dxi-column>
239+
<dxi-column dataField="number"></dxi-column>
240+
<dxi-column dataField="number"></dxi-column>
241+
<dxi-column dataField="number"></dxi-column>
242+
<dxi-column dataField="number"></dxi-column>
243+
<dxi-column dataField="number"></dxi-column>
244+
<dxi-column dataField="number"></dxi-column>
245+
<dxi-column dataField="number"></dxi-column>
246+
<dxi-column dataField="number"></dxi-column>
247+
<dxi-column dataField="number"></dxi-column>
248+
<dxi-column dataField="number"></dxi-column>
249+
<dxi-column dataField="number"></dxi-column>
250+
<dxi-column dataField="number"></dxi-column>
251+
<dxi-column dataField="number"></dxi-column>
252+
<dxi-column dataField="number"></dxi-column>
253+
</dx-data-grid>
254+
</div>
255+
</dx-data-grid>
256+
`
257+
}
258+
});
259+
260+
let fixture = TestBed.createComponent(TestContainerComponent);
261+
fixture.detectChanges();
262+
263+
setTimeout(() => {
264+
fixture.detectChanges();
265+
266+
let testComponent = fixture.componentInstance,
267+
widgetComponent = testComponent.innerWidgets.first;
268+
269+
widgetComponent.instance.expandRow(1);
270+
fixture.detectChanges();
271+
272+
setTimeout(() => {
273+
fixture.detectChanges();
274+
275+
expect(testComponent.columsChanged).toBe(0);
276+
testComponent.columsChanged = 0;
277+
278+
done();
279+
}, 1000);
280+
}, 1000);
281+
});
282+
});

‎tests/src/ui/list.spec.ts‎

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -93,48 +93,6 @@ describe('DxList', () => {
9393
instance.option.calls.reset();
9494
}));
9595

96-
it('should not react if the identicaly value is assigned to the collection', async(() => {
97-
TestBed.overrideComponent(TestContainerComponent, {
98-
set: {
99-
template: '<dx-list [items]="items"></dx-list>'
100-
}
101-
});
102-
let fixture = TestBed.createComponent(TestContainerComponent);
103-
fixture.detectChanges();
104-
105-
let testComponent = fixture.componentInstance,
106-
instance = getWidget(fixture);
107-
108-
spyOn(instance, 'option').and.callThrough();
109-
110-
testComponent.items = testComponent.items.slice(0);
111-
fixture.detectChanges();
112-
113-
expect(instance.option).toHaveBeenCalledTimes(1);
114-
instance.option.calls.reset();
115-
}));
116-
117-
it('should react if the initial value is assigned to the collection', async(() => {
118-
TestBed.overrideComponent(TestContainerComponent, {
119-
set: {
120-
template: '<dx-list [items]="emptyItems"></dx-list>'
121-
}
122-
});
123-
let fixture = TestBed.createComponent(TestContainerComponent);
124-
fixture.detectChanges();
125-
126-
let testComponent = fixture.componentInstance,
127-
instance = getWidget(fixture);
128-
129-
spyOn(instance, 'option').and.callThrough();
130-
131-
testComponent.emptyItems = [];
132-
fixture.detectChanges();
133-
134-
expect(instance.option.calls.count()).toBeGreaterThan(1);
135-
instance.option.calls.reset();
136-
}));
137-
13896
it('should be able to accept items as a static nested components list', async(() => {
13997
TestBed.overrideComponent(TestContainerComponent, {
14098
set: {

0 commit comments

Comments
 (0)