From 942306572a934f93e66c2cd31c590f56de06ca38 Mon Sep 17 00:00:00 2001
From: Simon Kohlmeyer <simon.kohlmeyer@gmail.com>
Date: Fri, 31 Mar 2023 22:02:08 +0200
Subject: [PATCH] Render options without a group even if groups are present.

Closes #615.

This pushes the conversion of OPTION/OPTGROUP elements to Choice objects
into the WrappedSelect class and unifies the code paths a little between
groups-present and groups-not-present.

Some work towards possibly fixing #615
---
 cypress/e2e/select-multiple.spec.ts      | 13 ++++++++
 public/test/select-multiple/index.html   | 21 +++++++++++++
 src/scripts/choices.ts                   | 32 +++++++------------
 src/scripts/components/wrapped-select.ts | 40 ++++++++++++++++++++++++
 src/scripts/lib/htmlElementGuards.ts     |  5 +++
 5 files changed, 91 insertions(+), 20 deletions(-)
 create mode 100644 src/scripts/lib/htmlElementGuards.ts

diff --git a/cypress/e2e/select-multiple.spec.ts b/cypress/e2e/select-multiple.spec.ts
index c35f781..4b88ca8 100644
--- a/cypress/e2e/select-multiple.spec.ts
+++ b/cypress/e2e/select-multiple.spec.ts
@@ -702,6 +702,19 @@ describe('Choices - select multiple', () => {
       });
     });
 
