fix(select, tree-select): need prevent selecting tags (#UIM-91) (#249)

positive-js · Sep 17, 2019 · 1532e07 · 1532e07
1 parent 825157a
commit 1532e07
Show file tree

Hide file tree

Showing 7 changed files with 68 additions and 29 deletions.
diff --git a/packages/mosaic-dev/tags/module.ts b/packages/mosaic-dev/tags/module.ts
@@ -1,4 +1,5 @@
 import {
+    AfterViewInit,
     Component,
     ElementRef,
     NgModule, OnInit,
@@ -25,7 +26,7 @@ import { map } from 'rxjs/operators';
     styleUrls: ['./styles.scss'],
     encapsulation: ViewEncapsulation.None
 })
-export class DemoComponent implements OnInit {
+export class DemoComponent implements AfterViewInit {
     visible = true;
     tagCtrl = new FormControl();
 
@@ -46,7 +47,7 @@ export class DemoComponent implements OnInit {
     @ViewChild('autocompleteTagInput', {static: false}) autocompleteTagInput: ElementRef<HTMLInputElement>;
     @ViewChild('autocompleteTagList', {static: false}) autocompleteTagList: McTagList;
 
-    ngOnInit(): void {
+    ngAfterViewInit(): void {
         this.autocompleteFilteredTags = merge(
             this.autocompleteTagList.tagChanges.asObservable()
                 .pipe(map((selectedTags) => {

diff --git a/packages/mosaic/select/select.html b/packages/mosaic/select/select.html
@@ -10,7 +10,10 @@
             <div *ngSwitchDefault [ngSwitch]="multiple" class="mc-select__match-container">
                 <span *ngSwitchCase="false" class="mc-select__matcher-text">{{ triggerValue }}</span>
                 <div *ngSwitchCase="true" class="mc-select__match-list">
-                    <mc-tag *ngFor="let option of triggerValues" [disabled]="disabled" [class.mc-error]="errorState">
+                    <mc-tag *ngFor="let option of triggerValues"
+                            [disabled]="disabled"
+                            [selectable]="false"
+                            [class.mc-error]="errorState">
                         {{ option.viewValue || option.value }}
                         <i mc-icon="mc-close-S_16" (click)="onRemoveMatcherItem(option, $event)"></i>
                     </mc-tag>

diff --git a/packages/mosaic/tags/_tag-theme.scss b/packages/mosaic/tags/_tag-theme.scss
@@ -11,8 +11,7 @@
     border-color: mc-color($main-color, if($is-dark-theme, 700, 100));
 
     &.mc-active,
-    &.mc-focused,
-    &:focus {
+    &.mc-focused {
         border-color: mc-color($focus-color);
         box-shadow: 0 0 0 1px mc-color($focus-color);
     }

diff --git a/packages/mosaic/tags/tag-list.component.spec.ts b/packages/mosaic/tags/tag-list.component.spec.ts
@@ -13,7 +13,7 @@ import {
     ViewChild,
     ViewChildren
 } from '@angular/core';
-import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing';
+import { ComponentFixture, fakeAsync, flush, TestBed, tick } from '@angular/core/testing';
 import { FormControl, FormsModule, NgForm, ReactiveFormsModule, Validators } from '@angular/forms';
 import { By } from '@angular/platform-browser';
 import { BrowserAnimationsModule, NoopAnimationsModule } from '@angular/platform-browser/animations';
@@ -206,21 +206,22 @@ describe('MatTagList', () => {
                     expect(manager.activeItemIndex).toEqual(2);
                 });
 
-                it('should focus the previous item', () => {
+                it('should focus the previous item', fakeAsync(() => {
                     const array = tags.toArray();
                     const lastIndex = array.length - 1;
                     const lastItem = array[lastIndex];
 
                     // Focus the last item
                     lastItem.focus();
+                    flush();
 
                     // Destroy the last item
                     testComponent.tags.pop();
                     fixture.detectChanges();
 
                     // It focuses the next-to-last item
                     expect(manager.activeItemIndex).toEqual(lastIndex - 1);
-                });
+                }));
 
                 it('should not focus if tag list is not focused', () => {
                     const array = tags.toArray();
@@ -253,6 +254,7 @@ describe('MatTagList', () => {
                     tags = tagListInstance.tags;
 
                     tags.last.focus();
+                    flush();
                     fixture.detectChanges();
 
                     expect(tagListInstance.keyManager.activeItemIndex).toBe(tags.length - 1);
@@ -274,7 +276,7 @@ describe('MatTagList', () => {
                     manager = tagListInstance.keyManager;
                 });
 
-                it('should focus previous item when press LEFT ARROW', () => {
+                it('should focus previous item when press LEFT ARROW', fakeAsync(() => {
                     const nativeTags = tagListNativeElement.querySelectorAll('mc-tag');
                     const lastNativeChip = nativeTags[nativeTags.length - 1] as HTMLElement;
 
@@ -285,6 +287,7 @@ describe('MatTagList', () => {
 
                     // Focus the last item in the array
                     lastItem.focus();
+                    flush();
                     expect(manager.activeItemIndex).toEqual(lastIndex);
 
                     // Press the LEFT arrow
@@ -294,9 +297,9 @@ describe('MatTagList', () => {
 
                     // It focuses the next-to-last item
                     expect(manager.activeItemIndex).toEqual(lastIndex - 1);
-                });
+                }));
 
-                it('should focus next item when press RIGHT ARROW', () => {
+                it('should focus next item when press RIGHT ARROW', fakeAsync(() => {
                     const nativeTags = tagListNativeElement.querySelectorAll('mc-tag');
                     const firstNativeChip = nativeTags[0] as HTMLElement;
 
@@ -306,6 +309,7 @@ describe('MatTagList', () => {
 
                     // Focus the last item in the array
                     firstItem.focus();
+                    flush();
                     expect(manager.activeItemIndex).toEqual(0);
 
                     // Press the RIGHT arrow
@@ -315,7 +319,7 @@ describe('MatTagList', () => {
 
                     // It focuses the next-to-last item
                     expect(manager.activeItemIndex).toEqual(1);
-                });
+                }));
 
                 it('should not handle arrow key events from non-chip elements', () => {
                     const event: KeyboardEvent = createKeyboardEvent('keydown', RIGHT_ARROW, tagListNativeElement);
@@ -366,7 +370,7 @@ describe('MatTagList', () => {
                     manager = tagListInstance.keyManager;
                 });
 
-                it('should focus previous item when press RIGHT ARROW', () => {
+                it('should focus previous item when press RIGHT ARROW', fakeAsync(() => {
                     const nativeTags = tagListNativeElement.querySelectorAll('mc-tag');
                     const lastNativeChip = nativeTags[nativeTags.length - 1] as HTMLElement;
 
@@ -378,6 +382,7 @@ describe('MatTagList', () => {
 
                     // Focus the last item in the array
                     lastItem.focus();
+                    flush();
                     expect(manager.activeItemIndex).toEqual(lastIndex);
 
                     // Press the RIGHT arrow
@@ -387,9 +392,9 @@ describe('MatTagList', () => {
 
                     // It focuses the next-to-last item
                     expect(manager.activeItemIndex).toEqual(lastIndex - 1);
-                });
+                }));
 
-                it('should focus next item when press LEFT ARROW', () => {
+                it('should focus next item when press LEFT ARROW', fakeAsync(() => {
                     const nativeTags = tagListNativeElement.querySelectorAll('mc-tag');
                     const firstNativeChip = nativeTags[0] as HTMLElement;
 
@@ -400,6 +405,7 @@ describe('MatTagList', () => {
 
                     // Focus the last item in the array
                     firstItem.focus();
+                    flush();
                     expect(manager.activeItemIndex).toEqual(0);
 
                     // Press the LEFT arrow
@@ -409,7 +415,7 @@ describe('MatTagList', () => {
 
                     // It focuses the next-to-last item
                     expect(manager.activeItemIndex).toEqual(1);
-                });
+                }));
 
                 it('should allow focus to escape when tabbing away', fakeAsync(() => {
                     tagListInstance.keyManager.onKeydown(createKeyboardEvent('keydown', TAB));
@@ -441,7 +447,7 @@ describe('MatTagList', () => {
                 }));
             });
 
-            it('should account for the direction changing', () => {
+            it('should account for the direction changing', fakeAsync(() => {
                 setupStandardList();
                 manager = tagListInstance.keyManager;
 
@@ -454,6 +460,7 @@ describe('MatTagList', () => {
                 const firstItem = array[0];
 
                 firstItem.focus();
+                flush();
                 expect(manager.activeItemIndex).toBe(0);
 
                 tagListInstance.keydown(RIGHT_EVENT);
@@ -470,7 +477,7 @@ describe('MatTagList', () => {
                 fixture.detectChanges();
 
                 expect(manager.activeItemIndex).toBe(0);
-            });
+            }));
         });
     });
 
@@ -485,19 +492,21 @@ describe('MatTagList', () => {
                 manager = tagListInstance.keyManager;
             });
 
-            it('should maintain focus if the active tag is deleted', () => {
+            it('should maintain focus if the active tag is deleted', fakeAsync(() => {
                 const secondTag = fixture.nativeElement.querySelectorAll('.mc-tag')[1];
 
                 secondTag.focus();
                 fixture.detectChanges();
+                flush();
 
                 expect(tagListInstance.tags.toArray().findIndex((tag) => tag.hasFocus)).toBe(1);
 
                 dispatchKeyboardEvent(secondTag, 'keydown', DELETE);
                 fixture.detectChanges();
+                flush();
 
                 expect(tagListInstance.tags.toArray().findIndex((tag) => tag.hasFocus)).toBe(1);
-            });
+            }));
 
             describe('when the input has focus', () => {
 
@@ -820,6 +829,7 @@ describe('MatTagList', () => {
                 const formField: HTMLElement = fixture.nativeElement.querySelector('.mc-form-field');
 
                 nativeTags[0].focus();
+                flush();
                 fixture.detectChanges();
 
                 expect(formField.classList).toContain('mc-focused');
@@ -1061,6 +1071,7 @@ describe('MatTagList', () => {
             // Remove the tags via backspace to simulate the user removing them.
             chipEls.forEach((tag) => {
                 tag.focus();
+                flush();
                 dispatchKeyboardEvent(tag, 'keydown', BACKSPACE);
                 fixture.detectChanges();
                 tick();
@@ -1430,7 +1441,7 @@ class MultiSelectionTagList {
                    [mcTagInputFor]="tagList1"
                    [mcTagInputSeparatorKeyCodes]="separatorKeyCodes"
                    [mcTagInputAddOnBlur]="addOnBlur"
-                   (mcTagInputTokenEnd)="add($event)"/>/>
+                   (mcTagInputTokenEnd)="add($event)"/>
         </mc-form-field>
     `
 })

diff --git a/packages/mosaic/tags/tag.component.spec.ts b/packages/mosaic/tags/tag.component.spec.ts
@@ -120,12 +120,12 @@ describe('Tags', () => {
 
             it('allows selection', () => {
                 spyOn(testComponent, 'tagSelectionChange');
-                expect(tagNativeElement.classList).not.toContain('mc-tag-selected');
+                expect(tagNativeElement.classList).not.toContain('mc-selected');
 
                 testComponent.selected = true;
                 fixture.detectChanges();
 
-                expect(tagNativeElement.classList).toContain('mc-tag-selected');
+                expect(tagNativeElement.classList).toContain('mc-selected');
                 expect(testComponent.tagSelectionChange)
                     .toHaveBeenCalledWith({ source: tagInstance, isUserInput: false, selected: true });
             });

diff --git a/packages/mosaic/tags/tag.component.ts b/packages/mosaic/tags/tag.component.ts
@@ -1,6 +1,7 @@
 import { coerceBooleanProperty } from '@angular/cdk/coercion';
 import {
     ChangeDetectionStrategy,
+    ChangeDetectorRef,
     Component,
     ContentChild,
     ContentChildren,
@@ -81,13 +82,17 @@ export const _McTagMixinBase: CanColorCtor & CanDisableCtor & typeof McTagBase =
     inputs: ['color', 'disabled'],
     host: {
         class: 'mc-tag',
-        '[attr.tabindex]': 'disabled ? null : -1',
-        '[class.mc-tag-selected]': 'selected',
+
+        '[attr.tabindex]': 'tabindex',
+        '[attr.disabled]': 'disabled || null',
+
+        '[class.mc-selected]': 'selected',
+        '[class.mc-focused]': 'hasFocus',
         '[class.mc-tag-with-avatar]': 'avatar',
         '[class.mc-tag-with-trailing-icon]': 'trailingIcon || removeIcon',
         '[class.mc-tag-disabled]': 'disabled',
         '[class.mc-disabled]': 'disabled',
-        '[attr.disabled]': 'disabled || null',
+
         '(click)': 'handleClick($event)',
         '(keydown)': 'handleKeydown($event)',
         '(focus)': 'focus()',
@@ -194,6 +199,12 @@ export class McTag extends _McTagMixinBase implements IFocusableOption, OnDestro
 
     private _removable: boolean = true;
 
+    get tabindex(): any {
+        if (!this.selectable) { return null; }
+
+        return this.disabled ? null : -1;
+    }
+
     get disabled() {
         return this._disabled;
     }
@@ -206,7 +217,11 @@ export class McTag extends _McTagMixinBase implements IFocusableOption, OnDestro
 
     private _disabled: boolean = false;
 
-    constructor(public elementRef: ElementRef, private _ngZone: NgZone) {
+    constructor(
+        public elementRef: ElementRef,
+        public changeDetectorRef: ChangeDetectorRef,
+        private _ngZone: NgZone
+    ) {
         super(elementRef);
 
         this.addHostClassName();
@@ -293,11 +308,18 @@ export class McTag extends _McTagMixinBase implements IFocusableOption, OnDestro
 
     /** Allows for programmatic focusing of the tag. */
     focus(): void {
+        if (!this.selectable) { return; }
+
         if (!this.hasFocus) {
             this.elementRef.nativeElement.focus();
+
             this.onFocus.next({ tag: this });
+
+            Promise.resolve().then(() => {
+                this.hasFocus = true;
+                this.changeDetectorRef.markForCheck();
+            });
         }
-        this.hasFocus = true;
     }
 
     /**

diff --git a/packages/mosaic/tree-select/tree-select.html b/packages/mosaic/tree-select/tree-select.html
@@ -10,7 +10,10 @@
             <div *ngSwitchDefault [ngSwitch]="multiple" class="mc-tree-select__match-container">
                 <span *ngSwitchCase="false" class="mc-tree-select__matcher-text">{{ triggerValue }}</span>
                 <div *ngSwitchCase="true" class="mc-tree-select__match-list">
-                    <mc-tag *ngFor="let option of selected" [disabled]="disabled" [class.mc-error]="errorState">
+                    <mc-tag *ngFor="let option of selected"
+                            [selectable]="false"
+                            [disabled]="disabled"
+                            [class.mc-error]="errorState">
                         {{ option }}
                         <i mc-icon="mc-close-S_16" (click)="onRemoveSelectedOption(option, $event)"></i>
                     </mc-tag>