Skip to content

Commit

Permalink
Merge pull request #1192 from Choices-js/fix-display-banner
Browse files Browse the repository at this point in the history
Fix regression "no choices to choose from"/"no results found" notice did not reliably trigger
  • Loading branch information
Xon authored Sep 4, 2024
2 parents daa506c + 6c183de commit 3d79892
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 20 deletions.
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@

### Bug Fixes (from 11.0.0)
* Fix choice disable state wasn't considered when showing the "no choices to choose from" notice
* Fix regression "no choices to choose from" notice not triggering when no selectable choices exist for select-one. [#1185](https://github.com/Choices-js/Choices/issues/1185)
* Fix regression where webpack doesn't permit importing scss/css @tagliala (#1193)
* Fix regression where webpack doesn't permit importing scss/css @tagliala [#1193](https://github.com/Choices-js/Choices/issues/1193)
* Fix regression "no choices to choose from"/"no results found" notice did not reliably trigger. [#1185](https://github.com/Choices-js/Choices/issues/1185) [#1191](https://github.com/Choices-js/Choices/issues/1191)
* Fix regression of UnhighlightItem event not firing [#1173](https://github.com/Choices-js/Choices/issues/1173)

### Chore
Expand Down
22 changes: 8 additions & 14 deletions src/scripts/choices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,7 @@ class Choices {
if (!choice) {
return this;
}
this._clearNotice();
this._store.dispatch(removeChoice(choice));
// @todo integrate with Store
this._searcher.reset();
Expand All @@ -846,7 +847,7 @@ class Choices {
clearStore(): this {
this.itemList.element.replaceChildren('');
this.choiceList.element.replaceChildren('');
this._clearNotice();
this._stopSearch();
this._store.reset();
this._lastAddedChoiceId = 0;
this._lastAddedGroupId = 0;
Expand All @@ -859,11 +860,7 @@ class Choices {
clearInput(): this {
const shouldSetInputWidth = !this._isSelectOneElement;
this.input.clear(shouldSetInputWidth);
this._clearNotice();

if (this._isSearching) {
this._stopSearch();
}
this._stopSearch();

return this;
}
Expand Down Expand Up @@ -999,17 +996,14 @@ class Choices {
}
}

const notice = this._notice;
if (!selectableChoices) {
if (!notice) {
if (!this._notice) {
this._notice = {
text: resolveStringFunction(config.noChoicesText),
type: NoticeTypes.noChoices,
text: resolveStringFunction(isSearching ? config.noResultsText : config.noChoicesText),
type: isSearching ? NoticeTypes.noResults : NoticeTypes.noChoices,
};
}
fragment.replaceChildren('');
} else if (notice && notice.type === NoticeTypes.noChoices) {
this._notice = undefined;
}

this._renderNotice(fragment);
Expand Down Expand Up @@ -1463,6 +1457,7 @@ class Choices {
const wasSearching = this._isSearching;
this._currentValue = '';
this._isSearching = false;
this._clearNotice();
if (wasSearching) {
this._store.dispatch(activateChoices(true));

Expand Down Expand Up @@ -1639,8 +1634,6 @@ class Choices {
this._stopSearch();
}

this._clearNotice();

return;
}

Expand Down Expand Up @@ -2090,6 +2083,7 @@ class Choices {
(choice.element as HTMLOptionElement).value = choice.value;
}

this._clearNotice();
this._store.dispatch(addChoice(choice));

if (choice.selected) {
Expand Down
18 changes: 16 additions & 2 deletions test-e2e/tests/select-multiple.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ describe(`Choices - select multiple`, () => {
await suite.startWithClick();

await expect(suite.selectableChoices).toHaveCount(0);
await suite.expectVisibleNoticeHtml('No choices to choose from');
await suite.expectVisibleNoticeHtml('No choices to choose from', true);
});
});

Expand All @@ -393,7 +393,21 @@ describe(`Choices - select multiple`, () => {
await suite.startWithClick();

await expect(suite.selectableChoices).toHaveCount(0);
await suite.expectVisibleNoticeHtml('No choices to choose from');
await suite.expectVisibleNoticeHtml('No choices to choose from', true);
});

test('shows no results banner and then no choices banner', async ({ page, bundle }) => {
const invalidLabel = 'faergge';
const suite = new SelectTestSuit(page, bundle, testUrl, testId);
await suite.startWithClick();

await suite.typeText(invalidLabel);
await expect(suite.selectableChoices).toHaveCount(0);
await suite.expectVisibleNoticeHtml('No results found', true);

await suite.typeText('');
await expect(suite.selectableChoices).toHaveCount(0);
await suite.expectVisibleNoticeHtml('No choices to choose from', true);
});
});

Expand Down
18 changes: 16 additions & 2 deletions test-e2e/tests/select-one.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ describe(`Choices - select one`, () => {
await suite.startWithClick();

await expect(suite.selectableChoices).toHaveCount(0);
await suite.expectVisibleNoticeHtml('No choices to choose from');
await suite.expectVisibleNoticeHtml('No choices to choose from', true);
});
});

Expand All @@ -250,7 +250,21 @@ describe(`Choices - select one`, () => {
await suite.startWithClick();

await expect(suite.selectableChoices).toHaveCount(0);
await suite.expectVisibleNoticeHtml('No choices to choose from');
await suite.expectVisibleNoticeHtml('No choices to choose from', true);
});

test('shows no results banner and then no choices banner', async ({ page, bundle }) => {
const invalidLabel = 'faergge';
const suite = new SelectTestSuit(page, bundle, testUrl, testId);
await suite.startWithClick();

await suite.typeText(invalidLabel);
await expect(suite.selectableChoices).toHaveCount(0);
await suite.expectVisibleNoticeHtml('No results found', true);

await suite.typeText('');
await expect(suite.selectableChoices).toHaveCount(0);
await suite.expectVisibleNoticeHtml('No choices to choose from', true);
});
});

Expand Down

0 comments on commit 3d79892

Please sign in to comment.