Skip to content

Commit

Permalink
fix: add value/selection checks to the tests and fix up the value logic
Browse files Browse the repository at this point in the history
  • Loading branch information
@adixon-adobe @Westbrook
adixon-adobe authored and Westbrook committed Jul 29, 2021
1 parent 270cae1 commit 5dc1e0a
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 16 deletions.
29 changes: 13 additions & 16 deletions packages/menu/src/Menu.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ export class Menu extends SpectrumElement {
this.removeEventListener('keydown', this.handleKeydown);
}

public async selectOrToggleItem(item: MenuItem): Promise<void> {
public async selectOrToggleItem(targetItem: MenuItem): Promise<void> {
const resolvedSelects = this.resolvedSelects;
if (resolvedSelects === 'none') {
return;
Expand All @@ -309,32 +309,28 @@ export class Menu extends SpectrumElement {

if (resolvedSelects === 'single') {
this.selectedItemsMap.clear();
this.selectedItemsMap.set(item, true);
this.value = item.value;
this.selected = [item.value];
this.selectedItems = [item];
this.selectedItemsMap.set(targetItem, true);
this.value = targetItem.value;
this.selected = [targetItem.value];
this.selectedItems = [targetItem];
} else {
if (this.selectedItemsMap.has(item)) {
this.selectedItemsMap.delete(item);
if (this.selectedItemsMap.has(targetItem)) {
this.selectedItemsMap.delete(targetItem);
} else {
this.selectedItemsMap.set(item, true);
this.selectedItemsMap.set(targetItem, true);
}

// Match HTML select and set the first selected
// item as the value. Also set the selected array
// in the order of the menu items.
let valueSet = false;
const selected: string[] = [];
const selectedItems: MenuItem[] = [];

for (const childItem of this.childItems) {
if (!childItem.managed) continue;

if (this.selectedItemsMap.has(childItem.menuItem)) {
if (!valueSet) {
this.value = childItem.menuItem.value;
valueSet = true;
}
const item = childItem.menuItem;
selected.push(item.value);
selectedItems.push(item);
}
Expand Down Expand Up @@ -362,13 +358,13 @@ export class Menu extends SpectrumElement {
// Apply the selection changes to the menu items
if (resolvedSelects === 'single') {
for (const oldItem of oldSelectedItemsMap.keys()) {
if (oldItem !== item) {
if (oldItem !== targetItem) {
oldItem.selected = false;
}
}
item.selected = true;
targetItem.selected = true;
} else if (resolvedSelects === 'multiple') {
item.selected = !item.selected;
targetItem.selected = !targetItem.selected;
}
}

Expand Down Expand Up @@ -465,6 +461,7 @@ export class Menu extends SpectrumElement {
this.selectedItemsMap = selectedItemsMap;
this.selected = selected;
this.selectedItems = selectedItems;
this.value = this.selected.join(this.valueSeparator);
this.focusedItemIndex = index;
this.focusInItemIndex = index;
}
Expand Down
14 changes: 14 additions & 0 deletions packages/menu/test/menu-group.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ describe('Menu group', () => {
expect(singleItem1.selected).to.be.true;
expect(firstItem.selected).to.be.true;
expect(secondItem.selected).to.be.false;
expect(el.value).to.equal('First');
expect(el.selectedItems.length).to.equal(1);

expect(firstItem.getAttribute('aria-checked')).to.equal('true');
expect(secondItem.getAttribute('aria-checked')).to.equal('false');
Expand All @@ -184,6 +186,8 @@ describe('Menu group', () => {
expect(secondItem.selected).to.be.true;
expect(firstItem.getAttribute('aria-checked')).to.equal('false');
expect(secondItem.getAttribute('aria-checked')).to.equal('true');
expect(el.value).to.equal('Second');
expect(el.selectedItems.length).to.equal(1);

inheritItem1.click();
await elementUpdated(el);
Expand All @@ -193,12 +197,16 @@ describe('Menu group', () => {
expect(inheritItem1.selected).to.be.true;
expect(secondItem.getAttribute('aria-checked')).to.equal('false');
expect(inheritItem1.getAttribute('aria-checked')).to.equal('true');
expect(el.value).to.equal('Inherit1');
expect(el.selectedItems.length).to.equal(1);

noneItem2.click();
await elementUpdated(el);
await elementUpdated(noneItem2);
expect(inheritItem1.selected).to.be.true;
expect(noneItem2.selected).to.be.false;
expect(el.value).to.equal('Inherit1');
expect(el.selectedItems.length).to.equal(1);

singleItem2.click();
await elementUpdated(el);
Expand All @@ -209,6 +217,10 @@ describe('Menu group', () => {
expect(inheritItem1.selected).to.be.true;
expect(singleItem1.getAttribute('aria-checked')).to.equal('false');
expect(singleItem2.getAttribute('aria-checked')).to.equal('true');
expect(el.value).to.equal('Inherit1');
expect(el.selectedItems.length).to.equal(1);
//expect(singleGroup.value).to.equal('Inherit1')
expect(singleGroup.selectedItems.length).to.equal(1);

multiItem2.click();
await elementUpdated(el);
Expand All @@ -218,6 +230,8 @@ describe('Menu group', () => {
expect(inheritItem1.selected).to.be.true;
expect(multiItem1.getAttribute('aria-checked')).to.equal('true');
expect(multiItem2.getAttribute('aria-checked')).to.equal('true');
//expect(multiGroup.value).to.equal('Inherit1')
expect(multiGroup.selectedItems.length).to.equal(2);
});

it('handles changing managed items for selects changes', async () => {
Expand Down
8 changes: 8 additions & 0 deletions packages/menu/test/menu.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ describe('Menu', () => {
expect(secondItem.selected).to.be.false;
expect(firstItem.getAttribute('aria-checked')).to.equal('true');
expect(secondItem.getAttribute('aria-checked')).to.equal('false');
expect(el.value).to.equal('First');

secondItem.click();

Expand All @@ -409,6 +410,7 @@ describe('Menu', () => {
expect(secondItem.selected).to.be.true;
expect(firstItem.getAttribute('aria-checked')).to.equal('false');
expect(secondItem.getAttribute('aria-checked')).to.equal('true');
expect(el.value).to.equal('Second');
});
it('handles multiple selection', async () => {
const el = await fixture<Menu>(
Expand Down Expand Up @@ -442,6 +444,8 @@ describe('Menu', () => {
expect(secondItem.selected).to.be.false;
expect(firstItem.getAttribute('aria-checked')).to.equal('true');
expect(secondItem.getAttribute('aria-checked')).to.equal('false');
expect(el.value).to.equal('First');
expect(el.selectedItems.length).to.equal(1);

secondItem.click();

Expand All @@ -453,6 +457,8 @@ describe('Menu', () => {
expect(secondItem.selected).to.be.true;
expect(firstItem.getAttribute('aria-checked')).to.equal('true');
expect(secondItem.getAttribute('aria-checked')).to.equal('true');
expect(el.value).to.equal('First,Second');
expect(el.selectedItems.length).to.equal(2);

firstItem.click();

Expand All @@ -464,5 +470,7 @@ describe('Menu', () => {
expect(secondItem.selected).to.be.true;
expect(firstItem.getAttribute('aria-checked')).to.equal('false');
expect(secondItem.getAttribute('aria-checked')).to.equal('true');
expect(el.value).to.equal('Second');
expect(el.selectedItems.length).to.equal(1);
});
});

0 comments on commit 5dc1e0a

Please sign in to comment.