Merge pull request #1270 from ticktackk/bugfix/issue-1249

Fix: Reached maximum item limit notice is not cleared after removing selections
Choices-js · Feb 22, 2025 · 3bd4414 · 3bd4414
2 parents 897d309 + 09f3e24
commit 3bd4414
Show file tree

Hide file tree

Showing 13 changed files with 125 additions and 21 deletions.
diff --git a/public/assets/scripts/choices.js b/public/assets/scripts/choices.js
@@ -4400,6 +4400,9 @@
                 this._displayNotice(typeof maxItemText === 'function' ? maxItemText(maxItemCount) : maxItemText, NoticeTypes.addChoice);
                 return false;
             }
+            if (this._notice && this._notice.type === NoticeTypes.addChoice) {
+                this._clearNotice();
+            }
             return true;
         };
         Choices.prototype._canCreateItem = function (value) {
@@ -4800,19 +4803,24 @@
             if (target === this.input.element) {
                 return;
             }
+            var preventDefault = true;
             var item = target.closest('[data-button],[data-item],[data-choice]');
             if (item instanceof HTMLElement) {
                 if ('button' in item.dataset) {
                     this._handleButtonAction(item);
                 }
                 else if ('item' in item.dataset) {
                     this._handleItemAction(item, event.shiftKey);
+                    // don't prevent default to support dragging
+                    preventDefault = false;
                 }
                 else if ('choice' in item.dataset) {
                     this._handleChoiceAction(item);
                 }
             }
-            event.preventDefault();
+            if (preventDefault) {
+                event.preventDefault();
+            }
         };
         /**
          * Handles mouseover event over this.dropdown

diff --git a/public/assets/scripts/choices.min.js b/public/assets/scripts/choices.min.js
diff --git a/public/assets/scripts/choices.mjs b/public/assets/scripts/choices.mjs
@@ -4022,7 +4022,7 @@ var Choices = /** @class */ (function () {
                 var dropdownItem = choice.choiceEl || _this._templates.choice(config, choice, config.itemSelectText, groupLabel);
                 choice.choiceEl = dropdownItem;
                 fragment.appendChild(dropdownItem);
-                if (!choice.disabled && (isSearching || !choice.selected)) {
+                if (isSearching || !choice.selected) {
                     selectableChoices = true;
                 }
                 return index < choiceLimit;
@@ -4394,6 +4394,9 @@ var Choices = /** @class */ (function () {
             this._displayNotice(typeof maxItemText === 'function' ? maxItemText(maxItemCount) : maxItemText, NoticeTypes.addChoice);
             return false;
         }
+        if (this._notice && this._notice.type === NoticeTypes.addChoice) {
+            this._clearNotice();
+        }
         return true;
     };
     Choices.prototype._canCreateItem = function (value) {
@@ -4544,24 +4547,24 @@ var Choices = /** @class */ (function () {
         https://en.wikipedia.org/wiki/UTF-16#Code_points_from_U+010000_to_U+10FFFF - UTF-16 surrogate pairs
         https://stackoverflow.com/a/70866532 - "Unidentified" for mobile
         http://www.unicode.org/versions/Unicode5.2.0/ch16.pdf#G19635 - U+FFFF is reserved (Section 16.7)
-
+    
         Logic: when a key event is sent, `event.key` represents its printable value _or_ one
         of a large list of special values indicating meta keys/functionality. In addition,
         key events for compose functionality contain a value of `Dead` when mid-composition.
-
+    
         I can't quite verify it, but non-English IMEs may also be able to generate key codes
         for code points in the surrogate-pair range, which could potentially be seen as having
         key.length > 1. Since `Fn` is one of the special keys, we can't distinguish by that
         alone.
-
+    
         Here, key.length === 1 means we know for sure the input was printable and not a special
         `key` value. When the length is greater than 1, it could be either a printable surrogate
         pair or a special `key` value. We can tell the difference by checking if the _character
         code_ value (not code point!) is in the "surrogate pair" range or not.
-
+    
         We don't use .codePointAt because an invalid code point would return 65535, which wouldn't
         pass the >= 0x10000 check we would otherwise use.
-
+    
         > ...The Unicode Standard sets aside 66 noncharacter code points. The last two code points
         > of each plane are noncharacters: U+FFFE and U+FFFF on the BMP...
         */
@@ -4794,19 +4797,24 @@ var Choices = /** @class */ (function () {
         if (target === this.input.element) {
             return;
         }
+        var preventDefault = true;
         var item = target.closest('[data-button],[data-item],[data-choice]');
         if (item instanceof HTMLElement) {
             if ('button' in item.dataset) {
                 this._handleButtonAction(item);
             }
             else if ('item' in item.dataset) {
                 this._handleItemAction(item, event.shiftKey);
+                // don't prevent default to support dragging
+                preventDefault = false;
             }
             else if ('choice' in item.dataset) {
                 this._handleChoiceAction(item);
             }
         }
-        event.preventDefault();
+        if (preventDefault) {
+            event.preventDefault();
+        }
     };
     /**
      * Handles mouseover event over this.dropdown

diff --git a/public/assets/scripts/choices.search-basic.js b/public/assets/scripts/choices.search-basic.js
@@ -3546,7 +3546,7 @@
                     var dropdownItem = choice.choiceEl || _this._templates.choice(config, choice, config.itemSelectText, groupLabel);
                     choice.choiceEl = dropdownItem;
                     fragment.appendChild(dropdownItem);
-                    if (!choice.disabled && (isSearching || !choice.selected)) {
+                    if (isSearching || !choice.selected) {
                         selectableChoices = true;
                     }
                     return index < choiceLimit;
@@ -3918,6 +3918,9 @@
                 this._displayNotice(typeof maxItemText === 'function' ? maxItemText(maxItemCount) : maxItemText, NoticeTypes.addChoice);
                 return false;
             }
+            if (this._notice && this._notice.type === NoticeTypes.addChoice) {
+                this._clearNotice();
+            }
             return true;
         };
         Choices.prototype._canCreateItem = function (value) {
@@ -4318,19 +4321,24 @@
             if (target === this.input.element) {
                 return;
             }
+            var preventDefault = true;
             var item = target.closest('[data-button],[data-item],[data-choice]');
             if (item instanceof HTMLElement) {
                 if ('button' in item.dataset) {
                     this._handleButtonAction(item);
                 }
                 else if ('item' in item.dataset) {
                     this._handleItemAction(item, event.shiftKey);
+                    // don't prevent default to support dragging
+                    preventDefault = false;
                 }
                 else if ('choice' in item.dataset) {
                     this._handleChoiceAction(item);
                 }
             }
-            event.preventDefault();
+            if (preventDefault) {
+                event.preventDefault();
+            }
         };
         /**
          * Handles mouseover event over this.dropdown

diff --git a/public/assets/scripts/choices.search-basic.min.js b/public/assets/scripts/choices.search-basic.min.js
diff --git a/public/assets/scripts/choices.search-basic.mjs b/public/assets/scripts/choices.search-basic.mjs
@@ -3540,7 +3540,7 @@ var Choices = /** @class */ (function () {
                 var dropdownItem = choice.choiceEl || _this._templates.choice(config, choice, config.itemSelectText, groupLabel);
                 choice.choiceEl = dropdownItem;
                 fragment.appendChild(dropdownItem);
-                if (!choice.disabled && (isSearching || !choice.selected)) {
+                if (isSearching || !choice.selected) {
                     selectableChoices = true;
                 }
                 return index < choiceLimit;
@@ -3912,6 +3912,9 @@ var Choices = /** @class */ (function () {
             this._displayNotice(typeof maxItemText === 'function' ? maxItemText(maxItemCount) : maxItemText, NoticeTypes.addChoice);
             return false;
         }
+        if (this._notice && this._notice.type === NoticeTypes.addChoice) {
+            this._clearNotice();
+        }
         return true;
     };
     Choices.prototype._canCreateItem = function (value) {
@@ -4312,19 +4315,24 @@ var Choices = /** @class */ (function () {
         if (target === this.input.element) {
             return;
         }
+        var preventDefault = true;
         var item = target.closest('[data-button],[data-item],[data-choice]');
         if (item instanceof HTMLElement) {
             if ('button' in item.dataset) {
                 this._handleButtonAction(item);
             }
             else if ('item' in item.dataset) {
                 this._handleItemAction(item, event.shiftKey);
+                // don't prevent default to support dragging
+                preventDefault = false;
             }
             else if ('choice' in item.dataset) {
                 this._handleChoiceAction(item);
             }
         }
-        event.preventDefault();
+        if (preventDefault) {
+            event.preventDefault();
+        }
     };
     /**
      * Handles mouseover event over this.dropdown

diff --git a/public/assets/scripts/choices.search-prefix.js b/public/assets/scripts/choices.search-prefix.js
@@ -2388,7 +2388,7 @@
                     var dropdownItem = choice.choiceEl || _this._templates.choice(config, choice, config.itemSelectText, groupLabel);
                     choice.choiceEl = dropdownItem;
                     fragment.appendChild(dropdownItem);
-                    if (!choice.disabled && (isSearching || !choice.selected)) {
+                    if (isSearching || !choice.selected) {
                         selectableChoices = true;
                     }
                     return index < choiceLimit;
@@ -2760,6 +2760,9 @@
                 this._displayNotice(typeof maxItemText === 'function' ? maxItemText(maxItemCount) : maxItemText, NoticeTypes.addChoice);
                 return false;
             }
+            if (this._notice && this._notice.type === NoticeTypes.addChoice) {
+                this._clearNotice();
+            }
             return true;
         };
         Choices.prototype._canCreateItem = function (value) {
@@ -3160,19 +3163,24 @@
             if (target === this.input.element) {
                 return;
             }
+            var preventDefault = true;
             var item = target.closest('[data-button],[data-item],[data-choice]');
             if (item instanceof HTMLElement) {
                 if ('button' in item.dataset) {
                     this._handleButtonAction(item);
                 }
                 else if ('item' in item.dataset) {
                     this._handleItemAction(item, event.shiftKey);
+                    // don't prevent default to support dragging
+                    preventDefault = false;
                 }
                 else if ('choice' in item.dataset) {
                     this._handleChoiceAction(item);
                 }
             }
-            event.preventDefault();
+            if (preventDefault) {
+                event.preventDefault();
+            }
         };
         /**
          * Handles mouseover event over this.dropdown

diff --git a/public/assets/scripts/choices.search-prefix.min.js b/public/assets/scripts/choices.search-prefix.min.js
diff --git a/public/assets/scripts/choices.search-prefix.mjs b/public/assets/scripts/choices.search-prefix.mjs
@@ -2382,7 +2382,7 @@ var Choices = /** @class */ (function () {
                 var dropdownItem = choice.choiceEl || _this._templates.choice(config, choice, config.itemSelectText, groupLabel);
                 choice.choiceEl = dropdownItem;
                 fragment.appendChild(dropdownItem);
-                if (!choice.disabled && (isSearching || !choice.selected)) {
+                if (isSearching || !choice.selected) {
                     selectableChoices = true;
                 }
                 return index < choiceLimit;
@@ -2754,6 +2754,9 @@ var Choices = /** @class */ (function () {
             this._displayNotice(typeof maxItemText === 'function' ? maxItemText(maxItemCount) : maxItemText, NoticeTypes.addChoice);
             return false;
         }
+        if (this._notice && this._notice.type === NoticeTypes.addChoice) {
+            this._clearNotice();
+        }
         return true;
     };
     Choices.prototype._canCreateItem = function (value) {
@@ -3154,19 +3157,24 @@ var Choices = /** @class */ (function () {
         if (target === this.input.element) {
             return;
         }
+        var preventDefault = true;
         var item = target.closest('[data-button],[data-item],[data-choice]');
         if (item instanceof HTMLElement) {
             if ('button' in item.dataset) {
                 this._handleButtonAction(item);
             }
             else if ('item' in item.dataset) {
                 this._handleItemAction(item, event.shiftKey);
+                // don't prevent default to support dragging
+                preventDefault = false;
             }
             else if ('choice' in item.dataset) {
                 this._handleChoiceAction(item);
             }
         }
-        event.preventDefault();
+        if (preventDefault) {
+            event.preventDefault();
+        }
     };
     /**
      * Handles mouseover event over this.dropdown

diff --git a/public/test/select-multiple/index.html b/public/test/select-multiple/index.html
@@ -335,6 +335,31 @@ <h2>Select multiple inputs</h2>
           </script>
         </div>
 
+        <div data-test-hook="selection-limit-note-after-unselecting-choice">
+          <label for="choices-selection-limit-note-after-unselecting-choice">Input limit note after unselecting choice</label>
+          <select
+            class="form-control"
+            name="choices-selection-limit-note-after-unselecting-choice"
+            id="choices-selection-limit-note-after-unselecting-choice"
+            multiple
+          >
+            <option value="Choice 1">Choice 1</option>
+            <option value="Choice 2">Choice 2</option>
+            <option value="Choice 3">Choice 3</option>
+            <option value="Choice 4">Choice 4</option>
+            <option value="Choice 5">Choice 5</option>
+          </select>
+          <script>
+            document.addEventListener('DOMContentLoaded', function() {
+              new Choices('#choices-selection-limit-note-after-unselecting-choice', {
+                allowHTML: true,
+                maxItemCount: 5,
+                removeItemButton: true
+              });
+            });
+          </script>
+        </div>
+
         <div data-test-hook="prepend-append">
           <label for="choices-prepend-append">Prepend/append</label>
           <select

diff --git a/src/scripts/choices.ts b/src/scripts/choices.ts
@@ -1404,6 +1404,10 @@ class Choices {
       return false;
     }
 
+    if (this._notice && this._notice.type === NoticeTypes.addChoice) {
+      this._clearNotice();
+    }
+
     return true;
   }
 

diff --git a/test-e2e/test-suit.ts b/test-e2e/test-suit.ts
@@ -165,6 +165,15 @@ export class TestSuit {
     await expect(this.dropdown).toBeHidden();
   }
 
+  async expectHiddenNotice(singleItem: boolean = false): Promise<void> {
+    await this.advanceClock();
+
+    if (singleItem) {
+      await expect(this.dropdown.locator('> *:not(input)')).toHaveCount(0);
+    }
+    await expect(this.dropdown.locator('.choices__notice')).toBeHidden();
+  }
+
   // eslint-disable-next-line class-methods-use-this
   getWrappedElement(): Locator {
     throw new Error('Not implemented');

diff --git a/test-e2e/tests/select-multiple.spec.ts b/test-e2e/tests/select-multiple.spec.ts
@@ -478,11 +478,24 @@ describe(`Choices - select multiple`, () => {
     });
 
     describe('selection limit', () => {
-      const testId = 'selection-limit';
+      const displaysTestId = 'selection-limit';
       const selectionLimit = 5;
 
       test('displays "limit reached" prompt', async ({ page, bundle }) => {
-        const suite = new SelectTestSuit(page, bundle, testUrl, testId);
+        const suite = new SelectTestSuit(page, bundle, testUrl, displaysTestId);
+        await suite.startWithClick();
+
+        for (let index = 0; index < selectionLimit; index++) {
+          await suite.selectableChoices.first().click();
+          await suite.advanceClock();
+        }
+
+        await suite.expectVisibleNoticeHtml(`Only ${selectionLimit} values can be added`);
+      });
+
+      const hidesTestId = 'selection-limit-note-after-unselecting-choice';
+      test('hides "limit reached" prompt', async ({ page, bundle }) => {
+        const suite = new SelectTestSuit(page, bundle, testUrl, hidesTestId);
         await suite.startWithClick();
 
         for (let index = 0; index < selectionLimit; index++) {
@@ -491,6 +504,11 @@ describe(`Choices - select multiple`, () => {
         }
 
         await suite.expectVisibleNoticeHtml(`Only ${selectionLimit} values can be added`);
+
+        await suite.items.getByRole('button', { name: 'Remove item' }).last().click();
+        await suite.advanceClock();
+
+        await suite.expectHiddenNotice();
       });
     });