From 9af61e84309c23ffbfd7562435a5fadd86cdf20c Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Mon, 19 Mar 2018 18:00:31 +0100 Subject: [PATCH] Don't forget to clean up subscriptions --- .../account-videos.component.ts | 13 ++++++--- .../app/shared/video/abstract-video-list.ts | 29 ++++++++++++++----- .../video/infinite-scroller.directive.ts | 22 ++++++++++---- .../video-list/video-local.component.ts | 8 +++-- .../video-recently-added.component.ts | 8 +++-- .../video-list/video-search.component.ts | 6 ++-- .../video-list/video-trending.component.ts | 8 +++-- 7 files changed, 68 insertions(+), 26 deletions(-) diff --git a/client/src/app/account/account-videos/account-videos.component.ts b/client/src/app/account/account-videos/account-videos.component.ts index a286bad1c..cd0f8e752 100644 --- a/client/src/app/account/account-videos/account-videos.component.ts +++ b/client/src/app/account/account-videos/account-videos.component.ts @@ -1,4 +1,4 @@ -import { Component, OnInit } from '@angular/core' +import { Component, OnInit, OnDestroy } from '@angular/core' import { ActivatedRoute, Router } from '@angular/router' import { immutableAssign } from '@app/shared/misc/utils' import { ComponentPagination } from '@app/shared/rest/component-pagination.model' @@ -17,18 +17,19 @@ import { VideoService } from '../../shared/video/video.service' templateUrl: './account-videos.component.html', styleUrls: [ './account-videos.component.scss' ] }) -export class AccountVideosComponent extends AbstractVideoList implements OnInit { +export class AccountVideosComponent extends AbstractVideoList implements OnInit, OnDestroy { titlePage = 'My videos' currentRoute = '/account/videos' checkedVideos: { [ id: number ]: boolean } = {} - videoHeight = 155 - videoWidth = -1 pagination: ComponentPagination = { currentPage: 1, itemsPerPage: 10, totalItems: null } + protected baseVideoWidth = -1 + protected baseVideoHeight = 155 + constructor (protected router: Router, protected route: ActivatedRoute, protected authService: AuthService, @@ -42,6 +43,10 @@ export class AccountVideosComponent extends AbstractVideoList implements OnInit super.ngOnInit() } + ngOnDestroy () { + super.ngOnDestroy() + } + abortSelectionMode () { this.checkedVideos = {} } diff --git a/client/src/app/shared/video/abstract-video-list.ts b/client/src/app/shared/video/abstract-video-list.ts index 570aaae9d..4a220c93d 100644 --- a/client/src/app/shared/video/abstract-video-list.ts +++ b/client/src/app/shared/video/abstract-video-list.ts @@ -1,4 +1,4 @@ -import { ElementRef, OnInit, ViewChild } from '@angular/core' +import { ElementRef, OnDestroy, OnInit, ViewChild } from '@angular/core' import { ActivatedRoute, Router } from '@angular/router' import { isInMobileView } from '@app/shared/misc/utils' import { InfiniteScrollerDirective } from '@app/shared/video/infinite-scroller.directive' @@ -6,12 +6,13 @@ import { NotificationsService } from 'angular2-notifications' import 'rxjs/add/operator/debounceTime' import { Observable } from 'rxjs/Observable' import { fromEvent } from 'rxjs/observable/fromEvent' +import { Subscription } from 'rxjs/Subscription' import { AuthService } from '../../core/auth' import { ComponentPagination } from '../rest/component-pagination.model' import { SortField } from './sort-field.type' import { Video } from './video.model' -export abstract class AbstractVideoList implements OnInit { +export abstract class AbstractVideoList implements OnInit, OnDestroy { private static LINES_PER_PAGE = 3 @ViewChild('videoElement') videosElement: ElementRef @@ -30,6 +31,9 @@ export abstract class AbstractVideoList implements OnInit { videoHeight: number videoPages: Video[][] = [] + protected baseVideoWidth = 215 + protected baseVideoHeight = 230 + protected abstract notificationsService: NotificationsService protected abstract authService: AuthService protected abstract router: Router @@ -40,6 +44,8 @@ export abstract class AbstractVideoList implements OnInit { protected loadedPages: { [ id: number ]: Video[] } = {} protected otherRouteParams = {} + private resizeSubscription: Subscription + abstract getVideosObservable (page: number): Observable<{ videos: Video[], totalVideos: number}> get user () { @@ -51,7 +57,7 @@ export abstract class AbstractVideoList implements OnInit { const routeParams = this.route.snapshot.params this.loadRouteParams(routeParams) - fromEvent(window, 'resize') + this.resizeSubscription = fromEvent(window, 'resize') .debounceTime(500) .subscribe(() => this.calcPageSizes()) @@ -59,6 +65,10 @@ export abstract class AbstractVideoList implements OnInit { if (this.loadOnInit === true) this.loadMoreVideos(this.pagination.currentPage) } + ngOnDestroy () { + if (this.resizeSubscription) this.resizeSubscription.unsubscribe() + } + onNearOfTop () { this.previousPage() } @@ -168,15 +178,15 @@ export abstract class AbstractVideoList implements OnInit { } private calcPageSizes () { - if (isInMobileView()) { + if (isInMobileView() || this.baseVideoWidth === -1) { this.pagination.itemsPerPage = 5 // Video takes all the width this.videoWidth = -1 this.pageHeight = this.pagination.itemsPerPage * this.videoHeight } else { - this.videoWidth = 215 - this.videoHeight = 230 + this.videoWidth = this.baseVideoWidth + this.videoHeight = this.baseVideoHeight const videosWidth = this.videosElement.nativeElement.offsetWidth this.pagination.itemsPerPage = Math.floor(videosWidth / this.videoWidth) * AbstractVideoList.LINES_PER_PAGE @@ -194,7 +204,12 @@ export abstract class AbstractVideoList implements OnInit { i++ } - this.buildVideoPages() + // Re fetch the last page + if (videos.length !== 0) { + this.loadMoreVideos(i) + } else { + this.buildVideoPages() + } console.log('Rebuilt pages with %s elements per page.', this.pagination.itemsPerPage) } diff --git a/client/src/app/shared/video/infinite-scroller.directive.ts b/client/src/app/shared/video/infinite-scroller.directive.ts index e0f9f4f83..e2730423f 100644 --- a/client/src/app/shared/video/infinite-scroller.directive.ts +++ b/client/src/app/shared/video/infinite-scroller.directive.ts @@ -1,18 +1,19 @@ -import { Directive, EventEmitter, Input, OnInit, Output } from '@angular/core' +import { Directive, EventEmitter, Input, OnDestroy, OnInit, Output } from '@angular/core' import 'rxjs/add/operator/debounceTime' import 'rxjs/add/operator/distinct' import 'rxjs/add/operator/distinctUntilChanged' import 'rxjs/add/operator/filter' import 'rxjs/add/operator/map' +import 'rxjs/add/operator/share' import 'rxjs/add/operator/startWith' import 'rxjs/add/operator/throttleTime' import { fromEvent } from 'rxjs/observable/fromEvent' -import 'rxjs/add/operator/share' +import { Subscription } from 'rxjs/Subscription' @Directive({ selector: '[myInfiniteScroller]' }) -export class InfiniteScrollerDirective implements OnInit { +export class InfiniteScrollerDirective implements OnInit, OnDestroy { private static PAGE_VIEW_TOP_MARGIN = 500 @Input() containerHeight: number @@ -27,6 +28,9 @@ export class InfiniteScrollerDirective implements OnInit { private decimalLimit = 0 private lastCurrentBottom = -1 private lastCurrentTop = 0 + private scrollDownSub: Subscription + private scrollUpSub: Subscription + private pageChangeSub: Subscription constructor () { this.decimalLimit = this.percentLimit / 100 @@ -36,6 +40,12 @@ export class InfiniteScrollerDirective implements OnInit { if (this.autoLoading === true) return this.initialize() } + ngOnDestroy () { + if (this.scrollDownSub) this.scrollDownSub.unsubscribe() + if (this.scrollUpSub) this.scrollUpSub.unsubscribe() + if (this.pageChangeSub) this.pageChangeSub.unsubscribe() + } + initialize () { // Emit the last value const throttleOptions = { leading: true, trailing: true } @@ -48,7 +58,7 @@ export class InfiniteScrollerDirective implements OnInit { .share() // Scroll Down - scrollObservable + this.scrollDownSub = scrollObservable // Check we scroll down .filter(({ current }) => { const res = this.lastCurrentBottom < current @@ -60,7 +70,7 @@ export class InfiniteScrollerDirective implements OnInit { .subscribe(() => this.nearOfBottom.emit()) // Scroll up - scrollObservable + this.scrollUpSub = scrollObservable // Check we scroll up .filter(({ current }) => { const res = this.lastCurrentTop > current @@ -74,7 +84,7 @@ export class InfiniteScrollerDirective implements OnInit { .subscribe(() => this.nearOfTop.emit()) // Page change - scrollObservable + this.pageChangeSub = scrollObservable .distinct() .map(({ current }) => this.calculateCurrentPage(current)) .distinctUntilChanged() diff --git a/client/src/app/videos/video-list/video-local.component.ts b/client/src/app/videos/video-list/video-local.component.ts index 8cac2c12c..8f9d50a7b 100644 --- a/client/src/app/videos/video-list/video-local.component.ts +++ b/client/src/app/videos/video-list/video-local.component.ts @@ -1,4 +1,4 @@ -import { Component, OnInit } from '@angular/core' +import { Component, OnDestroy, OnInit } from '@angular/core' import { ActivatedRoute, Router } from '@angular/router' import { immutableAssign } from '@app/shared/misc/utils' import { NotificationsService } from 'angular2-notifications' @@ -12,7 +12,7 @@ import { VideoService } from '../../shared/video/video.service' styleUrls: [ '../../shared/video/abstract-video-list.scss' ], templateUrl: '../../shared/video/abstract-video-list.html' }) -export class VideoLocalComponent extends AbstractVideoList implements OnInit { +export class VideoLocalComponent extends AbstractVideoList implements OnInit, OnDestroy { titlePage = 'Local videos' currentRoute = '/videos/local' sort = '-createdAt' as SortField @@ -29,6 +29,10 @@ export class VideoLocalComponent extends AbstractVideoList implements OnInit { super.ngOnInit() } + ngOnDestroy () { + super.ngOnDestroy() + } + getVideosObservable (page: number) { const newPagination = immutableAssign(this.pagination, { currentPage: page }) diff --git a/client/src/app/videos/video-list/video-recently-added.component.ts b/client/src/app/videos/video-list/video-recently-added.component.ts index f150e38da..1cecd14a0 100644 --- a/client/src/app/videos/video-list/video-recently-added.component.ts +++ b/client/src/app/videos/video-list/video-recently-added.component.ts @@ -1,4 +1,4 @@ -import { Component, OnInit } from '@angular/core' +import { Component, OnDestroy, OnInit } from '@angular/core' import { ActivatedRoute, Router } from '@angular/router' import { immutableAssign } from '@app/shared/misc/utils' import { NotificationsService } from 'angular2-notifications' @@ -12,7 +12,7 @@ import { VideoService } from '../../shared/video/video.service' styleUrls: [ '../../shared/video/abstract-video-list.scss' ], templateUrl: '../../shared/video/abstract-video-list.html' }) -export class VideoRecentlyAddedComponent extends AbstractVideoList implements OnInit { +export class VideoRecentlyAddedComponent extends AbstractVideoList implements OnInit, OnDestroy { titlePage = 'Recently added' currentRoute = '/videos/recently-added' sort: SortField = '-createdAt' @@ -29,6 +29,10 @@ export class VideoRecentlyAddedComponent extends AbstractVideoList implements On super.ngOnInit() } + ngOnDestroy () { + super.ngOnDestroy() + } + getVideosObservable (page: number) { const newPagination = immutableAssign(this.pagination, { currentPage: page }) diff --git a/client/src/app/videos/video-list/video-search.component.ts b/client/src/app/videos/video-list/video-search.component.ts index 241b97bc7..788797a4c 100644 --- a/client/src/app/videos/video-list/video-search.component.ts +++ b/client/src/app/videos/video-list/video-search.component.ts @@ -47,9 +47,9 @@ export class VideoSearchComponent extends AbstractVideoList implements OnInit, O } ngOnDestroy () { - if (this.subActivatedRoute) { - this.subActivatedRoute.unsubscribe() - } + super.ngOnDestroy() + + if (this.subActivatedRoute) this.subActivatedRoute.unsubscribe() } getVideosObservable (page: number) { diff --git a/client/src/app/videos/video-list/video-trending.component.ts b/client/src/app/videos/video-list/video-trending.component.ts index a42457273..1dd1ad23b 100644 --- a/client/src/app/videos/video-list/video-trending.component.ts +++ b/client/src/app/videos/video-list/video-trending.component.ts @@ -1,4 +1,4 @@ -import { Component, OnInit } from '@angular/core' +import { Component, OnDestroy, OnInit } from '@angular/core' import { ActivatedRoute, Router } from '@angular/router' import { immutableAssign } from '@app/shared/misc/utils' import { NotificationsService } from 'angular2-notifications' @@ -12,7 +12,7 @@ import { VideoService } from '../../shared/video/video.service' styleUrls: [ '../../shared/video/abstract-video-list.scss' ], templateUrl: '../../shared/video/abstract-video-list.html' }) -export class VideoTrendingComponent extends AbstractVideoList implements OnInit { +export class VideoTrendingComponent extends AbstractVideoList implements OnInit, OnDestroy { titlePage = 'Trending' currentRoute = '/videos/trending' defaultSort: SortField = '-views' @@ -29,6 +29,10 @@ export class VideoTrendingComponent extends AbstractVideoList implements OnInit super.ngOnInit() } + ngOnDestroy () { + super.ngOnDestroy() + } + getVideosObservable (page: number) { const newPagination = immutableAssign(this.pagination, { currentPage: page }) return this.videoService.getVideos(newPagination, this.sort) -- 2.41.0