Skip to content

Commit

Permalink
fix: include late added items in the item list for the Picker
Browse files Browse the repository at this point in the history
  • Loading branch information
@Westbrook
Westbrook committed Oct 28, 2021
1 parent 972ac68 commit 9232eb1
Show file tree
Hide file tree
Showing 3 changed files with 145 additions and 42 deletions.
107 changes: 67 additions & 40 deletions packages/picker/src/Picker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,15 +207,19 @@ export class PickerBase extends SizedMixin(Focusable) {
if (menuChangeEvent) {
menuChangeEvent.preventDefault();
}
this.selectedItem.selected = false;
if (oldSelectedItem) {
oldSelectedItem.selected = true;
}
this.selectedItem = oldSelectedItem;
this.value = oldValue;
this.open = true;
return;
}
if (oldSelectedItem) {
oldSelectedItem.selected = !!this.selects ? false : false;
oldSelectedItem.selected = false;
}
item.selected = !!this.selects ? true : false;
item.selected = !!this.selects;
}

public toggle(target?: boolean): void {
Expand Down Expand Up @@ -276,11 +280,13 @@ export class PickerBase extends SizedMixin(Focusable) {
const { popover } = this;
this.addEventListener(
'sp-opened',
() => {
this.manageSelection();
this.optionsMenu.updateComplete.then(() => {
this.menuStateResolver();
});
async () => {
this.updateMenuItems();
await Promise.all([
this.itemsUpdated,
this.optionsMenu.updateComplete,
]);
this.menuStateResolver();
},
{ once: true }
);
Expand Down Expand Up @@ -411,10 +417,37 @@ export class PickerBase extends SizedMixin(Focusable) {
`;
}

private _willUpdateItems = false;
protected itemsUpdated: Promise<void> = Promise.resolve();

/**
* Acquire the available MenuItems in the Picker by
* direct element query or by assuming the list managed
* by the Menu within the open options overlay.
*/
protected updateMenuItems(): void {
this.menuItems = [
...this.querySelectorAll('sp-menu-item'),
] as MenuItem[];
if (this._willUpdateItems) return;
this._willUpdateItems = true;

let resolve = (): void => {
return;
};
this.itemsUpdated = new Promise((res) => (resolve = res));
// Debounce the update so we only update once
// if multiple items have changed
window.requestAnimationFrame(async () => {
if (this.open) {
await this.optionsMenu.updateComplete;
this.menuItems = this.optionsMenu.childItems;
} else {
this.menuItems = [
...this.querySelectorAll('sp-menu-item'),
] as MenuItem[];
}
this.manageSelection();
resolve();
this._willUpdateItems = false;
});
}

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

this.updateMenuItems();

const deprecatedMenu = this.querySelector('sp-menu');
if (deprecatedMenu) {
console.warn(
Expand All @@ -439,7 +470,7 @@ export class PickerBase extends SizedMixin(Focusable) {
changedProperties.has('value') &&
!changedProperties.has('selectedItem')
) {
this.manageSelection();
this.updateMenuItems();
}
if (changedProperties.has('disabled') && this.disabled) {
this.open = false;
Expand All @@ -460,32 +491,25 @@ export class PickerBase extends SizedMixin(Focusable) {
}

protected manageSelection(): void {
if (!this.open) {
this.updateMenuItems();
}
/* c8 ignore next 3 */
if (this.menuItems.length > 0) {
let selectedItem: MenuItem | undefined;
this.menuItems.forEach((item) => {
if (this.value === item.value && !item.disabled) {
selectedItem = item;
} else {
item.selected = !!this.selects ? false : false;
}
});
if (selectedItem) {
selectedItem.selected = !!this.selects ? true : false;
this.selectedItem = selectedItem;
let selectedItem: MenuItem | undefined;
this.menuItems.forEach((item) => {
if (this.value === item.value && !item.disabled) {
selectedItem = item;
} else {
this.value = '';
this.selectedItem = undefined;
}
if (this.open) {
this.optionsMenu.updateComplete.then(() => {
this.optionsMenu.updateSelectedItemIndex();
});
item.selected = false;
}
return;
});
if (selectedItem) {
selectedItem.selected = !!this.selects;
this.selectedItem = selectedItem;
} else {
this.value = '';
this.selectedItem = undefined;
}
if (this.open) {
this.optionsMenu.updateComplete.then(() => {
this.optionsMenu.updateSelectedItemIndex();
});
}
}

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

public connectedCallback(): void {
if (!this.open) {
this.updateMenuItems();
}
this.updateMenuItems();
this.addEventListener(
'sp-menu-item-added-or-updated',
this.updateMenuItems
);
super.connectedCallback();
}

Expand Down
40 changes: 39 additions & 1 deletion packages/picker/test/picker-sync.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ describe('Picker, sync', () => {

await expect(el).to.be.accessible();
});

it('accepts a new item and value at the same time', async () => {
const el = await pickerFixture();

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

el.append(item);
await elementUpdated(el);

el.value = 'option-new';

await elementUpdated(el);
expect(el.value).to.equal('option-new');
});
it('accepts a new item that can be selected', async () => {
const el = await pickerFixture();

await elementUpdated(el);

el.value = 'option-2';

await elementUpdated(el);
expect(el.value).to.equal('option-2');
const item = document.createElement('sp-menu-item');
item.value = 'option-new';
item.textContent = 'New Option';

el.append(item);

await elementUpdated(item);
await elementUpdated(el);

let opened = oneEvent(el, 'sp-opened');
el.open = true;
await opened;
// Overlaid content is outside of the context of the Picker element
// and cannot be managed via its updateComplete cycle.
await nextFrame();

const close = oneEvent(el, 'sp-closed');
item.click();
await close;

expect(el.value, 'first time').to.equal('option-new');

opened = oneEvent(el, 'sp-opened');
el.open = true;
await opened;

expect(el.value, 'second time').to.equal('option-new');
});
it('manages its "name" value in the accessibility tree', async () => {
const el = await pickerFixture();

Expand Down
40 changes: 39 additions & 1 deletion packages/picker/test/picker.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ describe('Picker', () => {

await expect(el).to.be.accessible();
});

it('accepts a new item and value at the same time', async () => {
const el = await pickerFixture();

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

el.append(item);
await elementUpdated(el);

el.value = 'option-new';

await elementUpdated(el);
expect(el.value).to.equal('option-new');
});
it('accepts a new item that can be selected', async () => {
const el = await pickerFixture();

await elementUpdated(el);

el.value = 'option-2';

await elementUpdated(el);
expect(el.value).to.equal('option-2');
const item = document.createElement('sp-menu-item');
item.value = 'option-new';
item.textContent = 'New Option';

el.append(item);

await elementUpdated(item);
await elementUpdated(el);

let opened = oneEvent(el, 'sp-opened');
el.open = true;
await opened;
// Overlaid content is outside of the context of the Picker element
// and cannot be managed via its updateComplete cycle.
await nextFrame();

const close = oneEvent(el, 'sp-closed');
item.click();
await close;

expect(el.value, 'first time').to.equal('option-new');

opened = oneEvent(el, 'sp-opened');
el.open = true;
await opened;

expect(el.value, 'second time').to.equal('option-new');
});
it('manages its "name" value in the accessibility tree', async () => {
const el = await pickerFixture();

Expand Down

0 comments on commit 9232eb1

Please sign in to comment.