Skip to content

Commit 3948b21

Browse files
authored
fix: couple of issues related to lazy loading (#363)
1 parent 86cb364 commit 3948b21

File tree

6 files changed

+146
-20
lines changed

6 files changed

+146
-20
lines changed

packages/demo/src/options/options42.html

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,4 +59,12 @@ <h2 class="bd-title">
5959
<select id="group" class="full-width" multiple data-test="select3"></select>
6060
</div>
6161
</div>
62+
63+
<div class="mb-3 row">
64+
<label class="col-sm-2"> Filter &amp; OK button </label>
65+
66+
<div class="col-sm-10">
67+
<select id="group" class="full-width" multiple data-test="select4"></select>
68+
</div>
69+
</div>
6270
</div>

packages/demo/src/options/options42.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ export default class Example {
66
ms1?: MultipleSelectInstance;
77
ms2?: MultipleSelectInstance;
88
ms3?: MultipleSelectInstance;
9+
ms4?: MultipleSelectInstance;
910
serverDelay = 1000;
1011

1112
changeServerDelay(event: Event) {
@@ -194,16 +195,85 @@ export default class Example {
194195
});
195196
},
196197
}) as MultipleSelectInstance;
198+
199+
this.ms4 = multipleSelect('select[data-test=select4]', {
200+
filter: true,
201+
filterPlaceholder: 'filter placeholder',
202+
showOkButton: true,
203+
showClear: true,
204+
minHeight: 70,
205+
lazyData: () => {
206+
return new Promise(resolve => {
207+
setTimeout(() => {
208+
resolve([
209+
{
210+
text: 'January',
211+
value: 1,
212+
},
213+
{
214+
text: 'February',
215+
value: 2,
216+
selected: true,
217+
},
218+
{
219+
text: 'March',
220+
value: 3,
221+
disabled: true,
222+
},
223+
{
224+
text: 'April',
225+
value: 4,
226+
selected: true,
227+
},
228+
{
229+
text: 'May',
230+
value: 5,
231+
},
232+
{
233+
text: 'June',
234+
value: 6,
235+
},
236+
{
237+
text: 'July',
238+
value: 7,
239+
},
240+
{
241+
text: 'August',
242+
value: 8,
243+
},
244+
{
245+
text: 'September',
246+
value: 9,
247+
},
248+
{
249+
text: 'October',
250+
value: 10,
251+
},
252+
{
253+
text: 'November',
254+
value: 11,
255+
},
256+
{
257+
text: 'December',
258+
value: 12,
259+
},
260+
]);
261+
}, this.serverDelay);
262+
});
263+
},
264+
}) as MultipleSelectInstance;
197265
}
198266

199267
unmount() {
200268
// destroy ms instance(s) to avoid DOM leaks
201269
this.ms1?.destroy();
202270
this.ms2?.destroy();
203271
this.ms3?.destroy();
272+
this.ms4?.destroy();
204273
this.ms1 = undefined;
205274
this.ms2 = undefined;
206275
this.ms3 = undefined;
276+
this.ms4 = undefined;
207277
this.serverDelayInput!.removeEventListener('click', this.changeServerDelay.bind(this));
208278
this.resetLazyBtn!.removeEventListener('click', this.createMultipleSelect.bind(this));
209279
}

