ngOnDestroy

[Grayfox12]

Can you add to docs about ngOnDestroy override? Cause if i implement ngOnDestroy method in my modal component, observable.next not working. And listener doesnt know about event from modal window

[kevcjones-archived]

Ah.. yes. Sorry , in fact I did fall for this. Tbh I had a not ' find a way not to use ngDestroy' as it's not great practice as it is.

I do accept PRs should you wish to update the docs yourself before I do.

Many thanks for the feedback

On Mon, 16 Jul 2018, 08:23 Dmitriy, notifications@github.com wrote:

Can you add to docs about ngOnDestroy override? Cause if i implement ngOnDestroy method in my modal component, observable.next not working. And listener doesnt know about event from modal window

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/KevCJones/ngx-simple-modal/issues/45, or mute the thread https://github.com/notifications/unsubscribe-auth/AAqe8Mi8i5OmwERJ-PiXAc8wL0h3CIFvks5uHD97gaJpZM4VQql3 .

[Grayfox12]

Or move this code: if (this.observer) { this.observer.next(this.result); this.observer.complete(); } to close method

[Grayfox12]

ok i'll try PR)))

[kevcjones-archived]

I'll check that, I seem to remember their being a good reason for doing stuff in destroy. When get back to a screen I'll review. Event listeners on document rings a bell and making sure I clean up , even when the app destroys the modal not the user.

On Mon, 16 Jul 2018, 08:26 Dmitriy, notifications@github.com wrote:

Or move this code: if (this.observer) { this.observer.next(this.result); this.observer.complete(); } to close method

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/KevCJones/ngx-simple-modal/issues/45#issuecomment-405166878, or mute the thread https://github.com/notifications/unsubscribe-auth/AAqe8M6Ko9EU3qhRBCWR04ZNgSZyFKNsks5uHEApgaJpZM4VQql3 .

[Grayfox12]

oh yes. And if i override ngondestroy, i have memory leak with unsubscrible observable)))

[kevcjones-archived]

fixed in 1.3.10

[kevcjones-archived]

@Grayfox12

In v1.3.13 I've since removed the code inside OnDestroy so that it can be safely extended without needing to know you should call super.ngOnDestroy.. instead @ the point of creation i wrap any ngOnDestroy and do tear down then while still invoking your ngOnDestroy. Thanks for bringing it to my attention and forcing me to think of a way around it 👍