2
\$\begingroup\$

This is the angular service I wrote window.service.ts.

import { Injectable } from '@angular/core';

import { Subject } from 'rxjs/subject';
import { Observable } from 'rxjs/observable';

declare let $: any;

@Injectable()
export class WindowService {

  private _subscriptions: Array<Subscription> = [];

  constructor() {
    $(window).bind('click', this._onWindowClick.bind(this));
  }

  ngOnDestroy() {
    $(window).unbind('click');
  }

  // Handle any click event on the document
  private _onWindowClick(event): void {
    let $target = $(event.target);
    // Check every subscription you to see if the $target element
    // that was clicked is contained inside the selector they subscribed with
    this._subscriptions.forEach((sub: Subscription) => {
      // IF you do find that selector continue - the click was inside this subscribers
      // element ELSE fire off the subject to let the subscriber know a click away from them occurred
      if ($target.closest(sub.selector).length == 0)
        sub.subject.next(event);
    });
  }

  // Sign up for clickAway event - called by components
  public clickAway(selector: string): Observable<Event> {
    let subject = new Subject();
    this._subscriptions.push(new Subscription(selector, subject));
    return subject.asObservable();
  }

}

class Subscription {
  constructor(
    public selector: string,
    public subject: Subject<Event>
  ) { }
}

The problem I was trying to solve with it was that I have custom components that open and close. Because of the strict component scope that Angular enforces I had no way to know if someone had clicked away from an open component.

Think of a native HTML select element. You click it to open it, then you click away from it and expect it to close. I needed that functionality.

So I created this service that takes a selector that should represent the most parent element of the component. When a click on the window occurs the service checks to see if the click occurred inside the element passed during subscription. If it was not on that element, it executes next() so the component can know that a click occurred away from itself and likely close itself.

Example use in a component:

ngOnInit() {
  this._windowService.clickAway('#customSelect')
    .subscribe(() => {
      console.log('clicked away');
      this.state = ViewState.Closed;
    });
}

I'm looking for a review on the code in specific to good Angular and javascript practices. I know using $ is considered a bad practice but I already have it in the project and it makes the check to see if the event occurred an easily understandable inside another element a one liner. Also, best practices on how Angular Services, Component, and Directives should interact and communicate is confusing to me so if there is changes that could make this better, looking for those.

Also of course if anyone sees any red flags, please point out.

To answer your first question: yes it works.

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Have you explore solutions with the HostListener decorator?

https://stackoverflow.com/questions/40107008/hostlistener-documentclick-angular-2-same-component


Subject and Observable imports are capitalized.

import { Subject } from 'rxjs/Subject';
import { Observable } from 'rxjs/Observable';

you can use const instead of let for $target and subject variables since they are not reassigned.

Use triple-equals.

if ($target.closest(sub.selector).length === 0) {
  sub.subject.next(event);
}

You can spot the previous points with the ng lint command.

Lifecycle hooks only work in components.

You can drop the JS convention of prefixing private members with _.

Members of the Subscription class can be marked as readonly. Moreover, Angular is a rxjs-everywhere-framework so Subscription is a confusing name since it's a common type of rxjs.

JQuery is a dependency of your service, maybe it can be treated like every other dependencies, with injection. You can also type the jQuery objects if you install the @types/jquery package.

import { Inject, Injectable, InjectionToken, ValueProvider } from '@angular/core';
import { Subject } from 'rxjs/Subject';
import { Observable } from 'rxjs/Observable';

import * as $ from 'jquery';

export const DOM_HELPER: InjectionToken<JQueryStatic> = new InjectionToken<JQueryStatic>('DomHelper');
// use the provider in your app.module
export const domHelperProvider: ValueProvider = {provide: DOM_HELPER, useValue: $};

class Subscription {
  constructor(public readonly selector: JQuery.Selector,
              public readonly subject: Subject<JQuery.Event>) {
  }
}

@Injectable()
export class WindowService {
  private subscriptions: Array<Subscription> = [];

  constructor(@Inject(DOM_HELPER) private $: JQueryStatic) {
    this.$(window).click((event: JQuery.Event): void => this.onWindowClick(event));
  }

  public clickAway(selector: JQuery.Selector): Observable<JQuery.Event> {
    const subject = new Subject();
    this.subscriptions.push(new Subscription(selector, subject));
    return subject.asObservable();
  }

  private onWindowClick(event: JQuery.Event): void {
    const $target = this.$(event.target);
    this.subscriptions.forEach((sub: Subscription): void => {
      if ($target.closest(sub.selector).length === 0) {
        sub.subject.next(event);
      }
    });
  }
}

Your code works but it's buggy. The subscriptions array will never stop growing and may cause performance issue. You can add an "unsubscribing" method in your service, components will call it in their ngOnDestroy method; but I think it's a bad design, consumers of your service will forget to call the method for sure because they will assume that they only have to unsubscribe from the observable you provide.

You have to get rid of this array. I suggest creating an Observable directly from the click events and then filtering it in clickAway:

import { Inject, Injectable, InjectionToken, ValueProvider } from '@angular/core';
import { Observable } from 'rxjs/Observable';
import 'rxjs/add/observable/fromEvent';
import 'rxjs/add/operator/filter';

import * as $ from 'jquery';

export const DOM_HELPER: InjectionToken<JQueryStatic> = new InjectionToken<JQueryStatic>('DomHelper');
export const domHelperProvider: ValueProvider = {provide: DOM_HELPER, useValue: $};

@Injectable()
export class WindowService {
  private observable: Observable<Event>;

  constructor(@Inject(DOM_HELPER) private $: JQueryStatic) {
    this.observable = Observable.fromEvent(window, 'click');
  }

  public clickAway(selector: JQuery.Selector): Observable<Event> {
    return this.observable
      .filter((event: Event): boolean => this.$(event.target).closest(selector).length === 0);
  }
}

Finally you don't need jQuery to achieve the functionality and you can use Angular and native types in your API:

import { ElementRef, Injectable } from '@angular/core';
import { Observable } from 'rxjs/Observable';
import 'rxjs/add/observable/fromEvent';
import 'rxjs/add/operator/filter';

@Injectable()
export class WindowService {
  private observable: Observable<Event>;

  constructor() {
    this.observable = Observable.fromEvent(window, 'click');
  }

  public clickAway(el: ElementRef): Observable<Event> {
    return this.observable.filter((event: Event): boolean => !el.nativeElement.contains(event.target));
  }
}

Example of a component using the service:

import { Component, ElementRef, OnDestroy, OnInit, ViewChild } from '@angular/core';
import { WindowService } from '../window.service';
import { Subscription } from 'rxjs/Subscription';

@Component({
  selector: 'app-closable',
  templateUrl: './closable.component.html',
  styleUrls: ['./closable.component.css']
})
export class ClosableComponent implements OnInit, OnDestroy {

  @ViewChild('outside')
  private outside: ElementRef;

  private subscription: Subscription;

  constructor(private window: WindowService) {
  }

  ngOnInit() {
    this.subscription = this.window.clickAway(this.outside).subscribe(() => console.log('clicked away!'));
  }

  ngOnDestroy() {
    this.subscription.unsubscribe();
  }
}

and its template:

<div>
  closable works!
  <div #outside>
    <button>click outside of me</button>
  </div>
</div>
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.