fix(overlay): fix click trigger #912
Conversation
| // 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<any[]> = observableFromEvent<Event>(this.document, 'click') |
There was a problem hiding this comment.
Why do you prefer Observable<any[]> to Observable<[boolean, Event]>?
There was a problem hiding this comment.
it just doesn't work as Observable<[boolean, Event]> :)
| } | ||
| readonly hide$: Observable<Event> = this.click$ | ||
| .pipe( | ||
| filter(([shouldShow]) => !shouldShow), |
There was a problem hiding this comment.
Let's unite both filters applied to this stream. I don't think we have to separate them.
There was a problem hiding this comment.
don't you think it looks cleaner?
There was a problem hiding this comment.
I think it calls two filter function instead of one)
| @@ -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), | |||
There was a problem hiding this comment.
You've personally introduced isNotOnHostOrContainer)
| @@ -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 | |||
There was a problem hiding this comment.
...in some time afther the initial one we won't...
| import { ComponentRef } from '@angular/core'; | ||
| import { map } from 'rxjs/operators'; |
There was a problem hiding this comment.
Let's unite two rxjs/operators imports;
Closes #907