From 3a89b69d82260e6ce57566fcccf81009f823292f Mon Sep 17 00:00:00 2001 From: Dmitry Nehaychik <4dmitr@gmail.com> Date: Tue, 16 Oct 2018 15:53:33 +0300 Subject: [PATCH] fix(overlay): fix click trigger Closes #907 --- .../cdk/overlay/overlay-trigger.spec.ts | 59 ++++-------- .../components/cdk/overlay/overlay-trigger.ts | 91 +++++++++---------- 2 files changed, 62 insertions(+), 88 deletions(-) diff --git a/src/framework/theme/components/cdk/overlay/overlay-trigger.spec.ts b/src/framework/theme/components/cdk/overlay/overlay-trigger.spec.ts index 90104a37c9..58cdee6da6 100644 --- a/src/framework/theme/components/cdk/overlay/overlay-trigger.spec.ts +++ b/src/framework/theme/components/cdk/overlay/overlay-trigger.spec.ts @@ -7,17 +7,29 @@ import { NB_DOCUMENT } from '../../../theme.options'; import createSpy = jasmine.createSpy; +const withContainer = el => () => ({ location: { nativeElement: el } }) as ComponentRef; +const createElement = (name = 'div') => { + const el = document.createElement(name); + document.body.appendChild(el); + return el; +}; +const click = el => el.dispatchEvent(new Event('click', { bubbles: true })); +const mouseMove = el => el.dispatchEvent(new Event('mousemove')); +const mouseEnter = el => el.dispatchEvent(new Event('mouseenter')); +const mouseLeave = el => el.dispatchEvent(new Event('mouseleave')); +const focus = el => el.dispatchEvent(new Event('focusin', { bubbles: true })); +const blur = el => el.dispatchEvent(new Event('focusout', { bubbles: true })); +const tab = (el) => el.dispatchEvent(new KeyboardEvent('keydown', { + bubbles: true, + keyCode: 9, +})); + describe('click-trigger-strategy', () => { let triggerStrategyBuilder: NbTriggerStrategyBuilder; let document: Document; let host: HTMLElement; let container: HTMLElement; - const withContainer = el => () => ({ location: { nativeElement: el } }) as ComponentRef; - - const click = el => el.dispatchEvent(new Event('click')); - - const createElement = () => document.createElement('div'); beforeEach(() => { const bed = TestBed.configureTestingModule({ providers: [{ provide: NB_DOCUMENT, useExisting: DOCUMENT }] }); @@ -26,7 +38,7 @@ describe('click-trigger-strategy', () => { beforeEach(() => { host = createElement(); - container = createElement(); + container = createElement('li'); triggerStrategyBuilder = new NbTriggerStrategyBuilder() .document(document) .trigger(NbTrigger.CLICK) @@ -73,16 +85,6 @@ describe('hover-trigger-strategy', () => { let host: HTMLElement; let container: HTMLElement; - const withContainer = el => () => ({ location: { nativeElement: el } }) as ComponentRef; - - const mouseMove = el => el.dispatchEvent(new Event('mousemove')); - - const mouseEnter = el => el.dispatchEvent(new Event('mouseenter')); - - const mouseLeave = el => el.dispatchEvent(new Event('mouseleave')); - - const createElement = () => document.createElement('div'); - beforeEach(() => { const bed = TestBed.configureTestingModule({ providers: [{ provide: NB_DOCUMENT, useExisting: DOCUMENT }] }); document = bed.get(NB_DOCUMENT); @@ -131,14 +133,6 @@ describe('hint-trigger-strategy', () => { let host: HTMLElement; let container: HTMLElement; - const withContainer = el => () => ({ location: { nativeElement: el } }) as ComponentRef; - - const mouseEnter = el => el.dispatchEvent(new Event('mouseenter')); - - const mouseLeave = el => el.dispatchEvent(new Event('mouseleave')); - - const createElement = () => document.createElement('div'); - beforeEach(() => { const bed = TestBed.configureTestingModule({ providers: [{ provide: NB_DOCUMENT, useExisting: DOCUMENT }] }); document = bed.get(NB_DOCUMENT); @@ -173,21 +167,6 @@ describe('focus-trigger-strategy', () => { let host: HTMLElement; let container: HTMLElement; - const withContainer = el => () => ({ location: { nativeElement: el } }) as ComponentRef; - - const focus = el => el.dispatchEvent(new Event('focusin', { bubbles: true })); - - const blur = el => el.dispatchEvent(new Event('focusout', { bubbles: true })); - - const click = el => el.dispatchEvent(new Event('click', { bubbles: true })); - - const tab = (el, target) => el.dispatchEvent(new KeyboardEvent('keydown', { - target: target, - keyCode: 9, - })); - - const createElement = () => document.createElement('div'); - beforeEach(() => { const bed = TestBed.configureTestingModule({ providers: [{ provide: NB_DOCUMENT, useExisting: DOCUMENT }] }); document = bed.get(NB_DOCUMENT); @@ -228,7 +207,7 @@ describe('focus-trigger-strategy', () => { it('should fire hide$ when tab pressed', done => { const triggerStrategy = triggerStrategyBuilder.build(); triggerStrategy.hide$.subscribe(done); - tab(document, host); + tab(host); }); it('should fire hide$ when focusout', done => { diff --git a/src/framework/theme/components/cdk/overlay/overlay-trigger.ts b/src/framework/theme/components/cdk/overlay/overlay-trigger.ts index 22c14410c2..79bd2d38d4 100644 --- a/src/framework/theme/components/cdk/overlay/overlay-trigger.ts +++ b/src/framework/theme/components/cdk/overlay/overlay-trigger.ts @@ -1,6 +1,7 @@ -import { fromEvent as observableFromEvent, merge as observableMerge, Observable, Subject } from 'rxjs'; -import { debounceTime, delay, filter, repeat, switchMap, takeUntil, takeWhile } from 'rxjs/operators'; +import { fromEvent as observableFromEvent, merge as observableMerge, Observable } from 'rxjs'; +import { debounceTime, delay, filter, repeat, share, switchMap, takeUntil, takeWhile } from 'rxjs/operators'; import { ComponentRef } from '@angular/core'; +import { map } from 'rxjs/operators'; export enum NbTrigger { @@ -20,6 +21,23 @@ export enum NbTrigger { * Renderer provides capability use it in service worker, ssr and so on. * */ export abstract class NbTriggerStrategy { + + protected isNotOnHostOrContainer(event: Event): boolean { + return !this.isOnHost(event) && !this.isOnContainer(event); + } + + protected isOnHostOrContainer(event: Event): boolean { + return this.isOnHost(event) || this.isOnContainer(event); + } + + protected isOnHost({ target }: Event): boolean { + return this.host.contains(target as Node); + } + + protected isOnContainer({ target }: Event): boolean { + return this.container() && this.container().location.nativeElement.contains(target); + } + abstract show$: Observable; abstract hide$: Observable; @@ -34,41 +52,29 @@ export abstract class NbTriggerStrategy { * not on the host or container. * */ export class NbClickTriggerStrategy extends NbTriggerStrategy { - protected show: Subject = new Subject(); - readonly show$: Observable = this.show.asObservable(); - protected hide: Subject = new Subject(); - readonly hide$: Observable = observableMerge( - this.hide.asObservable(), - observableFromEvent(this.document, 'click') - .pipe(filter((event: Event) => this.isNotHostOrContainer(event))), - ); - - constructor(protected document: Document, protected host: HTMLElement, protected container: () => ComponentRef) { - super(document, host, container); - this.subscribeOnHostClick(); - } - - protected subscribeOnHostClick() { - observableFromEvent(this.host, 'click') - .subscribe((event: Event) => { - if (this.isContainerExists()) { - this.hide.next(event); - } else { - this.show.next(event); - } - }) - } + // since we should track click for both SHOW and HIDE event we firstly need to track the click and the state + // of the container and then later on decide should we hide it or show + // if we track the click & state separately this will case a behavior when the container is getting shown + // and then hidden right away + protected click$: Observable = observableFromEvent(this.document, 'click') + .pipe( + map(event => [!this.container() && this.isOnHost(event), event]), + share(), + ); - protected isContainerExists(): boolean { - return !!this.container(); - } + readonly show$: Observable = this.click$ + .pipe( + filter(([shouldShow]) => shouldShow), + map(([, event]) => event), + ); - protected isNotHostOrContainer(event: Event): boolean { - return !this.host.contains(event.target as Node) - && this.isContainerExists() - && !this.container().location.nativeElement.contains(event.target as Node); - } + readonly hide$: Observable = this.click$ + .pipe( + filter(([shouldShow]) => !shouldShow), + map(([, event]) => event), + filter((event: Event) => !this.isOnContainer(event)), + ); } /** @@ -91,8 +97,7 @@ export class NbHoverTriggerStrategy extends NbTriggerStrategy { .pipe( debounceTime(100), takeWhile(() => !!this.container()), - filter(event => !this.host.contains(event.target as Node) - && !this.container().location.nativeElement.contains(event.target as Node), + filter(event => !this.isOnHostOrContainer(event), ), ), ), @@ -109,6 +114,8 @@ export class NbHintTriggerStrategy extends NbTriggerStrategy { .pipe( delay(100), takeUntil(observableFromEvent(this.host, 'mouseleave')), + // this `delay & takeUntil & repeat` operators combination is a synonym for `conditional debounce` + // meaning that if one event occurs in some time afther the initial one we won't react to it repeat(), ); @@ -123,18 +130,6 @@ export class NbHintTriggerStrategy extends NbTriggerStrategy { * */ export class NbFocusTriggerStrategy extends NbTriggerStrategy { - protected isNotOnHostOrContainer(event: Event): boolean { - return !this.isOnHost(event) && !this.isOnContainer(event); - } - - protected isOnHost({ target }: Event): boolean { - return this.host.contains(target as Node); - } - - protected isOnContainer({ target }: Event): boolean { - return this.container() && this.container().location.nativeElement.contains(target); - } - protected focusOut$: Observable = observableFromEvent(this.host, 'focusout') .pipe( switchMap(() => observableFromEvent(this.document, 'focusin')