From e968989b6b15d46fd1613cc31d82adad117dead9 Mon Sep 17 00:00:00 2001
From: Josh Johnson <josh@joshuajohnson.co.uk>
Date: Sun, 25 Nov 2018 12:41:58 +0000
Subject: [PATCH] Resolve bug with setChoiceByValue not removing choice from
 dropdown

---
 cypress/integration/select-multiple.spec.js | 35 +++++++++++++++++++++
 cypress/integration/select-one.spec.js      | 32 +++++++++++++++++++
 public/test/select-multiple.html            | 11 +++++++
 public/test/select-one.html                 | 11 +++++++
 src/scripts/choices.js                      | 28 ++++++-----------
 5 files changed, 98 insertions(+), 19 deletions(-)

diff --git a/cypress/integration/select-multiple.spec.js b/cypress/integration/select-multiple.spec.js
index 3a35bf2..731721b 100644
--- a/cypress/integration/select-multiple.spec.js
+++ b/cypress/integration/select-multiple.spec.js
@@ -818,5 +818,40 @@ describe('Choices - select multiple', () => {
         });
       });
     });
+
+    describe('dynamically setting choice by value', () => {
+      const dynamicallySelectedChoiceValue = 'Choice 2';
+
+      it('selects choice', () => {
+        cy.get('[data-test-hook=set-choice-by-value]')
+          .find('.choices__list--multiple .choices__item')
+          .last()
+          .should($choice => {
+            expect($choice.text().trim()).to.equal(
+              dynamicallySelectedChoiceValue,
+            );
+          });
+      });
+
+      it('removes choice from dropdown list', () => {
+        cy.get('[data-test-hook=set-choice-by-value]')
+          .find('.choices__list--dropdown .choices__list')
+          .children()
+          .each($choice => {
+            expect($choice.text().trim()).to.not.equal(
+              dynamicallySelectedChoiceValue,
+            );
+          });
+      });
+
+      it('updates the value of the original input', () => {
+        cy.get('[data-test-hook=set-choice-by-value]')
+          .find('.choices__input.is-hidden')
+          .should($select => {
+            const val = $select.val() || [];
+            expect(val).to.contain(dynamicallySelectedChoiceValue);
+          });
+      });
+    });
   });
 });
diff --git a/cypress/integration/select-one.spec.js b/cypress/integration/select-one.spec.js
index 7fc6513..f2d331e 100644
--- a/cypress/integration/select-one.spec.js
+++ b/cypress/integration/select-one.spec.js
@@ -835,5 +835,37 @@ describe('Choices - select one', () => {
         });
       });
     });
+
+    describe('dynamically setting choice by value', () => {
+      const dynamicallySelectedChoiceValue = 'Choice 2';
+
+      it('selects choice', () => {
+        cy.get('[data-test-hook=set-choice-by-value]')
+          .find('.choices__list--single .choices__item')
+          .last()
+          .should($choice => {
+            expect($choice.text().trim()).to.equal(
+              dynamicallySelectedChoiceValue,
+            );
+          });
+      });
+
+      it('does not remove choice from dropdown list', () => {
+        cy.get('[data-test-hook=set-choice-by-value]')
+          .find('.choices__list--dropdown .choices__list')
+          .then($choicesList => {
+            expect($choicesList).to.contain(dynamicallySelectedChoiceValue);
+          });
+      });
+
+      it('updates the value of the original input', () => {
+        cy.get('[data-test-hook=set-choice-by-value]')
+          .find('.choices__input.is-hidden')
+          .should($select => {
+            const val = $select.val() || [];
+            expect(val).to.contain(dynamicallySelectedChoiceValue);
+          });
+      });
+    });
   });
 });
diff --git a/public/test/select-multiple.html b/public/test/select-multiple.html
index 54e0561..5b1d1dc 100644
--- a/public/test/select-multiple.html
+++ b/public/test/select-multiple.html
@@ -185,6 +185,15 @@
             </select>
           </form>
         </div>
+
+        <div data-test-hook="set-choice-by-value">
+          <label for="choices-set-choice-by-value">Dynamically set choice by value</label>
+          <select class="form-control" name="choices-set-choice-by-value" id="choices-set-choice-by-value" multiple>
+            <option value="Choice 1">Choice 1</option>
+            <option value="Choice 2">Choice 2</option>
+            <option value="Choice 3">Choice 3</option>
+          </select>
+        </div>
       </div>
     </div>
     <script>
@@ -296,6 +305,8 @@
         });
 
         new Choices('#choices-within-form');
+
+        new Choices('#choices-set-choice-by-value').setChoiceByValue('Choice 2');
       });
     </script>
   </body>
diff --git a/public/test/select-one.html b/public/test/select-one.html
index 5772c26..5611c4d 100644
--- a/public/test/select-one.html
+++ b/public/test/select-one.html
@@ -189,6 +189,15 @@
             </select>
           </form>
         </div>
+
+        <div data-test-hook="set-choice-by-value">
+          <label for="choices-set-choice-by-value">Dynamically set choice by value</label>
+          <select class="form-control" name="choices-set-choice-by-value" id="choices-set-choice-by-value">
+            <option value="Choice 1">Choice 1</option>
+            <option value="Choice 2">Choice 2</option>
+            <option value="Choice 3">Choice 3</option>
+          </select>
+        </div>
       </div>
     </div>
     <script>
@@ -308,6 +317,8 @@
         });
 
         new Choices('#choices-within-form');
+
+        new Choices('#choices-set-choice-by-value').setChoiceByValue('Choice 2');
       });
     </script>
   </body>
diff --git a/src/scripts/choices.js b/src/scripts/choices.js
index 809de1c..bf57025 100644
--- a/src/scripts/choices.js
+++ b/src/scripts/choices.js
@@ -1628,24 +1628,14 @@ class Choices {
     }
 
     // Trigger change event
-    if (group && group.value) {
-      this.passedElement.triggerEvent(EVENTS.addItem, {
-        id,
-        value: passedValue,
-        label: passedLabel,
-        customProperties: passedCustomProperties,
-        groupValue: group.value,
-        keyCode: passedKeyCode,
-      });
-    } else {
-      this.passedElement.triggerEvent(EVENTS.addItem, {
-        id,
-        value: passedValue,
-        label: passedLabel,
-        customProperties: passedCustomProperties,
-        keyCode: passedKeyCode,
-      });
-    }
+    this.passedElement.triggerEvent(EVENTS.addItem, {
+      id,
+      value: passedValue,
+      label: passedLabel,
+      customProperties: passedCustomProperties,
+      groupValue: group && group.value ? group.value : undefined,
+      keyCode: passedKeyCode,
+    });
 
     return this;
   }
@@ -2044,7 +2034,7 @@ class Choices {
       this._addItem({
         value: foundChoice.value,
         label: foundChoice.label,
-        id: foundChoice.id,
+        choiceId: foundChoice.id,
         groupId: foundChoice.groupId,
         customProperties: foundChoice.customProperties,
         placeholder: foundChoice.placeholder,