From 5237fdb30694364934e1cd30f3d9cf82efa2c5c5 Mon Sep 17 00:00:00 2001
From: Hunter Loftis <hunter@hunterloftis.com>
Date: Wed, 14 Sep 2022 15:32:31 -0400
Subject: [PATCH] fix(action-menu): never set item selected values when selects
 is undefined

---
 packages/action-menu/test/action-menu.test.ts | 19 ++++++++++++++++++-
 packages/picker/src/Picker.ts                 | 18 ++++++++++++------
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/packages/action-menu/test/action-menu.test.ts b/packages/action-menu/test/action-menu.test.ts
index 6ff8e4c9f5..7cd7cecc05 100644
--- 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
index 7ff1078df7..66ddded7ed 100644
--- a/packages/picker/src/Picker.ts
+++ b/packages/picker/src/Picker.ts
@@ -231,9 +231,9 @@ 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;
@@ -241,9 +241,15 @@ export class PickerBase extends SizedMixin(Focusable) {
             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') {