Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(cdk): destroy overlay triggers manually #1316

Merged
merged 6 commits into from
Mar 29, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
Input,
Type,
} from '@angular/core';
import { takeUntil } from 'rxjs/operators';

import { NbRenderableContainer } from '../overlay-container';
import {
Expand Down Expand Up @@ -146,6 +147,8 @@ export class MockTriggerStrategyBuilder {
show$ = new Subject<any>();
hide$ = new Subject<any>();

private destroyed$ = new Subject();

trigger(trigger: NbTrigger): this {
this._trigger = trigger;
return this;
Expand All @@ -163,9 +166,10 @@ export class MockTriggerStrategyBuilder {

build(): NbTriggerStrategy {
return {
show$: this.show$,
hide$: this.hide$,
} as NbTriggerStrategy;
show$: this.show$.asObservable().pipe(takeUntil(this.destroyed$)),
hide$: this.hide$.asObservable().pipe(takeUntil(this.destroyed$)),
destroy: () => this.destroyed$.next(),
};
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import { ElementRef, Injectable, SimpleChange, Type } from '@angular/core';
import { takeUntil } from 'rxjs/operators';
import { Subject } from 'rxjs';

import { NbTrigger, NbTriggerStrategy, NbTriggerStrategyBuilderService } from '../overlay-trigger';
import {
Expand Down Expand Up @@ -37,9 +35,9 @@ export class NbDynamicOverlayHandler {
protected _offset: number = 15;

protected dynamicOverlay: NbDynamicOverlay;
protected triggerStrategy: NbTriggerStrategy;
Tibing marked this conversation as resolved.
Show resolved Hide resolved

protected positionStrategy: NbAdjustableConnectedPositionStrategy;
protected disconnect$ = new Subject();

protected changes: { [key: string]: NbDynamicOverlayChange } = {};

Expand Down Expand Up @@ -155,7 +153,9 @@ export class NbDynamicOverlayHandler {
}

disconnect() {
this.disconnect$.next();
if (this.triggerStrategy) {
this.triggerStrategy.destroy();
}
}

destroy() {
Expand All @@ -175,22 +175,14 @@ export class NbDynamicOverlayHandler {
}

protected subscribeOnTriggers(dynamicOverlay: NbDynamicOverlay) {

const triggerStrategy: NbTriggerStrategy = this.triggerStrategyBuilder
this.triggerStrategy = this.triggerStrategyBuilder
.trigger(this._trigger)
.host(this._host.nativeElement)
.container(() => dynamicOverlay.getContainer())
.build();

triggerStrategy.show$.pipe(
takeUntil(this.disconnect$),
)
.subscribe(() => dynamicOverlay.show());

triggerStrategy.hide$.pipe(
takeUntil(this.disconnect$),
)
.subscribe(() => dynamicOverlay.hide());
this.triggerStrategy.show$.subscribe(() => dynamicOverlay.show());
this.triggerStrategy.hide$.subscribe(() => dynamicOverlay.hide());
}

protected isContainerRerenderRequired() {
Expand Down
82 changes: 49 additions & 33 deletions src/framework/theme/components/cdk/overlay/overlay-trigger.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { ComponentRef, Inject, Injectable } from '@angular/core';
import { EMPTY, fromEvent as observableFromEvent, merge as observableMerge, Observable } from 'rxjs';
import { debounceTime, delay, filter, repeat, share, switchMap, takeUntil, takeWhile, map } from 'rxjs/operators';
import { EMPTY, fromEvent as observableFromEvent, merge as observableMerge, Observable, Subject } from 'rxjs';
import { debounceTime, delay, filter, map, repeat, share, switchMap, takeUntil, takeWhile } from 'rxjs/operators';
import { NB_DOCUMENT } from '../../../theme.options';

export enum NbTrigger {
Expand All @@ -19,6 +19,8 @@ export enum NbTrigger {
export interface NbTriggerStrategy {
show$: Observable<never | Event>;
hide$: Observable<never | Event>;

destroy();
}

/**
Expand All @@ -27,6 +29,12 @@ export interface NbTriggerStrategy {
* */
export abstract class NbTriggerStrategyBase implements NbTriggerStrategy {

destroy() {
this.destroyed$.next();
}

protected destroyed$ = new Subject();

protected isNotOnHostOrContainer(event: Event): boolean {
return !this.isOnHost(event) && !this.isOnContainer(event);
}
Expand All @@ -43,14 +51,6 @@ export abstract class NbTriggerStrategyBase implements NbTriggerStrategy {
return this.container() && this.container().location.nativeElement.contains(target);
}

protected isElementInBody(element: HTMLElement): boolean {
return this.document.body.contains(element);
}

protected isHostInBody(): boolean {
return this.isElementInBody(this.host);
}

abstract show$: Observable<Event>;
abstract hide$: Observable<Event>;

Expand All @@ -72,23 +72,23 @@ export class NbClickTriggerStrategy extends NbTriggerStrategyBase {
// and then hidden right away
protected click$: Observable<[boolean, Event]> = observableFromEvent<Event>(this.document, 'click')
.pipe(
takeWhile(() => this.isHostInBody()),
map((event: Event) => [!this.container() && this.isOnHost(event), event] as [boolean, Event]),
share(),
takeUntil(this.destroyed$),
);

readonly show$: Observable<Event> = this.click$
.pipe(
takeWhile(() => this.isHostInBody()),
filter(([shouldShow]) => shouldShow),
map(([, event]) => event),
takeUntil(this.destroyed$),
);

readonly hide$: Observable<Event> = this.click$
.pipe(
takeWhile(() => this.isHostInBody()),
filter(([shouldShow, event]) => !shouldShow && !this.isOnContainer(event)),
map(([, event]) => event),
takeUntil(this.destroyed$),
);
}

Expand All @@ -101,25 +101,27 @@ export class NbHoverTriggerStrategy extends NbTriggerStrategyBase {

show$: Observable<Event> = observableFromEvent<Event>(this.host, 'mouseenter')
.pipe(
takeWhile(() => this.isHostInBody()),
filter(() => !this.container()),
delay(100),
takeUntil(observableFromEvent(this.host, 'mouseleave')),
takeUntil(
observableMerge(
observableFromEvent(this.host, 'mouseleave'),
this.destroyed$,
),
),
repeat(),
);

hide$: Observable<Event> = observableFromEvent<Event>(this.host, 'mouseleave')
.pipe(
takeWhile(() => this.isHostInBody()),
switchMap(() => observableFromEvent<Event>(this.document, 'mousemove')
.pipe(
takeWhile(() => this.isHostInBody()),
debounceTime(100),
takeWhile(() => !!this.container()),
filter(event => this.isNotOnHostOrContainer(event),
),
filter(event => this.isNotOnHostOrContainer(event)),
),
),
takeUntil(this.destroyed$),
);
}

Expand All @@ -131,15 +133,20 @@ export class NbHoverTriggerStrategy extends NbTriggerStrategyBase {
export class NbHintTriggerStrategy extends NbTriggerStrategyBase {
show$: Observable<Event> = observableFromEvent<Event>(this.host, 'mouseenter')
.pipe(
takeWhile(() => this.isHostInBody()),
delay(100),
takeUntil(observableFromEvent(this.host, 'mouseleave')),
takeUntil(
observableMerge(
observableFromEvent(this.host, 'mouseleave'),
this.destroyed$,
),
),
// this `delay & takeUntil & repeat` operators combination is a synonym for `conditional debounce`
// meaning that if one event occurs in some time after the initial one we won't react to it
repeat(),
);

hide$: Observable<Event> = observableFromEvent(this.host, 'mouseleave');
hide$: Observable<Event> = observableFromEvent(this.host, 'mouseleave')
.pipe(takeUntil(this.destroyed$));
}


Expand All @@ -152,46 +159,58 @@ export class NbFocusTriggerStrategy extends NbTriggerStrategyBase {

protected focusOut$: Observable<Event> = observableFromEvent<Event>(this.host, 'focusout')
.pipe(
takeWhile(() => this.isHostInBody()),
switchMap(() => observableFromEvent<Event>(this.document, 'focusin')
.pipe(
takeWhile(() => !!this.container()),
filter(event => this.isNotOnHostOrContainer(event)),
),
),
takeUntil(this.destroyed$),
);

protected clickIn$: Observable<Event> = observableFromEvent<Event>(this.host, 'click')
.pipe(
takeWhile(() => this.isHostInBody()),
filter(() => !this.container()),
takeUntil(this.destroyed$),
);

protected clickOut$: Observable<Event> = observableFromEvent<Event>(this.document, 'click')
.pipe(
takeWhile(() => this.isHostInBody()),
filter(() => !!this.container()),
filter(event => this.isNotOnHostOrContainer(event)),
takeUntil(this.destroyed$),
);

protected tabKeyPress$: Observable<Event> = observableFromEvent<Event>(this.document, 'keydown')
.pipe(
takeWhile(() => this.isHostInBody()),
filter((event: KeyboardEvent) => event.keyCode === 9),
filter(() => !!this.container()),
takeUntil(this.destroyed$),
);

show$: Observable<Event> = observableMerge(observableFromEvent<Event>(this.host, 'focusin'), this.clickIn$)
.pipe(
takeWhile(() => this.isHostInBody()),
filter(() => !this.container()),
debounceTime(100),
takeUntil(observableFromEvent(this.host, 'focusout')),
takeUntil(
observableMerge(
observableFromEvent(this.host, 'focusout'),
this.destroyed$,
),
),
repeat(),
);

hide$ = observableMerge(this.focusOut$, this.tabKeyPress$, this.clickOut$)
.pipe(takeWhile(() => this.isHostInBody()));
.pipe(takeUntil(this.destroyed$));
}

/**
* Creates empty show and hide event streams.
* */
export class NbDummyTriggerStrategy extends NbTriggerStrategyBase {
show$: Observable<Event> = EMPTY;
hide$: Observable<Event> = EMPTY;
}

@Injectable()
Expand Down Expand Up @@ -230,10 +249,7 @@ export class NbTriggerStrategyBuilderService {
case NbTrigger.FOCUS:
return new NbFocusTriggerStrategy(this._document, this._host, this._container);
case NbTrigger.NOOP:
return {
show$: EMPTY,
hide$: EMPTY,
};
return new NbDummyTriggerStrategy(this._document, this._host, this._container);
yggg marked this conversation as resolved.
Show resolved Hide resolved
default:
throw new Error('Trigger have to be provided');
}
Expand Down
13 changes: 10 additions & 3 deletions src/framework/theme/components/datepicker/datepicker.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,11 @@ export abstract class NbBasePicker<D, T, P>
* */
protected positionStrategy: NbAdjustableConnectedPositionStrategy;

/**
* Trigger strategy used by overlay
* */
protected triggerStrategy: NbTriggerStrategy;

/**
* HTML input reference to which datepicker connected.
* */
Expand Down Expand Up @@ -232,6 +237,8 @@ export abstract class NbBasePicker<D, T, P>
if (this.ref) {
this.ref.dispose();
}

this.triggerStrategy.destroy();
}

/**
Expand Down Expand Up @@ -314,9 +321,9 @@ export abstract class NbBasePicker<D, T, P>
}

protected subscribeOnTriggers() {
const triggerStrategy = this.createTriggerStrategy();
triggerStrategy.show$.pipe(takeWhile(() => this.alive)).subscribe(() => this.show());
triggerStrategy.hide$.pipe(takeWhile(() => this.alive)).subscribe(() => {
this.triggerStrategy = this.createTriggerStrategy();
this.triggerStrategy.show$.subscribe(() => this.show());
this.triggerStrategy.hide$.subscribe(() => {
this.blur$.next();
this.hide();
});
Expand Down
20 changes: 8 additions & 12 deletions src/framework/theme/components/select/select.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ export class NbSelectComponent<T> implements OnInit, AfterViewInit, AfterContent
this.alive = false;

this.ref.dispose();
this.triggerStrategy.destroy();
}

show() {
Expand Down Expand Up @@ -482,18 +483,13 @@ export class NbSelectComponent<T> implements OnInit, AfterViewInit, AfterContent
}

protected subscribeOnTriggers() {
this.triggerStrategy.show$
.pipe(takeWhile(() => this.alive))
.subscribe(() => this.show());

this.triggerStrategy.hide$
.pipe(takeWhile(() => this.alive))
.subscribe(($event: Event) => {
this.hide();
if (!this.isClickedWithinComponent($event)) {
this.onTouched();
}
});
this.triggerStrategy.show$.subscribe(() => this.show());
this.triggerStrategy.hide$.subscribe(($event: Event) => {
this.hide();
if (!this.isClickedWithinComponent($event)) {
this.onTouched();
}
});
}

protected subscribeOnPositionChange() {
Expand Down