packages/multiple-select-vanilla/src/MultipleSelectInstance.ts

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,7 @@ export class MultipleSelectInstance {
173173
// label element
174174
this.labelElm = this.elm.closest('label');
175175
if (!this.labelElm && this.elm.id) {
176-
this.labelElm = document.createElement('label');
177-
this.labelElm.htmlFor = this.elm.id;
176+
this.labelElm = createDomElement('label', { htmlFor: this.elm.id });
178177
}
179178
if (this.labelElm?.querySelector('input')) {
180179
this.labelElm = null;
@@ -458,13 +457,15 @@ export class MultipleSelectInstance {
458457
}
459458
}
460459

461-
this.ulElm = document.createElement('ul');
462-
this.ulElm.role = 'combobox';
463-
this.ulElm.ariaExpanded = 'false';
464-
this.ulElm.ariaMultiSelectable = String(!this.options.single);
460+
this.ulElm = createDomElement('ul', {
461+
className: 'ms-list',
462+
role: 'listbox',
463+
ariaExpanded: 'false',
464+
ariaMultiSelectable: String(!this.options.single),
465+
});
465466
this.dropElm?.appendChild(this.ulElm);
466467

467-
if (this.options.showOkButton && !this.options.single) {
468+
if (renderFilterAndSearchAll && this.options.showOkButton && !this.options.single) {
468469
this.okButtonElm = createDomElement(
469470
'button',
470471
{ className: 'ms-ok-button', type: 'button', textContent: this.formatOkButton() },
@@ -627,12 +628,16 @@ export class MultipleSelectInstance {
627628
} else {
628629
itemOrGroupBlock = {
629630
tagName: 'div',
630-
props: { className: `icon-checkbox-container${type === 'radio' ? ' radio' : ''}` },
631+
props: {
632+
className: `icon-checkbox-container${type === 'radio' ? ' radio' : ''}`,
633+
},
631634
children: [
632635
inputCheckboxStruct,
633636
{
634637
tagName: 'div',
635-
props: { className: `ms-icon ${isChecked ? (type === 'radio' ? 'ms-icon-radio' : 'ms-icon-check') : 'ms-icon-uncheck'}` },
638+
props: {
639+
className: `ms-icon ${isChecked ? (type === 'radio' ? 'ms-icon-radio' : 'ms-icon-check') : 'ms-icon-uncheck'}`,
640+
},
636641
},
637642
],
638643
};
@@ -708,7 +713,9 @@ export class MultipleSelectInstance {
708713

709714
const iconContainerBlock: HtmlStruct = {
710715
tagName: 'div',
711-
props: { className: `icon-checkbox-container${type === 'radio' ? ' radio' : ''}` },
716+
props: {
717+
className: `icon-checkbox-container${type === 'radio' ? ' radio' : ''}`,
718+
},
712719
children: [
713720
inputBlock,
714721
{
@@ -1193,6 +1200,7 @@ export class MultipleSelectInstance {
11931200
let isLazyProcess = false;
11941201
if (this.options.lazyData && !this._isLazyLoaded) {
11951202
isLazyProcess = true;
1203+
this.dropElm?.querySelector('ul.ms-list')?.remove();
11961204
this.options.lazyData().then(data => {
11971205
// when data is ready, remove spinner & update dropdown and selection
11981206
this.options.data = data;
@@ -1223,7 +1231,7 @@ export class MultipleSelectInstance {
12231231
if (this.selectAllElm?.parentElement) {
12241232
this.selectAllElm.parentElement.style.display = 'none';
12251233
}
1226-
if (isLazyProcess && !this._isLazyLoaded) {
1234+
if (isLazyProcess && !this._isLazyLoaded && !this.dropElm.querySelector('.ms-loading')) {
12271235
const loadingElm = createDomElement('div', { className: 'ms-loading' });
12281236
loadingElm.appendChild(createDomElement('div', { className: 'ms-icon ms-icon-loading ms-spin' }));
12291237
loadingElm.appendChild(createDomElement('span', { textContent: this.formatLazyLoading() }));
@@ -1284,7 +1292,7 @@ export class MultipleSelectInstance {
12841292
}
12851293
this.ulElm ??= this.dropElm.querySelector('ul');
12861294
if (this.ulElm) {
1287-
if (minHeight) {
1295+
if (minHeight && (!this.options.lazyData || this._isLazyLoaded)) {
12881296
this.ulElm.style.minHeight = `${minHeight}px`;
12891297
}
12901298
this.ulElm.style.maxHeight = `${maxHeight}px`;
@@ -1983,19 +1991,15 @@ export class MultipleSelectInstance {
19831991
}
19841992

19851993
getScrollbarWidth() {
1986-
const outer = document.createElement('div');
1987-
outer.style.visibility = 'hidden';
1988-
outer.style.width = '100px';
1989-
1994+
const outer = createDomElement('div', { style: { visibility: 'hidden', width: '100px' } });
19901995
document.body.appendChild(outer);
19911996

19921997
const widthNoScroll = outer.offsetWidth;
19931998
// force scrollbars
19941999
outer.style.overflow = 'scroll';
19952000

19962001
// add innerdiv
1997-
const inner = document.createElement('div');
1998-
inner.style.width = '100%';
2002+
const inner = createDomElement('div', { style: { width: '100%' } });
19992003
outer.appendChild(inner);
20002004

20012005
const widthWithScroll = inner.offsetWidth;

packages/multiple-select-vanilla/src/styles/_variables.scss

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ $ms-label-min-height: 1.25rem !default;
7676
$ms-label-padding: 0 0 0 2px !default;
7777
$ms-label-text-padding-left: 8px !default;
7878
$ms-loading-gap: 4px !default;
79-
$ms-loading-icon-size: 1.25em !default;
79+
$ms-loading-icon-size: 1.15em !default;
8080
$ms-loading-padding: 0.6rem 8px !default;
8181
$ms-loading-spin-animation: 2s infinite linear !default;
8282
$ms-option-highlight-bg-color: rgba($ms-primary-color, 8%) !default;

packages/multiple-select-vanilla/src/styles/multiple-select.scss

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,8 @@
402402
padding: var(--ms-loading-padding, v.$ms-loading-padding);
403403
.ms-icon-loading {
404404
font-size: var(--ms-loading-icon-size, v.$ms-loading-icon-size);
405+
height: var(--ms-loading-icon-size, v.$ms-loading-icon-size);
406+
width: var(--ms-loading-icon-size, v.$ms-loading-icon-size);
405407
}
406408
}
407409

playwright/e2e/options42.spec.ts

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ test.describe('Options 42 - Lazy Load Data', () => {
7171
await page.locator('div[data-test=select2].ms-parent').click();
7272
await page.locator('div[data-test=select2].ms-parent').click();
7373

74-
// 3th select
74+
// 3rd select
7575
await expect(await page.locator('div[data-test=select3] .ms-choice')).toHaveText('');
7676
await page.locator('div[data-test=select3].ms-parent').click();
7777
let loading3Loc = await page.locator('.ms-loading');
@@ -101,5 +101,47 @@ test.describe('Options 42 - Lazy Load Data', () => {
101101
await expect(loading1Loc).not.toBeVisible();
102102
await page.locator('div[data-test=select2].ms-parent').click();
103103
await page.locator('div[data-test=select2].ms-parent').click();
104+
105+
// --
106+
// 2nd select
107+
await expect(await page.locator('div[data-test=select4] .ms-choice')).toHaveText('');
108+
await page.locator('div[data-test=select4].ms-parent').click();
109+
let loading4Loc = await page.locator('.ms-loading');
110+
await expect(loading4Loc).toBeVisible();
111+
await expect(loading4Loc).toContainText('Loading...');
112+
await page.waitForTimeout(200);
113+
loading4Loc = await page.locator('.ms-loading');
114+
await expect(loading4Loc).not.toBeVisible();
115+
116+
await expect(await page.locator('div[data-test=select4] .ms-choice')).toHaveText('February, April');
117+
let select4LiElms = await page.locator('div[data-test=select4] li[role="option"]');
118+
await expect(select4LiElms).toHaveCount(12);
119+
const select4li1 = await page.locator('div[data-test=select4] .ms-drop li:visible').nth(0);
120+
await expect(await select4li1.locator('label')).toHaveText('January');
121+
const select4li2 = await page.locator('div[data-test=select4] .ms-drop li:visible').nth(1);
122+
await expect(await select4li2.locator('label')).toHaveText('February');
123+
const select4li3 = await page.locator('div[data-test=select4] .ms-drop li:visible').nth(2);
124+
await expect(await select4li3.locator('label')).toHaveText('March');
125+
await page.locator('div[data-test=select4].ms-parent').click();
126+
await expect(await page.locator('div[data-test=select4] .ms-drop .ms-ok-button')).toHaveText('OK');
127+
128+
// 4nd select, open/close multiple times should now be instant without any lazy reloading
129+
await page.locator('div[data-test=select4].ms-parent').click();
130+
loading1Loc = await page.locator('.ms-loading');
131+
await expect(loading1Loc).not.toBeVisible();
132+
select4LiElms = await page.locator('div[data-test=select4] li[role="option"]');
133+
await expect(select4LiElms).toHaveCount(12);
134+
await page.locator('div[data-test=select4].ms-parent').click();
135+
loading1Loc = await page.locator('.ms-loading');
136+
await expect(loading1Loc).not.toBeVisible();
137+
await page.locator('div[data-test=select4].ms-parent').click();
138+
139+
const placeholderLocator = await page.getByPlaceholder('filter placeholder');
140+
expect(placeholderLocator).toHaveCount(1);
141+
placeholderLocator.focus();
142+
await page.keyboard.type('april');
143+
select4LiElms = await page.locator('div[data-test=select4] li[role="option"]');
144+
await expect(select4LiElms).toHaveCount(1);
145+
await page.locator('div[data-test=select4].ms-parent').click();
104146
});
105147
});

0 commit comments

Comments
 (0)