Skip to content

Commit f054597

Browse files
Fix bug where suggestions could never be set if falsy (#48)
Also adds test coverage for updateSuggestions and hinting.
1 parent 8e36129 commit f054597

File tree

3 files changed

+111
-61
lines changed

3 files changed

+111
-61
lines changed

README.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,14 @@ displayed.
163163
To display a hint to the user in addition to suggestions, resolve the promise with an object that
164164
has keys for `hint` and `suggestions`: `{hint: 'My hint', suggestions: [...]}`. A hint is displayed
165165
to the right of the text that has been input by the user. Pressing RIGHT on the keyboard replaces
166-
the input text query with the hint.
166+
the input text query with the hint. The hint should include the entirety of the query, plus whatever
167+
else you want to hint with. When displayed, only the addition to the query is shown as a hint, but
168+
if the user presses RIGHT to complete it, it will use the entirety of the hint provided. For
169+
example, if the query is 'my query' and you want to hinted text to display ' is awesome', then you
170+
should pass 'my query is awesome' as the hint. However, if you want the completed hint to be
171+
formatted as 'My Query is Awesome', you should set that as the hint. When hinted, it will be
172+
displayed as 'my query is Awesome', but when completed via RIGHT on the keyboard, it will replace
173+
the query with the submitted formatting: 'My Query is Awesome'.
167174
- `ngModel {Any}` _(Required)_: This is a one-way binding to the ngModel for the Omnibox. When the
168175
`multiple` option is set to `true`, then the `ngModel` should be an array. Each item in the array
169176
will be populated with choices from the objects passed via the `source` function. If the `multiple`

spec/tests/angularComponent/ngcOmniboxControllerSpec.js

Lines changed: 51 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,51 @@ describe('ngcOmnibox.angularComponent.ngcOmniboxController', () => {
1818
omniboxController.isSelectable = () => {};
1919
});
2020

21+
describe('when populating suggestions via the source function', () => {
22+
it('it should empty out the suggestions when resolving to a falsy value', (done, fail) => {
23+
omniboxController.query = 'my query';
24+
omniboxController.suggestions = ['suggestion 1', 'suggestion 2'];
25+
omniboxController.source = () => Promise.resolve(null);
26+
omniboxController.updateSuggestions().then(() => {
27+
expect(omniboxController.suggestions).toBe(null);
28+
}).then(done, fail);
29+
});
30+
31+
it('it should set the suggestions when resolving to an Array', (done, fail) => {
32+
omniboxController.query = 'my query';
33+
omniboxController.suggestions = ['suggestion 1', 'suggestion 2'];
34+
omniboxController.source = () => Promise.resolve(['new suggestion 1', 'new suggestion 2']);
35+
omniboxController.updateSuggestions().then(() => {
36+
expect(omniboxController.suggestions.join(',')).toBe('new suggestion 1,new suggestion 2');
37+
}).then(done, fail);
38+
});
39+
40+
it('it should throw an error when resolving to a truthy, non-Array value', (done, fail) => {
41+
omniboxController.query = 'my query';
42+
omniboxController.suggestions = ['suggestion 1', 'suggestion 2'];
43+
omniboxController.source = () => Promise.resolve(true);
44+
omniboxController.updateSuggestions().then(fail, (e) => {
45+
expect(e.message).toBe('Suggestions must be an Array');
46+
}).then(done, fail);
47+
});
48+
49+
it('should support resolving suggestions and hints', (done, fail) => {
50+
omniboxController.query = 'my query';
51+
omniboxController.suggestions = [];
52+
omniboxController.source = () => {
53+
return Promise.resolve({
54+
suggestions: ['new suggestion 1', 'new suggestion 2'],
55+
hint: 'My Query is Awesome'
56+
});
57+
};
58+
omniboxController.updateSuggestions().then(() => {
59+
expect(omniboxController.suggestions.join(',')).toBe('new suggestion 1,new suggestion 2');
60+
expect(omniboxController.hint).toBe('my query is Awesome');
61+
expect(omniboxController.fullHint).toBe('My Query is Awesome');
62+
}).then(done, fail);
63+
});
64+
});
65+
2166
describe('when determining if there are suggestions', () => {
2267
it('should return false for falsy values', () => {
2368
omniboxController.suggestions = false;
@@ -30,18 +75,13 @@ describe('ngcOmnibox.angularComponent.ngcOmniboxController', () => {
3075
expect(omniboxController.hasSuggestions).toBe(false);
3176
});
3277

33-
it('should return false for suggestions that aren\'t an Array', () => {
34-
omniboxController.suggestions = 'false';
35-
expect(omniboxController.hasSuggestions).toBe(false);
36-
37-
omniboxController.suggestions = true;
38-
expect(omniboxController.hasSuggestions).toBe(false);
39-
40-
omniboxController.suggestions = 100;
41-
expect(omniboxController.hasSuggestions).toBe(false);
78+
it('should throw an Error for suggestions that aren\'t an Array', () => {
79+
const errorMsg = 'Suggestions must be an Array';
4280

43-
omniboxController.suggestions = {test: 'me'};
44-
expect(omniboxController.hasSuggestions).toBe(false);
81+
expect(() => omniboxController.suggestions = 'false').toThrowError(errorMsg);
82+
expect(() => omniboxController.suggestions = true).toThrowError(errorMsg);
83+
expect(() => omniboxController.suggestions = 100).toThrowError(errorMsg);
84+
expect(() => omniboxController.suggestions = {test: 'me'}).toThrowError(errorMsg);
4585
});
4686

4787
it('should return false for empty Arrays', () => {

src/angularComponent/ngcOmniboxController.js

Lines changed: 52 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,13 @@ export default class NgcOmniboxController {
7777
}
7878

7979
set suggestions(suggestions) {
80-
if (Array.isArray(suggestions)) {
80+
if (!suggestions) {
81+
this._suggestions = null;
82+
} else if (Array.isArray(suggestions)) {
8183
this._suggestions = Array.prototype.slice.apply(suggestions);
8284
this._buildSuggestionsUiList();
85+
} else {
86+
throw new Error('Suggestions must be an Array');
8387
}
8488

8589
this.hasSuggestions = Array.isArray(suggestions) && !!suggestions.length;
@@ -430,12 +434,56 @@ export default class NgcOmniboxController {
430434
return this._shouldShowSuggestions;
431435
}
432436

437+
/**
438+
* Updates the list of suggestions based on the current query and the source function.
439+
*
440+
* @returns {Promise} -- Resolved after the suggestions have been updated
441+
*/
442+
updateSuggestions() {
443+
this._suggestionsUiList.length = 0;
444+
this.hint = null;
445+
446+
this.highlightedIndex = -1; // Forcibly select nothing
447+
this._showLoading();
448+
this.hideSuggestions = false;
449+
450+
const promise = this.source({query: this.query, suggestions: this.suggestions});
451+
this._sourceFunctionPromise = promise;
452+
453+
return promise.then((suggestions) => {
454+
// Bail out if the promise has changed
455+
if (promise !== this._sourceFunctionPromise) {
456+
return;
457+
}
458+
459+
let hint;
460+
if (suggestions && !Array.isArray(suggestions) && typeof suggestions === 'object') {
461+
hint = suggestions.hint;
462+
suggestions = suggestions.suggestions;
463+
}
464+
465+
this.suggestions = suggestions;
466+
467+
if (hint) {
468+
// Hint with just the part of the hint that isn't the query
469+
this.hint = this.query + hint.slice(this.query.length, hint.length);
470+
this.fullHint = hint; // Store this for completion of the hint
471+
}
472+
473+
this._hideLoading();
474+
475+
if (this.requireMatch) {
476+
this.highlightNextSuggestion();
477+
}
478+
});
479+
}
480+
433481
onInputChange() {
434482
if (!this.query) {
435483
this.hideSuggestions = true;
436484
this.hint = null;
437485
} else {
438-
this._updateSuggestions();
486+
this.updateSuggestions();
439487
}
440488
}
441489

@@ -472,7 +520,7 @@ export default class NgcOmniboxController {
472520

473521
if (this.hint) {
474522
if (keyCode === KEY.RIGHT && this.selectionEndKeyDown === inputLength) {
475-
this.query = this._fullHint;
523+
this.query = this.fullHint;
476524
this.onInputChange();
477525
} else if (keyCode === KEY.ESC) {
478526
this.hint = null;
@@ -504,7 +552,7 @@ export default class NgcOmniboxController {
504552
}
505553
}
506554
} else if (keyCode === KEY.DOWN) {
507-
this._updateSuggestions();
555+
this.updateSuggestions();
508556
}
509557
}
510558

@@ -546,51 +594,6 @@ export default class NgcOmniboxController {
546594
}
547595
}
548596

549-
_updateSuggestions() {
550-
this._suggestionsUiList.length = 0;
551-
this.hint = null;
552-
553-
this.highlightedIndex = -1; // Forcibly select nothing
554-
this._showLoading();
555-
this.hideSuggestions = false;
556-
557-
const promise = this.source({query: this.query, suggestions: this.suggestions});
558-
this._sourceFunctionPromise = promise;
559-
560-
promise.then((suggestions) => {
561-
// Bail out if the promise has changed
562-
if (promise !== this._sourceFunctionPromise) {
563-
return;
564-
}
565-
566-
let hint;
567-
if (typeof suggestions === 'object') {
568-
hint = suggestions.hint;
569-
suggestions = suggestions.suggestions;
570-
}
571-
572-
if (!suggestions) {
573-
this.suggestions = null;
574-
} else if (Array.isArray(suggestions)) {
575-
this.suggestions = suggestions;
576-
} else {
577-
throw new Error('Suggestions must be an Array');
578-
}
579-
580-
if (hint) {
581-
// Hint with just the part of the hint that isn't the query
582-
this.hint = this.query + hint.slice(this.query.length, hint.length);
583-
this._fullHint = hint; // Store this for completion of the hint
584-
}
585-
586-
this._hideLoading();
587-
588-
if (this.requireMatch) {
589-
this.highlightNextSuggestion();
590-
}
591-
});
592-
}
593-
594597
_showLoading() {
595598
this.isLoading = true;
596599

0 commit comments

Comments
 (0)