Skip to content

Commit 9232eb1

Browse files
WestbrookWestbrook
committed
fix: include late added items in the item list for the Picker
1 parent 972ac68 commit 9232eb1

File tree

3 files changed

+145
-42
lines changed

3 files changed

+145
-42
lines changed

‎packages/picker/src/Picker.ts

+67-40
Original file line numberDiff line numberDiff line change
@@ -207,15 +207,19 @@ export class PickerBase extends SizedMixin(Focusable) {
207207
if (menuChangeEvent) {
208208
menuChangeEvent.preventDefault();
209209
}
210+
this.selectedItem.selected = false;
211+
if (oldSelectedItem) {
212+
oldSelectedItem.selected = true;
213+
}
210214
this.selectedItem = oldSelectedItem;
211215
this.value = oldValue;
212216
this.open = true;
213217
return;
214218
}
215219
if (oldSelectedItem) {
216-
oldSelectedItem.selected = !!this.selects ? false : false;
220+
oldSelectedItem.selected = false;
217221
}
218-
item.selected = !!this.selects ? true : false;
222+
item.selected = !!this.selects;
219223
}
220224

221225
public toggle(target?: boolean): void {
@@ -276,11 +280,13 @@ export class PickerBase extends SizedMixin(Focusable) {
276280
const { popover } = this;
277281
this.addEventListener(
278282
'sp-opened',
279-
() => {
280-
this.manageSelection();
281-
this.optionsMenu.updateComplete.then(() => {
282-
this.menuStateResolver();
283-
});
283+
async () => {
284+
this.updateMenuItems();
285+
await Promise.all([
286+
this.itemsUpdated,
287+
this.optionsMenu.updateComplete,
288+
]);
289+
this.menuStateResolver();
284290
},
285291
{ once: true }
286292
);
@@ -411,10 +417,37 @@ export class PickerBase extends SizedMixin(Focusable) {
411417
`;
412418
}
413419

420+
private _willUpdateItems = false;
421+
protected itemsUpdated: Promise<void> = Promise.resolve();
422+
423+
/**
424+
* Acquire the available MenuItems in the Picker by
425+
* direct element query or by assuming the list managed
426+
* by the Menu within the open options overlay.
427+
*/
414428
protected updateMenuItems(): void {
415-
this.menuItems = [
416-
...this.querySelectorAll('sp-menu-item'),
417-
] as MenuItem[];
429+
if (this._willUpdateItems) return;
430+
this._willUpdateItems = true;
431+
432+
let resolve = (): void => {
433+
return;
434+
};
435+
this.itemsUpdated = new Promise((res) => (resolve = res));
436+
// Debounce the update so we only update once
437+
// if multiple items have changed
438+
window.requestAnimationFrame(async () => {
439+
if (this.open) {
440+
await this.optionsMenu.updateComplete;
441+
this.menuItems = this.optionsMenu.childItems;
442+
} else {
443+
this.menuItems = [
444+
...this.querySelectorAll('sp-menu-item'),
445+
] as MenuItem[];
446+
}
447+
this.manageSelection();
448+
resolve();
449+
this._willUpdateItems = false;
450+
});
418451
}
419452

420453
protected firstUpdated(changedProperties: PropertyValues): void {
@@ -423,8 +456,6 @@ export class PickerBase extends SizedMixin(Focusable) {
423456
// Since the sp-menu gets reparented by the popover, initialize it here
424457
this.optionsMenu = this.shadowRoot.querySelector('sp-menu') as Menu;
425458

426-
this.updateMenuItems();
427-
428459
const deprecatedMenu = this.querySelector('sp-menu');
429460
if (deprecatedMenu) {
430461
console.warn(
@@ -439,7 +470,7 @@ export class PickerBase extends SizedMixin(Focusable) {
439470
changedProperties.has('value') &&
440471
!changedProperties.has('selectedItem')
441472
) {
442-
this.manageSelection();
473+
this.updateMenuItems();
443474
}
444475
if (changedProperties.has('disabled') && this.disabled) {
445476
this.open = false;
@@ -460,32 +491,25 @@ export class PickerBase extends SizedMixin(Focusable) {
460491
}
461492

462493
protected manageSelection(): void {
463-
if (!this.open) {
464-
this.updateMenuItems();
465-
}
466-
/* c8 ignore next 3 */
467-
if (this.menuItems.length > 0) {
468-
let selectedItem: MenuItem | undefined;
469-
this.menuItems.forEach((item) => {
470-
if (this.value === item.value && !item.disabled) {
471-
selectedItem = item;
472-
} else {
473-
item.selected = !!this.selects ? false : false;
474-
}
475-
});
476-
if (selectedItem) {
477-
selectedItem.selected = !!this.selects ? true : false;
478-
this.selectedItem = selectedItem;
494+
let selectedItem: MenuItem | undefined;
495+
this.menuItems.forEach((item) => {
496+
if (this.value === item.value && !item.disabled) {
497+
selectedItem = item;
479498
} else {
480-
this.value = '';
481-
this.selectedItem = undefined;
482-
}
483-
if (this.open) {
484-
this.optionsMenu.updateComplete.then(() => {
485-
this.optionsMenu.updateSelectedItemIndex();
486-
});
499+
item.selected = false;
487500
}
488-
return;
501+
});
502+
if (selectedItem) {
503+
selectedItem.selected = !!this.selects;
504+
this.selectedItem = selectedItem;
505+
} else {
506+
this.value = '';
507+
this.selectedItem = undefined;
508+
}
509+
if (this.open) {
510+
this.optionsMenu.updateComplete.then(() => {
511+
this.optionsMenu.updateSelectedItemIndex();
512+
});
489513
}
490514
}
491515

@@ -495,13 +519,16 @@ export class PickerBase extends SizedMixin(Focusable) {
495519
protected async _getUpdateComplete(): Promise<boolean> {
496520
const complete = (await super._getUpdateComplete()) as boolean;
497521
await this.menuStatePromise;
522+
await this.itemsUpdated;
498523
return complete;
499524
}
500525

501526
public connectedCallback(): void {
502-
if (!this.open) {
503-
this.updateMenuItems();
504-
}
527+
this.updateMenuItems();
528+
this.addEventListener(
529+
'sp-menu-item-added-or-updated',
530+
this.updateMenuItems
531+
);
505532
super.connectedCallback();
506533
}
507534

‎packages/picker/test/picker-sync.test.ts

+39-1
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,6 @@ describe('Picker, sync', () => {
166166

167167
await expect(el).to.be.accessible();
168168
});
169-
170169
it('accepts a new item and value at the same time', async () => {
171170
const el = await pickerFixture();
172171

@@ -182,11 +181,50 @@ describe('Picker, sync', () => {
182181
item.textContent = 'New Option';
183182

184183
el.append(item);
184+
await elementUpdated(el);
185+
185186
el.value = 'option-new';
186187

187188
await elementUpdated(el);
188189
expect(el.value).to.equal('option-new');
189190
});
191+
it('accepts a new item that can be selected', async () => {
192+
const el = await pickerFixture();
193+
194+
await elementUpdated(el);
195+
196+
el.value = 'option-2';
197+
198+
await elementUpdated(el);
199+
expect(el.value).to.equal('option-2');
200+
const item = document.createElement('sp-menu-item');
201+
item.value = 'option-new';
202+
item.textContent = 'New Option';
203+
204+
el.append(item);
205+
206+
await elementUpdated(item);
207+
await elementUpdated(el);
208+
209+
let opened = oneEvent(el, 'sp-opened');
210+
el.open = true;
211+
await opened;
212+
// Overlaid content is outside of the context of the Picker element
213+
// and cannot be managed via its updateComplete cycle.
214+
await nextFrame();
215+
216+
const close = oneEvent(el, 'sp-closed');
217+
item.click();
218+
await close;
219+
220+
expect(el.value, 'first time').to.equal('option-new');
221+
222+
opened = oneEvent(el, 'sp-opened');
223+
el.open = true;
224+
await opened;
225+
226+
expect(el.value, 'second time').to.equal('option-new');
227+
});
190228
it('manages its "name" value in the accessibility tree', async () => {
191229
const el = await pickerFixture();
192230

‎packages/picker/test/picker.test.ts

+39-1
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,6 @@ describe('Picker', () => {
166166

167167
await expect(el).to.be.accessible();
168168
});
169-
170169
it('accepts a new item and value at the same time', async () => {
171170
const el = await pickerFixture();
172171

@@ -182,11 +181,50 @@ describe('Picker', () => {
182181
item.textContent = 'New Option';
183182

184183
el.append(item);
184+
await elementUpdated(el);
185+
185186
el.value = 'option-new';
186187

187188
await elementUpdated(el);
188189
expect(el.value).to.equal('option-new');
189190
});
191+
it('accepts a new item that can be selected', async () => {
192+
const el = await pickerFixture();
193+
194+
await elementUpdated(el);
195+
196+
el.value = 'option-2';
197+
198+
await elementUpdated(el);
199+
expect(el.value).to.equal('option-2');
200+
const item = document.createElement('sp-menu-item');
201+
item.value = 'option-new';
202+
item.textContent = 'New Option';
203+
204+
el.append(item);
205+
206+
await elementUpdated(item);
207+
await elementUpdated(el);
208+
209+
let opened = oneEvent(el, 'sp-opened');
210+
el.open = true;
211+
await opened;
212+
// Overlaid content is outside of the context of the Picker element
213+
// and cannot be managed via its updateComplete cycle.
214+
await nextFrame();
215+
216+
const close = oneEvent(el, 'sp-closed');
217+
item.click();
218+
await close;
219+
220+
expect(el.value, 'first time').to.equal('option-new');
221+
222+
opened = oneEvent(el, 'sp-opened');
223+
el.open = true;
224+
await opened;
225+
226+
expect(el.value, 'second time').to.equal('option-new');
227+
});
190228
it('manages its "name" value in the accessibility tree', async () => {
191229
const el = await pickerFixture();
192230

0 commit comments

Comments
 (0)