Modal component in Angular 2

Question

I've recently started to check out Angular 2 and TypeScript that comes with it, and I guess I'm kinda into it. However I'm not too sure about the whole "TypeScript way" of doing things, specifically the typing of variables and such.

I wrote this simple modal component which displays a modal when the ModalApiService has its open function called from anywhere in the application. There can be multiple modals on the same page that's why I had to also include the passing of a modalId to the modal so I could identify them correctly.

• Can this code be optimized in any way?
• Do I set the types in a correct way? Are there more things I should set a type for?
• Does TypeScript have any features that I'm not taking full advantage of here?

Component:

export class ModalComponent {
 public isOpen: boolean = false;
 constructor(private _modalApi: ModalApiService, private _elementRef: ElementRef) {
 let overlay = <HTMLScriptElement>document.getElementsByClassName('modal-overlay')[0];
 if (overlay) {
 overlay.addEventListener('click', (e: any) => {
 // Close the modal if a click is made on the modal overlay itself, not its children
 if ((e.target.getAttribute('class') || []).indexOf('modal-overlay') !== -1) {
 _modalApi.close(this.modalId);
 }
 });
 }
 _modalApi.isOpen$.subscribe(
 modal => {
 this.isOpen = modal.id === this.modalId ? modal.isOpen : false;
 }
 )
 }
}

Component API service:

@Injectable()
export class ModalApiService {
 private _isOpen = new Subject<boolean>();
 isOpen$ = this._isOpen.asObservable();
 open(id: string) {
 this._isOpen.next({isOpen: true, id: id});
 }
 close(id: string) {
 this._isOpen.next({isOpen: false, id: id});
 }
}

Answer 1

Cool stuff, though instead of declaring _isOpen property as a <boolean> in the service class, you should provide a new interface (as you added modalId). Something like:

interface IModalElementStatus {
 isOpen: boolean;
 id: string;
}

• ferada: Welcome to Code Review and good first answer, enjoy your stay!
• Chrillewoodz: And how would I use it? Every time I try to use an Interface like this I fail miserably.
• Daniel Mylian: What do you mean by failing? In this case just replace a '<boolean>' with a interface name. Or am I missing something?
• Chrillewoodz: Well since it's a Subject idk if your interface would work since _isOpen would be something completely different than just 2 properties. Right?

Answer 2

Other suggestions:

• You should check the HostListener attribute from angular side
• Also I would recommend to check Renderer, I think it's not the best way how you try to access to element properties
• Why are you using e: any? this can be Event

I'm also not even sure how this code works, this.modelId is not defined. Finally you should implement some methods, and for sure define the return value. E.g.:

close(id: string): Observable<IModalElementStatus> {
 this._isOpen.next({isOpen: false, id: id});
 return Observable.of.....
}

I know currently it's not returning anything!

Answer 3

Yes, lots of optimisations can be done here. Have a look at

$uibModal's open function - https://angular-ui.github.io/bootstrap/#/modal

Particularly templateUrl. where you can pass in a url of a modal to use. Generally putting html onto a page which may never be used is bad practice

• Chrillewoodz: I'm using angular 2 not angular 1.