Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Reached maximum item limit notice is not cleared after removing selections #1270

Merged
merged 2 commits into from
Feb 22, 2025
Merged

Fix: Reached maximum item limit notice is not cleared after removing selections #1270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e50fa98
Fix: Unselecting option after reaching maximum item limit does not re…
ticktackk Feb 19, 2025
09f3e24
Compile JS files
Xon Feb 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion public/assets/scripts/choices.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion public/assets/scripts/choices.min.js
View file
Open in desktop

Large diffs are not rendered by default.

22 changes: 15 additions & 7 deletions public/assets/scripts/choices.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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...
*/
Expand Down Expand Up @@ -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
Expand Down
12 changes: 10 additions & 2 deletions public/assets/scripts/choices.search-basic.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion public/assets/scripts/choices.search-basic.min.js
View file
Open in desktop

Large diffs are not rendered by default.

12 changes: 10 additions & 2 deletions public/assets/scripts/choices.search-basic.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
12 changes: 10 additions & 2 deletions public/assets/scripts/choices.search-prefix.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion public/assets/scripts/choices.search-prefix.min.js
View file
Open in desktop

Large diffs are not rendered by default.

12 changes: 10 additions & 2 deletions public/assets/scripts/choices.search-prefix.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions public/test/select-multiple/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/scripts/choices.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1404,6 +1404,10 @@ class Choices {
return false;
}

if (this._notice && this._notice.type === NoticeTypes.addChoice) {
this._clearNotice();
}

return true;
}

Expand Down
9 changes: 9 additions & 0 deletions test-e2e/test-suit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
22 changes: 20 additions & 2 deletions test-e2e/tests/select-multiple.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand All @@ -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();
});
});

Expand Down
Loading