fix(navigation): close dropdown on tab click fixes #46 (#53)

ng-select · Sep 30, 2017 · 32ff7e3 · 32ff7e3
1 parent 509c376
commit 32ff7e3
Show file tree

Hide file tree

Showing 4 changed files with 122 additions and 73 deletions.
diff --git a/src/lib/src/items-list.ts b/src/lib/src/items-list.ts
@@ -40,7 +40,7 @@ export class ItemsList {
         item.selected = false;
     }
 
-    unSelectLastItem() {
+    unselectLastItem() {
         if (this._selected.length === 0) {
             return;
         }

diff --git a/src/lib/src/ng-select.component.spec.ts b/src/lib/src/ng-select.component.spec.ts
@@ -266,75 +266,123 @@ describe('NgSelectComponent', function () {
                 </ng-select>`);
         });
 
-        it('open dropdown on space click', () => {
-            triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Space);
-            expect(fixture.componentInstance.select.isOpen).toBe(true);
-        });
+        describe('space', () => {
+            it('should open dropdown', () => {
+                triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Space);
+                expect(fixture.componentInstance.select.isOpen).toBe(true);
+            });
 
-        it('should open empty dropdown', fakeAsync(() => {
-            fixture.componentInstance.cities = [];
-            tickAndDetectChanges(fixture);
-            triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Space);
-            tickAndDetectChanges(fixture);
-            const text = fixture.debugElement.query(By.css('.as-option')).nativeElement.innerHTML;
-            expect(text).toContain('No items found');
-        }));
+            it('should open empty dropdown if no items', fakeAsync(() => {
+                fixture.componentInstance.cities = [];
+                tickAndDetectChanges(fixture);
+                triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Space);
+                tickAndDetectChanges(fixture);
+                const text = fixture.debugElement.query(By.css('.as-option')).nativeElement.innerHTML;
+                expect(text).toContain('No items found');
+            }));
 
-        it('should mark first item on open', () => {
-            const result = fixture.componentInstance.cities[0];
-            triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Space);
-            expect(fixture.componentInstance.select.itemsList.markedItem).toEqual(jasmine.objectContaining(result));
-            triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Enter);
-            expect(fixture.componentInstance.select.value).toEqual(jasmine.objectContaining(result));
+            it('should open dropdown and mark first item', () => {
+                const result = fixture.componentInstance.cities[0];
+                triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Space);
+                expect(fixture.componentInstance.select.itemsList.markedItem).toEqual(jasmine.objectContaining(result));
+            });
         });
 
-        it('select next value on arrow down', () => {
-            selectOption(fixture, KeyCode.ArrowDown, 1);
-            expect(fixture.componentInstance.select.value).toEqual(jasmine.objectContaining(fixture.componentInstance.cities[1]));
-        });
+        describe('arrows', () => {
+            it('should select next value on arrow down', () => {
+                selectOption(fixture, KeyCode.ArrowDown, 1);
+                expect(fixture.componentInstance.select.value).toEqual(jasmine.objectContaining(fixture.componentInstance.cities[1]));
+            });
 
-        it('select first value on arrow down when current selected value is last', async(() => {
-            fixture.componentInstance.selectedCity = fixture.componentInstance.cities[2];
-            fixture.detectChanges();
+            it('should select first value on arrow down when current value is last', fakeAsync(() => {
+                fixture.componentInstance.selectedCity = fixture.componentInstance.cities[2];
+                tickAndDetectChanges(fixture);
+                selectOption(fixture, KeyCode.ArrowDown, 1);
+                tickAndDetectChanges(fixture);
+                expect(fixture.componentInstance.select.value).toEqual(jasmine.objectContaining(fixture.componentInstance.cities[0]));
+            }));
 
-            fixture.whenStable().then(() => {
+            it('should skip disabled option and select next one', fakeAsync(() => {
+                const city: any = fixture.componentInstance.cities[0];
+                city.disabled = true;
                 selectOption(fixture, KeyCode.ArrowDown, 1);
+                tickAndDetectChanges(fixture);
+                expect(fixture.componentInstance.select.value).toEqual(jasmine.objectContaining(fixture.componentInstance.cities[1]));
+            }));
+
+            it('should select previous value on arrow up', fakeAsync(() => {
+                fixture.componentInstance.selectedCity = fixture.componentInstance.cities[1];
+                tickAndDetectChanges(fixture);
+                selectOption(fixture, KeyCode.ArrowUp, 1);
+                tickAndDetectChanges(fixture);
                 expect(fixture.componentInstance.select.value).toEqual(jasmine.objectContaining(fixture.componentInstance.cities[0]));
+            }));
+
+            it('should select last value on arrow up', () => {
+                selectOption(fixture, KeyCode.ArrowUp, 1);
+                expect(fixture.componentInstance.select.value).toEqual(jasmine.objectContaining(fixture.componentInstance.cities[2]));
             });
-        }));
+        });
 
-        it('should skip disabled option and select next one', fakeAsync(() => {
-            const city: any = fixture.componentInstance.cities[0];
-            city.disabled = true;
-            selectOption(fixture, KeyCode.ArrowDown, 1);
-            tickAndDetectChanges(fixture);
-            expect(fixture.componentInstance.select.value).toEqual(jasmine.objectContaining(fixture.componentInstance.cities[1]));
-        }));
+        describe('esc', () => {
+            it('should close opened dropdown', () => {
+                fixture.componentInstance.select.isOpen = true;
+                triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Esc);
+                expect(fixture.componentInstance.select.isOpen).toBe(false);
+            });
+        });
 
-        it('select previous value on arrow up', fakeAsync(() => {
-            fixture.componentInstance.selectedCity = fixture.componentInstance.cities[1];
-            tickAndDetectChanges(fixture);
-            selectOption(fixture, KeyCode.ArrowUp, 1);
-            tickAndDetectChanges(fixture);
-            expect(fixture.componentInstance.select.value).toEqual(jasmine.objectContaining(fixture.componentInstance.cities[0]));
-        }));
+        describe('tab', () => {
+            it('should close dropdown and select marked value', () => {
+                triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Space);
+                triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Tab);
+                expect(fixture.componentInstance.select.value).toEqual(jasmine.objectContaining(fixture.componentInstance.cities[0]));
+                expect(fixture.componentInstance.select.isOpen).toBeFalsy()
+            });
 
-        it('select last value on arrow up', () => {
-            selectOption(fixture, KeyCode.ArrowUp, 1);
-            expect(fixture.componentInstance.select.value).toEqual(jasmine.objectContaining(fixture.componentInstance.cities[2]));
-        });
+            it('should close dropdown when marked item is already selected', () => {
+                fixture.componentInstance.selectedCity = fixture.componentInstance.cities[0];
+                triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Space);
+                triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Tab);
+                expect(fixture.componentInstance.select.value).toEqual(jasmine.objectContaining(fixture.componentInstance.cities[0]));
+                expect(fixture.componentInstance.select.isOpen).toBeFalsy()
+            });
 
-        it('close opened dropdown on esc click', () => {
-            fixture.componentInstance.select.isOpen = true;
-            triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Esc);
-            expect(fixture.componentInstance.select.isOpen).toBe(false);
+            it('should close dropdown and select marked when multiple', () => {
+                fixture.componentInstance.select.multiple = true;
+                triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Space);
+                triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Tab);
+                expect(fixture.componentInstance.select.value).toEqual(jasmine.objectContaining(fixture.componentInstance.cities[0]));
+                expect(fixture.componentInstance.select.isOpen).toBeFalsy()
+            });
         });
 
-        it('should close opened dropdown and select marked value on tab click', () => {
-            triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Space);
-            triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Tab);
-            expect(fixture.componentInstance.select.value).toEqual(jasmine.objectContaining(fixture.componentInstance.cities[0]));
-            expect(fixture.componentInstance.select.isOpen).toBeFalsy()
+        describe('backspace', () => {
+            it('should remove selected value', fakeAsync(() => {
+                fixture.componentInstance.selectedCity = fixture.componentInstance.cities[0];
+                tickAndDetectChanges(fixture);
+                triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Backspace);
+                expect(fixture.componentInstance.select.value).toBeNull();
+            }));
+
+            it('should not remove selected value if filter is set', fakeAsync(() => {
+                fixture.componentInstance.select.filterValue = 'a';
+                fixture.componentInstance.selectedCity = fixture.componentInstance.cities[0];
+                tickAndDetectChanges(fixture);
+                triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Backspace);
+                expect(fixture.componentInstance.select.value).toEqual(jasmine.objectContaining(fixture.componentInstance.cities[0]));
+            }));
+
+            it('should remove last selected value when multiple', fakeAsync(() => {
+                fixture.componentInstance.select.multiple = true;
+                fixture.componentInstance.cities = [...fixture.componentInstance.cities];
+                tickAndDetectChanges(fixture);
+                selectOption(fixture, KeyCode.ArrowDown, 1);
+                selectOption(fixture, KeyCode.ArrowDown, 1);
+                tickAndDetectChanges(fixture);
+                triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Backspace);
+                expect(fixture.componentInstance.select.value).toEqual([jasmine.objectContaining(fixture.componentInstance.cities[1])]);
+            }));
         });
     });
 
@@ -652,14 +700,14 @@ describe('NgSelectComponent', function () {
         }));
 
         it('should clear model on clear icon click', fakeAsync(() => {
-            clickIcon.triggerEventHandler('click', {stopPropagation: () => {}});
+            clickIcon.triggerEventHandler('click', { stopPropagation: () => { } });
             tickAndDetectChanges(fixture);
 
             expect(fixture.componentInstance.selectedCity).toBe(null);
         }));
 
         it('should not open dropdown on clear click', fakeAsync(() => {
-            clickIcon.triggerEventHandler('click', {stopPropagation: () => {}});
+            clickIcon.triggerEventHandler('click', { stopPropagation: () => { } });
             tickAndDetectChanges(fixture);
 
             expect(fixture.componentInstance.select.isOpen).toBe(false);
@@ -684,7 +732,7 @@ describe('NgSelectComponent', function () {
         }));
 
         it('should toggle dropdown', fakeAsync(() => {
-            const clickArrow = () => arrowIcon.triggerEventHandler('click', {stopPropagation: () => {}});
+            const clickArrow = () => arrowIcon.triggerEventHandler('click', { stopPropagation: () => { } });
             // open
             clickArrow();
             tickAndDetectChanges(fixture);

diff --git a/src/lib/src/ng-select.component.ts b/src/lib/src/ng-select.component.ts
@@ -139,7 +139,7 @@ export class NgSelectComponent implements OnInit, OnDestroy, ControlValueAccesso
                 case KeyCode.Esc:
                     this.close();
                     break;
-                case KeyCode.BackSpace:
+                case KeyCode.Backspace:
                     this.handleBackspace();
                     break;
             }
@@ -264,13 +264,12 @@ export class NgSelectComponent implements OnInit, OnDestroy, ControlValueAccesso
     }
 
     select(item: NgOption) {
-        if (item.selected) {
-            return;
+        if (!item.selected) {
+            this.itemsList.select(item);
+            this.updateModel();
         }
 
-        this.itemsList.select(item);
-        this.updateModel();
-        if (!this.multiple) {
+        if (this.single) {
             this.close();
         }
     }
@@ -333,6 +332,9 @@ export class NgSelectComponent implements OnInit, OnDestroy, ControlValueAccesso
     }
 
     onItemHover(item: NgOption) {
+        if (item.disabled) {
+            return;
+        }
         this.itemsList.markItem(item);
     }
 
@@ -374,7 +376,7 @@ export class NgSelectComponent implements OnInit, OnDestroy, ControlValueAccesso
 
     private focusSearchInput() {
         setTimeout(() => {
-            this.filterInput.nativeElement.focus();
+            this.filterInput.nativeElement.focus(); // TODO: this won't work on mobile
         });
     }
 
@@ -384,7 +386,10 @@ export class NgSelectComponent implements OnInit, OnDestroy, ControlValueAccesso
 
     private handleTab($event: KeyboardEvent) {
         if (this.isOpen) {
-            this.toggle(this.itemsList.markedItem);
+            this.select(this.itemsList.markedItem);
+            if (this.multiple) {
+                this.close();
+            }
         }
     }
 
@@ -421,7 +426,8 @@ export class NgSelectComponent implements OnInit, OnDestroy, ControlValueAccesso
 
     private handleBackspace() {
         if (this.multiple) {
-            this.unSelectLastItem();
+            this.itemsList.unselectLastItem();
+            this.updateModel();
         } else {
             if (this.filterValue) {
                 return;
@@ -430,11 +436,6 @@ export class NgSelectComponent implements OnInit, OnDestroy, ControlValueAccesso
         }
     }
 
-    private unSelectLastItem() {
-        this.itemsList.unSelectLastItem();
-        this.updateModel();
-    }
-
     private notifyModelChanged() {
         const value = this.itemsList.value;
         if (!value) {

diff --git a/src/lib/src/ng-select.types.ts b/src/lib/src/ng-select.types.ts
@@ -15,7 +15,7 @@ export enum KeyCode {
     Space = 32,
     ArrowUp = 38,
     ArrowDown = 40,
-    BackSpace = 8
+    Backspace = 8
 }
 
 export class NgSelectConfig {