Skip to content

Commit

Permalink
fix(picker): allow menu items to be added, updated, and removed
Browse files Browse the repository at this point in the history
  • Loading branch information
@Westbrook
Westbrook committed Nov 9, 2021
1 parent 25c780b commit 73511ba
Show file tree
Hide file tree
Showing 6 changed files with 168 additions and 26 deletions.
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@
"rollup": "2.56.2",
"rollup-plugin-workbox": "6.1.1"
},
"customElements": "projects/documentation/custom-elements.json",
"customElements": ".storybook/custom-elements.json",
"workspaces": [
"packages/*",
"projects/*"
Expand Down
26 changes: 22 additions & 4 deletions packages/menu/src/MenuItem.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ export class MenuItemAddedOrUpdatedEvent extends Event {
}
}

export type MenuItemChildren = { icon: Element[]; content: Node[] };

const addOrUpdateEvent = new MenuItemAddedOrUpdatedEvent();
const removeEvent = new MenuItemRemovedEvent();

Expand Down Expand Up @@ -152,7 +154,11 @@ export class MenuItem extends LikeAnchor(Focusable) {
return this;
}

public get itemChildren(): { icon: Element[]; content: Node[] } {
public get itemChildren(): MenuItemChildren {
if (this._itemChildren) {
return this._itemChildren;
}

const iconSlot = this.shadowRoot.querySelector(
'slot[name="icon"]'
) as HTMLSlotElement;
Expand All @@ -170,9 +176,13 @@ export class MenuItem extends LikeAnchor(Focusable) {
const content = !contentSlot
? []
: contentSlot.assignedNodes().map((node) => node.cloneNode(true));
return { icon, content };
this._itemChildren = { icon, content };

return this._itemChildren;
}

private _itemChildren?: MenuItemChildren;

constructor() {
super();
this.proxyFocus = this.proxyFocus.bind(this);
Expand Down Expand Up @@ -216,11 +226,19 @@ export class MenuItem extends LikeAnchor(Focusable) {
return handled;
}

protected breakItemChildrenCache(): void {
this._itemChildren = undefined;
this.triggerUpdate();
}

protected render(): TemplateResult {
return html`
<slot name="icon"></slot>
<slot name="icon" @slotchange=${this.breakItemChildrenCache}></slot>
<div id="label">
<slot id="slot"></slot>
<slot
id="slot"
@slotchange=${this.breakItemChildrenCache}
></slot>
</div>
<slot name="value"></slot>
${this.selected
Expand Down
36 changes: 19 additions & 17 deletions packages/picker/src/Picker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,13 @@ import { reparentChildren } from '@spectrum-web-components/shared/src/reparent-c
import '@spectrum-web-components/icons-ui/icons/sp-icon-chevron100.js';
import '@spectrum-web-components/icons-workflow/icons/sp-icon-alert.js';
import '@spectrum-web-components/menu/sp-menu.js';
import { Menu, MenuItem } from '@spectrum-web-components/menu';
import type {
Menu,
MenuItem,
MenuItemAddedOrUpdatedEvent,
MenuItemChildren,
MenuItemRemovedEvent,
} from '@spectrum-web-components/menu';
import '@spectrum-web-components/popover/sp-popover.js';
import { Popover } from '@spectrum-web-components/popover';
import {
Expand Down Expand Up @@ -326,21 +332,13 @@ export class PickerBase extends SizedMixin(Focusable) {
}
}

protected get selectedItemContent(): {
icon: Element[];
content: Node[];
} {
if (!this._selectedItemContent && this.selectedItem) {
this._selectedItemContent = this.selectedItem.itemChildren;
protected get selectedItemContent(): MenuItemChildren {
if (this.selectedItem) {
return this.selectedItem.itemChildren;
}
return this._selectedItemContent || { icon: [], content: [] };
return { icon: [], content: [] };
}

private _selectedItemContent?: {
icon: Element[];
content: Node[];
};

protected renderLabelContent(content: Node[]): TemplateResult | Node[] {
if (this.value && this.selectedItem) {
return content;
Expand Down Expand Up @@ -404,9 +402,6 @@ export class PickerBase extends SizedMixin(Focusable) {
}

protected update(changes: PropertyValues<this>): void {
if (changes.has('selectedItem')) {
this._selectedItemContent = undefined;
}
if (this.selects) {
// Always force `selects` to "single" when set.
// TODO: Add support functionally and visually for "multiple"
Expand Down Expand Up @@ -438,6 +433,7 @@ export class PickerBase extends SizedMixin(Focusable) {
<sp-popover
id="popover"
role="dialog"
@sp-menu-item-added-or-updated=${this.updateMenuItems}
@sp-overlay-closed=${this.onOverlayClosed}
.overlayCloseCallback=${this.overlayCloseCallback}
>
Expand All @@ -461,9 +457,14 @@ export class PickerBase extends SizedMixin(Focusable) {
* direct element query or by assuming the list managed
* by the Menu within the open options overlay.
*/
protected updateMenuItems(): void {
protected updateMenuItems(
event?: MenuItemAddedOrUpdatedEvent | MenuItemRemovedEvent
): void {
if (this._willUpdateItems) return;
this._willUpdateItems = true;
if (event?.item === this.selectedItem) {
this.requestUpdate();
}

let resolve = (): void => {
return;
Expand Down Expand Up @@ -565,6 +566,7 @@ export class PickerBase extends SizedMixin(Focusable) {
'sp-menu-item-added-or-updated',
this.updateMenuItems
);
this.addEventListener('sp-menu-item-removed', this.updateMenuItems);
super.connectedCallback();
}

Expand Down
61 changes: 61 additions & 0 deletions packages/picker/test/picker-sync.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,67 @@ describe('Picker, sync', () => {
it('loads accessibly', async () => {
await expect(el).to.be.accessible();
});
it('accepts new selected item content', async () => {
const option2 = el.querySelector('[value="option-2"') as MenuItem;
el.value = 'option-2';
await elementUpdated(el);
expect(el.value).to.equal('option-2');
expect((el.button.textContent || '').trim()).to.equal(
'Select Inverse'
);
const itemUpdated = oneEvent(el, 'sp-menu-item-added-or-updated');
option2.innerHTML = 'Invert Selection';
await itemUpdated;
await elementUpdated(el);
expect(el.value).to.equal('option-2');
expect((el.button.textContent || '').trim()).to.equal(
'Invert Selection'
);
});
it('accepts new selected item content when open', async () => {
const option2 = el.querySelector('[value="option-2"') as MenuItem;
el.value = 'option-2';
await elementUpdated(el);
expect(el.value).to.equal('option-2');
expect((el.button.textContent || '').trim()).to.equal(
'Select Inverse'
);
const opened = oneEvent(el, 'sp-opened');
el.open = true;
await opened;
const itemUpdated = oneEvent(
option2,
'sp-menu-item-added-or-updated'
);
option2.innerHTML = 'Invert Selection';
await itemUpdated;
await elementUpdated(el);
expect(el.value).to.equal('option-2');
expect((el.button.textContent || '').trim()).to.equal(
'Invert Selection'
);
});
it('unsets value when children removed', async () => {
el.value = 'option-2';

await elementUpdated(el);
expect(el.value).to.equal('option-2');
expect((el.button.textContent || '').trim()).to.equal(
'Select Inverse'
);

const items = el.querySelectorAll('sp-menu-item');
const removals: Promise<unknown>[] = [];
items.forEach((item) => {
const removal = oneEvent(el, 'sp-menu-item-removed');
item.remove();
removals.push(removal);
});
await Promise.all(removals);
await elementUpdated(el);
expect(el.value).to.equal('');
expect((el.button.textContent || '').trim()).to.equal('');
});
it('accepts a new item and value at the same time', async () => {
el.value = 'option-2';

Expand Down
61 changes: 61 additions & 0 deletions packages/picker/test/picker.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,67 @@ describe('Picker, sync', () => {
it('loads accessibly', async () => {
await expect(el).to.be.accessible();
});
it('accepts new selected item content', async () => {
const option2 = el.querySelector('[value="option-2"') as MenuItem;
el.value = 'option-2';
await elementUpdated(el);
expect(el.value).to.equal('option-2');
expect((el.button.textContent || '').trim()).to.equal(
'Select Inverse'
);
const itemUpdated = oneEvent(el, 'sp-menu-item-added-or-updated');
option2.innerHTML = 'Invert Selection';
await itemUpdated;
await elementUpdated(el);
expect(el.value).to.equal('option-2');
expect((el.button.textContent || '').trim()).to.equal(
'Invert Selection'
);
});
it('accepts new selected item content when open', async () => {
const option2 = el.querySelector('[value="option-2"') as MenuItem;
el.value = 'option-2';
await elementUpdated(el);
expect(el.value).to.equal('option-2');
expect((el.button.textContent || '').trim()).to.equal(
'Select Inverse'
);
const opened = oneEvent(el, 'sp-opened');
el.open = true;
await opened;
const itemUpdated = oneEvent(
option2,
'sp-menu-item-added-or-updated'
);
option2.innerHTML = 'Invert Selection';
await itemUpdated;
await elementUpdated(el);
expect(el.value).to.equal('option-2');
expect((el.button.textContent || '').trim()).to.equal(
'Invert Selection'
);
});
it('unsets value when children removed', async () => {
el.value = 'option-2';

await elementUpdated(el);
expect(el.value).to.equal('option-2');
expect((el.button.textContent || '').trim()).to.equal(
'Select Inverse'
);

const items = el.querySelectorAll('sp-menu-item');
const removals: Promise<unknown>[] = [];
items.forEach((item) => {
const removal = oneEvent(el, 'sp-menu-item-removed');
item.remove();
removals.push(removal);
});
await Promise.all(removals);
await elementUpdated(el);
expect(el.value).to.equal('');
expect((el.button.textContent || '').trim()).to.equal('');
});
it('accepts a new item and value at the same time', async () => {
el.value = 'option-2';

Expand Down
8 changes: 4 additions & 4 deletions web-test-runner.config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ governing permissions and limitations under the License.
*/
import { playwrightLauncher } from '@web/test-runner-playwright';
import {
sendKeysPlugin,
a11ySnapshotPlugin,
sendKeysPlugin,
} from '@web/test-runner-commands/plugins';
import { sendMousePlugin } from './test/plugins/send-mouse-plugin.js';
import {
configuredVisualRegressionPlugin,
packages,
vrtGroups,
configuredVisualRegressionPlugin,
} from './web-test-runner.utils.js';
import { fromRollup } from '@web/dev-server-rollup';
import rollupJson from '@rollup/plugin-json';
Expand All @@ -44,7 +44,7 @@ export default {
nodeResolve: true,
concurrency: 4,
concurrentBrowsers: 1,
testsFinishTimeout: 20000,
testsFinishTimeout: 30000,
coverageConfig: {
report: true,
reportDir: 'coverage',
Expand All @@ -67,7 +67,7 @@ export default {
},
testFramework: {
config: {
timeout: 10000,
timeout: 5000,
},
},
groups: [
Expand Down

0 comments on commit 73511ba

Please sign in to comment.