Skip to content

Commit

Permalink
Fix: Disabled options are not visible (#1257)
Browse files Browse the repository at this point in the history
  • Loading branch information
ticktackk committed Feb 21, 2025
1 parent e36aa55 commit 1100233
Show file tree
Hide file tree
Showing 8 changed files with 138 additions and 8 deletions.
2 changes: 1 addition & 1 deletion public/assets/scripts/choices.js
Original file line number Diff line number Diff line change
Expand Up @@ -4028,7 +4028,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
2 changes: 1 addition & 1 deletion public/assets/scripts/choices.min.js

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions public/assets/scripts/choices.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4544,24 +4544,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
52 changes: 52 additions & 0 deletions public/test/disabled-data.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
[
{
"label": "Disabled Label 1",
"value": "Disabled Value 1",
"disabled": true
},
{
"label": "Disabled Label 2",
"value": "Disabled Value 2",
"disabled": true
},
{
"label": "Disabled Label 3",
"value": "Disabled Value 3",
"disabled": true
},
{
"label": "Disabled Label 4",
"value": "Disabled Value 4",
"disabled": true
},
{
"label": "Disabled Label 5",
"value": "Disabled Value 5",
"disabled": true
},
{
"label": "Disabled Label 6",
"value": "Disabled Value 6",
"disabled": true
},
{
"label": "Disabled Label 7",
"value": "Disabled Value 7",
"disabled": true
},
{
"label": "Disabled Label 8",
"value": "Disabled Value 8",
"disabled": true
},
{
"label": "Disabled Label 9",
"value": "Disabled Value 9",
"disabled": true
},
{
"label": "Disabled Label 10",
"value": "Disabled Value 10",
"disabled": true
}
]
27 changes: 27 additions & 0 deletions public/test/select-one/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,33 @@ <h2>Select one inputs</h2>
</script>
</div>

<div data-test-hook="remote-disabled-data">
<label for="choices-remote-disabled-data">Remote disabled data</label>
<select
class="form-control"
name="choices-remote-disabled-data"
id="choices-remote-disabled-data"
>
<option value="">I am a placeholder</option>
</select>
<script>
document.addEventListener('DOMContentLoaded', function() {
new Choices('#choices-remote-disabled-data', {
allowHTML: true,
shouldSort: false,
}).setChoices(async () => {
try {
const data = await fetch('../disabled-data.json');
return data.json();
} catch (e) {
console.log(e);
return [];
}
});
});
</script>
</div>

<div data-test-hook="scrolling-dropdown">
<label for="choices-scrolling-dropdown">Scrolling dropdown</label>
<select
Expand Down
2 changes: 1 addition & 1 deletion src/scripts/choices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -974,7 +974,7 @@ class Choices {
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)) {

Check failure on line 977 in src/scripts/choices.ts

View workflow job for this annotation

GitHub Actions / lint

Replace `(isSearching·||·!choice.selected)` with `isSearching·||·!choice.selected`
selectableChoices = true;
}

Expand Down
25 changes: 25 additions & 0 deletions test-e2e/select-test-suit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,31 @@ export class SelectTestSuit extends TestSuit {
return stopJsonWaiting;
}

async delayDisaabledData(): Promise<() => void> {
let stopJsonWaiting = (): void => {};
const jsonWaiting = new Promise<void>((f) => {
stopJsonWaiting = f;
});

await this.page.route('**/disabled-data.json', async (route) => {
await jsonWaiting;

const fakeData = [...new Array(10)].map((_, index) => ({
label: `Disabled Label ${index + 1}`,
value: `Disabled Value ${index + 1}`,
disabled: true,
}));

await route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify(fakeData),
});
});

return stopJsonWaiting;
}

getWrappedElement(): Locator {
return this.wrappedSelect;
}
Expand Down
26 changes: 26 additions & 0 deletions test-e2e/tests/select-one.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,32 @@ describe(`Choices - select one`, () => {
await expect(firstItem).toHaveText('I am a placeholder');
await expect(suite.selectableChoices).toHaveCount(10);
});

const testIdForDisabled = 'remote-disabled-data';
test('checking disabled items are shown in dropdown', async ({ page, bundle }) => {
const suite = new SelectTestSuit(page, bundle, testUrl, testIdForDisabled);

const jsonLoad = page.waitForResponse('**/disabled-data.json');
const stopJsonWaiting = await suite.delayDisaabledData();
await suite.start();

await expect(suite.itemList.first()).toHaveText('Loading...');

stopJsonWaiting();
await jsonLoad;
await suite.selectByClick();

const firstItem = suite.itemsWithPlaceholder.first();
await expect(firstItem).toHaveClass(/choices__placeholder/);
await expect(firstItem).toHaveText('I am a placeholder');

const lastChoice = suite.selectableChoices.last();
await expect(lastChoice).toHaveClass(/choices__item--disabled/);
await expect(lastChoice).toHaveText('Disabled Label 10');

await expect(suite.selectableChoices.locator(':not(.choices__item--disabled)')).toHaveCount(0);
await expect(suite.selectableChoices.locator('+ .choices__item--disabled')).toHaveCount(9);
});
});

describe('scrolling dropdown', () => {
Expand Down

0 comments on commit 1100233

Please sign in to comment.