fix(action-menu): never set item selected values when selects is unde…

…fined
adobe · Sep 15, 2022 · 5237fdb · 5237fdb
1 parent 6df9f58
commit 5237fdb
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 7 deletions.
diff --git a/packages/action-menu/test/action-menu.test.ts b/packages/action-menu/test/action-menu.test.ts
@@ -271,9 +271,11 @@ describe('Action menu', () => {
         const root = await actionSubmenuFixture();
         const unselectedItem = root.querySelector('sp-menu-item') as MenuItem;
         const selectedItem = root.querySelector('.selected-item') as MenuItem;
+        let selected = true;
 
         selectedItem.addEventListener('click', () => {
-            selectedItem.selected = false;
+            selected = !selected;
+            selectedItem.selected = selected;
         });
 
         expect(unselectedItem.innerHTML).to.equal('One');
@@ -309,5 +311,20 @@ describe('Action menu', () => {
         expect(unselectedItem.selected).to.be.false;
         expect(selectedItem.innerHTML).to.equal('Two');
         expect(selectedItem.selected).to.be.false;
+
+        // close by clicking selected
+        // (with event listener: should set selected = false)
+        closed = oneEvent(root, 'sp-closed');
+        selectedItem.click();
+        await closed;
+
+        opened = oneEvent(root, 'sp-opened');
+        root.click();
+        await opened;
+
+        expect(unselectedItem.innerHTML).to.equal('One');
+        expect(unselectedItem.selected).to.be.false;
+        expect(selectedItem.innerHTML).to.equal('Two');
+        expect(selectedItem.selected).to.be.true;
     });
 });
diff --git a/packages/picker/src/Picker.ts b/packages/picker/src/Picker.ts
@@ -231,19 +231,25 @@ export class PickerBase extends SizedMixin(Focusable) {
             if (menuChangeEvent) {
                 menuChangeEvent.preventDefault();
             }
-            this.selectedItem.selected = false;
+            this.setSelected(this.selectedItem, false);
             if (oldSelectedItem) {
-                oldSelectedItem.selected = true;
+                this.setSelected(oldSelectedItem, true);
             }
             this.selectedItem = oldSelectedItem;
             this.value = oldValue;
             this.open = true;
             return;
         }
         if (oldSelectedItem) {
-            oldSelectedItem.selected = false;
+            this.setSelected(oldSelectedItem, false);
         }
-        item.selected = !!this.selects;
+        this.setSelected(item, !!this.selects);
+    }
+
+    protected setSelected(item: MenuItem, value: boolean): void {
+        // matches null | undefined
+        if (this.selects == null) return;
+        item.selected = value;
     }
 
     public toggle(target?: boolean): void {
@@ -319,8 +325,8 @@ export class PickerBase extends SizedMixin(Focusable) {
                     selected?: boolean;
                 }
             ) => {
-                if (this.selects != null && this.value === el.value) {
-                    el.selected = true;
+                if (this.value === el.value) {
+                    this.setSelected(el as MenuItem, true);
                 }
                 return (el) => {
                     if (typeof el.focused !== 'undefined') {