+    describe('some options with a group and some without', () => {
+      it('Shows all options, whether they are in a group or not', () => {
+        cy.get('[data-test-hook=mixed-groups]')
+          .find('.choices__input--cloned')
+          .focus();
+        cy.get('[data-test-hook=mixed-groups]')
+          .find('.choices__list--dropdown .choices__item')
+          .should(($choices) => {
+            expect($choices.length).to.equal(3);
+          });
+      });
+    });
+
     describe('custom properties', () => {
       beforeEach(() => {
         cy.get('[data-test-hook=custom-properties]')
diff --git a/public/test/select-multiple/index.html b/public/test/select-multiple/index.html
index e4091e5..9f621b5 100644
--- a/public/test/select-multiple/index.html
+++ b/public/test/select-multiple/index.html
@@ -283,6 +283,23 @@
             </optgroup>
           </select>
         </div>
+        <div data-test-hook="mixed-groups">
+          <label for="choices-groups"
+            >Choice groups with some elements without a group</label
+          >
+          <select
+            class="form-control"
+            name="mixed-choices-groups"
+            id="mixed-choices-groups"
+            multiple
+          >
+            <optgroup label="UK">
+              <option value="London">London</option>
+            </optgroup>
+            <option value="Paris">Paris</option>
+            <option value="Lyon">Lyon</option>
+          </select>
+        </div>
 
         <div data-test-hook="custom-properties">
           <label for="choices-custom-properties">Custom properties</label>
@@ -481,6 +498,10 @@
           allowHTML: true,
         });
 
+        new Choices('#mixed-choices-groups', {
+          allowHTML: false,
+        });
+
         new Choices('#choices-custom-properties', {
           allowHTML: true,
           searchFields: ['label', 'value', 'customProperties.country'],
diff --git a/src/scripts/choices.ts b/src/scripts/choices.ts
index 36a51da..d58395f 100644
--- a/src/scripts/choices.ts
+++ b/src/scripts/choices.ts
@@ -46,7 +46,6 @@ import {
   isType,
   sortByScore,
   strToEl,
-  parseCustomProperties,
 } from './lib/utils';
 import { defaultState } from './reducers';
 import Store from './store/store';
@@ -283,19 +282,10 @@ class Choices implements Choices {
     }
     // Create array of choices from option elements
     if ((this.passedElement as WrappedSelect).options) {
-      (this.passedElement as WrappedSelect).options.forEach((option) => {
-        this._presetChoices.push({
-          value: option.value,
-          label: option.innerHTML,
-          selected: !!option.selected,
-          disabled: option.disabled || option.parentNode.disabled,
-          placeholder:
-            option.value === '' || option.hasAttribute('placeholder'),
-          customProperties: parseCustomProperties(
-            option.dataset.customProperties,
-          ),
-        });
-      });
+      const choicesFromOptions = (
+        this.passedElement as WrappedSelect
+      ).optionsAsChoices();
+      this._presetChoices.push(...choicesFromOptions);
     }
 
     this._render = this._render.bind(this);
@@ -903,6 +893,13 @@ class Choices implements Choices {
       groups.sort(this.config.sorter);
     }
 
+    // Add Choices without group first, regardless of sort, otherwise they won't be distinguishable
+    // from the last group
+    const choicesWithoutGroup = choices.filter((c) => c.groupId == -1);
+    if (choicesWithoutGroup.length > 0) {
+      this._createChoicesFragment(choicesWithoutGroup, fragment, false);
+    }
+
     groups.forEach((group) => {
       const groupChoices = getGroupChoices(group);
       if (groupChoices.length >= 1) {
@@ -2216,12 +2213,7 @@ class Choices implements Choices {
       this._highlightPosition = 0;
       this._isSearching = false;
       this._startLoading();
-
-      if (this._presetGroups.length) {
-        this._addPredefinedGroups(this._presetGroups);
-      } else {
-        this._addPredefinedChoices(this._presetChoices);
-      }
+      this._addPredefinedChoices(this._presetChoices);
 
       this._stopLoading();
     }
diff --git a/src/scripts/components/wrapped-select.ts b/src/scripts/components/wrapped-select.ts
index 3b2ada6..c0a22ba 100644
--- a/src/scripts/components/wrapped-select.ts
+++ b/src/scripts/components/wrapped-select.ts
@@ -1,6 +1,10 @@
+import { Choice } from '../interfaces/choice';
+import { parseCustomProperties } from '../lib/utils';
 import { ClassNames } from '../interfaces/class-names';
 import { Item } from '../interfaces/item';
 import WrappedElement from './wrapped-element';
+import { isHTMLOptgroup } from '../lib/htmlElementGuards';
+import { isHTMLOption } from '../lib/htmlElementGuards';
 
 export default class WrappedSelect extends WrappedElement {
   element: HTMLSelectElement;
@@ -53,6 +57,42 @@ export default class WrappedSelect extends WrappedElement {
     this.appendDocFragment(fragment);
   }
 
+  optionsAsChoices(): Partial<Choice>[] {
+    const choices: Partial<Choice>[] = [];
+
+    for (const e of Array.from(this.element.querySelectorAll(':scope > *'))) {
+      if (isHTMLOption(e)) {
+        choices.push(this._optionToChoice(e as HTMLOptionElement));
+      } else if (isHTMLOptgroup(e)) {
+        choices.push(this._optgroupToChoice(e as HTMLOptGroupElement));
+      }
+      // There should only be those two in a <select> and we wouldn't care about others anyways
+    }
+
+    return choices;
+  }
+
+  _optionToChoice(option: HTMLOptionElement): Choice {
+    return {
+      value: option.value,
+      label: option.innerHTML,
+      selected: !!option.selected,
+      disabled: option.disabled || this.element.disabled,
+      placeholder: option.value === '' || option.hasAttribute('placeholder'),
+      customProperties: parseCustomProperties(option.dataset.customProperties),
+    };
+  }
+
+  _optgroupToChoice(optgroup: HTMLOptGroupElement): Partial<Choice> {
+    return {
+      label: optgroup.label || '',
+      disabled: !!optgroup.disabled,
+      choices: Array.from(optgroup.querySelectorAll('option')).map((option) =>
+        this._optionToChoice(option),
+      ),
+    };
+  }
+
   appendDocFragment(fragment: DocumentFragment): void {
     this.element.innerHTML = '';
     this.element.appendChild(fragment);
diff --git a/src/scripts/lib/htmlElementGuards.ts b/src/scripts/lib/htmlElementGuards.ts
new file mode 100644
index 0000000..324462e
--- /dev/null
+++ b/src/scripts/lib/htmlElementGuards.ts
@@ -0,0 +1,5 @@
+export const isHTMLOption = (e: Element): e is HTMLOptionElement =>
+  e.tagName === 'OPTION';
+
+export const isHTMLOptgroup = (e: Element): e is HTMLOptGroupElement =>
+  e.tagName === 'OPTGROUP';