Skip to content

Commit 0c27a48

Browse files
committed
MAGETWO-59801: [Performance Bug] Tax Rules Form unusable with large # of tax rates
1 parent c1f88cb commit 0c27a48

File tree

2 files changed

+80
-24
lines changed

2 files changed

+80
-24
lines changed

dev/tests/js/jasmine/tests/lib/mage/multiselect.test.js

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ define([
3737
expect(instance.data('mage-multiselect2').onCheck).toBeDefined();
3838
expect(instance.data('mage-multiselect2').onError).toBeDefined();
3939
expect(instance.data('mage-multiselect2').onOptionsChange).toBeDefined();
40+
expect(instance.data('mage-multiselect2').getCurrentPage).toBeDefined();
41+
expect(instance.data('mage-multiselect2').setCurrentPage).toBeDefined();
4042
});
4143

4244
it('multiselect2 options check', function () {
@@ -101,7 +103,35 @@ define([
101103
instance.data('mage-multiselect2').onKeyUp();
102104

103105
expect(instance.data('mage-multiselect2').setFilter).toHaveBeenCalled();
104-
expect(instance.data('mage-multiselect2').loadOptions).toHaveBeenCalledWith(1);
106+
expect(instance.data('mage-multiselect2').loadOptions).toHaveBeenCalled();
107+
expect(instance.data('mage-multiselect2').getCurrentPage()).toEqual(0);
108+
});
109+
110+
it('multiselect2 paging test', function () {
111+
spyOn(instance.data('mage-multiselect2'), 'appendOptions').and.callFake(function () {
112+
return true;
113+
});
114+
spyOn(instance.data('mage-multiselect2'), 'setCurrentPage').and.callFake(function () {
115+
return true;
116+
});
117+
118+
$.get = jasmine.createSpy().and.callFake(function () {
119+
var d = $.Deferred();
120+
121+
d.resolve({
122+
'success': true
123+
});
124+
125+
return d.promise();
126+
});
127+
128+
expect(instance.data('mage-multiselect2').getCurrentPage()).toEqual(1);
129+
130+
instance.data('mage-multiselect2').loadOptions();
131+
132+
expect($.get).toHaveBeenCalled();
133+
expect(instance.data('mage-multiselect2').appendOptions).toHaveBeenCalled();
134+
expect(instance.data('mage-multiselect2').setCurrentPage).toHaveBeenCalledWith(2);
105135
});
106136
});
107137
});

lib/web/mage/multiselect.js

Lines changed: 49 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ define([
1313
], function (_, $, searchTemplate, alert) {
1414
'use strict';
1515

16-
var nativeConstructor = $.fn.multiselect;
17-
1816
$.widget('mage.multiselect2', {
1917
options: {
2018
mselectContainer: 'section.mselect-list',
@@ -25,13 +23,13 @@ define([
2523
selectedItemsCountClass: 'admin__action-multiselect-items-selected',
2624
currentPage: 1,
2725
lastAppendValue: 0,
28-
updateDelay: 500,
26+
updateDelay: 1000,
2927
optionsLoaded: false
3028
},
3129

3230
/** @inheritdoc */
3331
_create: function () {
34-
nativeConstructor.call(this.element, this.options);
32+
$.fn.multiselect.call(this.element, this.options);
3533
},
3634

3735
/** @inheritdoc */
@@ -42,6 +40,7 @@ define([
4240
this.$wrapper = this.$container.find('.' + this.options.mselectItemsWrapperClass);
4341
this.$item = this.$wrapper.find('div').first();
4442
this.selectedValues = [];
43+
this.values = {};
4544

4645
this.$container.addClass(this.options.containerClass).prepend(searchTemplate);
4746
this.$input = this.$container.find('.' + this.options.searchInputClass);
@@ -109,15 +108,18 @@ define([
109108
}
110109

111110
this.setFilter();
112-
this.emptyMultiselectOptions();
113-
this.loadOptions(1);
111+
this.clearMultiselectOptions();
112+
this.setCurrentPage(0);
113+
this.loadOptions();
114114
},
115115

116116
/**
117117
* Callback for select change event
118118
*/
119119
onOptionsChange: function () {
120120
this.selectedValues = _.map(this.domElement.options, function (option) {
121+
this.values[option.value] = true;
122+
121123
return option.value;
122124
}, this);
123125

@@ -170,30 +172,32 @@ define([
170172
/**
171173
* Load options data.
172174
*/
173-
loadOptions: function (page) {
174-
var self = this,
175-
requestPage = page || ++this.options.currentPage;
175+
loadOptions: function () {
176+
var nextPage = this.getCurrentPage() + 1;
176177

177178
this.$wrapper.trigger('processStart');
178179
this.$input.prop('disabled', true);
179180

180181
$.get(this.options.nextPageUrl, {
181-
p: requestPage,
182+
p: nextPage,
182183
s: this.filter
183-
}).done(function (response) {
184+
})
185+
.done(function (response) {
184186
if (response.success) {
185-
self.appendOptions(response.result);
187+
this.appendOptions(response.result);
188+
this.setCurrentPage(nextPage);
186189
} else {
187-
self.onError(response.errorMessage);
190+
this.onError(response.errorMessage);
188191
}
189-
}).always(function () {
190-
self.$wrapper.trigger('processStop');
191-
self.$input.prop('disabled', false);
192+
}.bind(this))
193+
.always(function () {
194+
this.$wrapper.trigger('processStop');
195+
this.$input.prop('disabled', false);
192196

193-
if (self.filter) {
194-
self.$input.focus();
197+
if (this.filter) {
198+
this.$input.focus();
195199
}
196-
});
200+
}.bind(this));
197201
},
198202

199203
/**
@@ -213,9 +217,12 @@ define([
213217
}
214218

215219
options.forEach(function (option) {
216-
option.selected = this._isOptionSelected(option);
217-
divOptions.push(this._createMultiSelectOption(option));
218-
this._setLastAppendOption(option.value);
220+
if (!this.values[option.value]) {
221+
this.values[option.value] = true;
222+
option.selected = this._isOptionSelected(option);
223+
divOptions.push(this._createMultiSelectOption(option));
224+
this._setLastAppendOption(option.value);
225+
}
219226
}, this);
220227

221228
this.$wrapper.append(divOptions);
@@ -224,8 +231,9 @@ define([
224231
/**
225232
* Clear multiselect options
226233
*/
227-
emptyMultiselectOptions: function () {
234+
clearMultiselectOptions: function () {
228235
this._setLastAppendOption(0);
236+
this.values = {};
229237
this.$wrapper.empty();
230238
},
231239

@@ -240,6 +248,24 @@ define([
240248
return this.options.optionsLoaded;
241249
},
242250

251+
/**
252+
* Setter for current page.
253+
*
254+
* @param {Number} page
255+
*/
256+
setCurrentPage: function (page) {
257+
this.options.currentPage = page;
258+
},
259+
260+
/**
261+
* Getter for current page.
262+
*
263+
* @return {Number}
264+
*/
265+
getCurrentPage: function () {
266+
return this.options.currentPage;
267+
},
268+
243269
/**
244270
* Creates new selected option for select element
245271
*

0 commit comments

Comments
 (0)