Skip to content

Commit

Permalink
Fix bugs with choose/unchoose (#63)
Browse files Browse the repository at this point in the history
1. If the default was prevented, then performed, we were no longer
  checking to make sure it wasn't already added to the list of choices.
  This meant that if the implementor added it themselves it would be
  then added twice
2. The UI behavior performed once a choice was added was being
  grouped into the performDefault function, meaning if the implementor
  wanted to update the model themselves and prevented default, the UI
  would be out of sync. I've pulled that out of the performDefault
  function and now always do it since it has nothing to do with the data
  model
  • Loading branch information
josephschmitt authored Jun 7, 2017
1 parent 66c3175 commit f685d5c
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 10 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "ngc-omnibox",
"version": "0.4.0",
"version": "0.4.1",
"description": "A modern, flexible, Angular 1.x autocomplete library with limited assumptions.",
"main": "dist/ngc-omnibox.js",
"scripts": {
Expand Down
15 changes: 15 additions & 0 deletions spec/tests/angularComponent/ngcOmniboxControllerSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,21 @@ describe('ngcOmnibox.angularComponent.ngcOmniboxController', () => {
omniboxController.choose('three');
});

it('should not add dupes to ngModel when onChosen event prevents/peforms default', (done) => {
omniboxController.onChosen = ({$event}) => {
$event.preventDefault();

omniboxController.ngModel.push('three');

$event.performDefault();
expect(omniboxController.ngModel).toEqual(['one', 'two', 'three']);

done();
};

omniboxController.choose('three');
});

it('should not update the ngModel when choosing if an item is not selectable', () => {
omniboxController.isSelectable = () => false;
omniboxController.choose('three');
Expand Down
17 changes: 8 additions & 9 deletions src/angularComponent/ngcOmniboxController.js
Original file line number Diff line number Diff line change
Expand Up @@ -393,30 +393,29 @@ export default class NgcOmniboxController {
* @param {Boolean} shouldFocusField -- Whether to focus the input field after choosing
*/
choose(item, shouldFocusField = true) {
if (item && !(Array.isArray(this.ngModel) && this.ngModel.indexOf(item) >= 0) &&
this.isSelectable({suggestion: item}) !== false) {
if (item && this.isSelectable({suggestion: item}) !== false) {

const $event = {
isDefaultPrevented: false,
preventDefault: () => $event.isDefaultPrevented = true,
performDefault: () => {
$event.isDefaultPrevented = false;

if (this.multiple) {
if (this.multiple && !(Array.isArray(this.ngModel) && this.ngModel.indexOf(item) >= 0)) {
this.ngModel = this.ngModel || [];
this.ngModel.push(item);
} else {
} else if (!this.multiple) {
this.ngModel = item;
}

this.query = '';
shouldFocusField && this.focus();
this.hideSuggestions = true;
}
};

this.onChosen({choice: item, $event});
!$event.isDefaultPrevented && $event.performDefault();

this.query = '';
shouldFocusField && this.focus();
this.hideSuggestions = true;
}
}

Expand All @@ -433,7 +432,7 @@ export default class NgcOmniboxController {
isDefaultPrevented: false,
preventDefault: () => $event.isDefaultPrevented = true,
performDefault: () => {
if (Array.isArray(this.ngModel)) {
if (this.multiple && Array.isArray(this.ngModel)) {
this.ngModel.splice(this.ngModel.indexOf(item), 1);
} else if (!this.multiple) {
this.ngModel = null;
Expand Down

0 comments on commit f685d5c

Please sign in to comment.