From 597c500494548dc047c32ce3d9c42fce18ad9476 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 12 Nov 2018 11:19:12 +0100 Subject: [PATCH] Avoid loading indicator closing too early --- .../routing/wp-list/wp-list.component.ts | 13 +++-- .../routing/wp-set/wp-set.component.ts | 1 + .../src/app/components/states/switch-state.ts | 4 +- .../app/components/wp-list/wp-list.service.ts | 53 +++++++++++-------- .../wp-query-select-dropdown.component.ts | 7 +-- 5 files changed, 47 insertions(+), 31 deletions(-) diff --git a/frontend/src/app/components/routing/wp-list/wp-list.component.ts b/frontend/src/app/components/routing/wp-list/wp-list.component.ts index d063d1535c..9bd1fbb0e8 100644 --- a/frontend/src/app/components/routing/wp-list/wp-list.component.ts +++ b/frontend/src/app/components/routing/wp-list/wp-list.component.ts @@ -26,7 +26,7 @@ // See doc/COPYRIGHT.rdoc for more details. // ++ -import {Component} from "@angular/core"; +import {Component, OnDestroy} from "@angular/core"; import {WorkPackagesSetComponent} from "core-components/routing/wp-set/wp-set.component"; import {StateService, TransitionService} from '@uirouter/core'; import {AuthorisationService} from 'core-app/modules/common/model-auth/model-auth.service'; @@ -50,12 +50,13 @@ import {I18nService} from "core-app/modules/common/i18n/i18n.service"; import {WorkPackageStaticQueriesService} from 'core-components/wp-query-select/wp-static-queries.service'; import {WorkPackageTableHighlightingService} from "core-components/wp-fast-table/state/wp-table-highlighting.service"; import {OpTitleService} from "core-components/html/op-title.service"; +import {Observable} from "rxjs"; @Component({ selector: 'wp-list', templateUrl: './wp.list.component.html' }) -export class WorkPackagesListComponent extends WorkPackagesSetComponent { +export class WorkPackagesListComponent extends WorkPackagesSetComponent implements OnDestroy { text = { 'jump_to_pagination': this.I18n.t('js.work_packages.jump_marks.pagination'), 'text_jump_to_pagination': this.I18n.t('js.work_packages.jump_marks.label_pagination'), @@ -65,6 +66,7 @@ export class WorkPackagesListComponent extends WorkPackagesSetComponent { titleEditingEnabled:boolean; selectedTitle?:string; currentQuery:QueryResource; + unRegisterTitleListener:Function; constructor(readonly states:States, readonly tableState:TableState, @@ -113,7 +115,7 @@ export class WorkPackagesListComponent extends WorkPackagesSetComponent { super.ngOnInit(); // Update title on entering this state - this.$transitions.onSuccess({to: 'work-packages.list'}, () => { + this.unRegisterTitleListener = this.$transitions.onSuccess({to: 'work-packages.list'}, () => { if (this.selectedTitle) { this.titleService.setFirstPart(this.selectedTitle); } @@ -128,6 +130,11 @@ export class WorkPackagesListComponent extends WorkPackagesSetComponent { }); } + ngOnDestroy():void { + super.ngOnDestroy(); + this.unRegisterTitleListener(); + } + public setAnchorToNextElement() { // Skip to next when visible, otherwise skip to previous const selectors = '#pagination--next-link, #pagination--prev-link, #pagination-empty-text'; diff --git a/frontend/src/app/components/routing/wp-set/wp-set.component.ts b/frontend/src/app/components/routing/wp-set/wp-set.component.ts index dac78adc5e..3efa2aba2f 100644 --- a/frontend/src/app/components/routing/wp-set/wp-set.component.ts +++ b/frontend/src/app/components/routing/wp-set/wp-set.component.ts @@ -202,6 +202,7 @@ export class WorkPackagesSetComponent implements OnInit, OnDestroy { let newChecksum = this.wpListService.getCurrentQueryProps(params); let newId = params.query_id && parseInt(params.query_id); + console.warn("TESTING %O and %O", params.query_id, params) this.wpListChecksumService .executeIfOutdated(newId, newChecksum, diff --git a/frontend/src/app/components/states/switch-state.ts b/frontend/src/app/components/states/switch-state.ts index 35e318e3a3..09d56b253e 100644 --- a/frontend/src/app/components/states/switch-state.ts +++ b/frontend/src/app/components/states/switch-state.ts @@ -24,10 +24,10 @@ export class SwitchState { this.contextSwitch$.clear(reason); } - public doAndTransition(context:StateName, callback:() => PromiseLike) { + public doAndTransition(context:StateName, callback:() => PromiseLike):PromiseLike { this.reset('Clearing before transitioning to ' + context); const promise = callback(); - promise.then(() => this.transition(context)); + return promise.then(() => this.transition(context)); } public fireOnTransition(cb:State, ...context:StateName[]):State { diff --git a/frontend/src/app/components/wp-list/wp-list.service.ts b/frontend/src/app/components/wp-list/wp-list.service.ts index 627615bae8..0f572ab5d4 100644 --- a/frontend/src/app/components/wp-list/wp-list.service.ts +++ b/frontend/src/app/components/wp-list/wp-list.service.ts @@ -46,7 +46,7 @@ import {NotificationsService} from 'core-app/modules/common/notifications/notifi import {I18nService} from "core-app/modules/common/i18n/i18n.service"; import {BehaviorSubject, from, Observable} from 'rxjs'; import {input} from "reactivestates"; -import {catchError, distinctUntilChanged, map, share, shareReplay, switchMap, take} from "rxjs/operators"; +import {catchError, mergeMap, share, switchMap, take, tap} from "rxjs/operators"; export interface QueryDefinition { queryParams:{ query_id?:number, query_props?:string }; @@ -63,8 +63,25 @@ export class WorkPackagesListService { private queryLoading = this.queryRequests .values$() .pipe( - switchMap((q:QueryDefinition) => this.handleQueryRequest(q.queryParams, q.projectIdentifier)), - shareReplay(1) + // Stream the query request, switchMap will call previous requests to be cancelled + switchMap((q:QueryDefinition) => this.streamQueryRequest(q.queryParams, q.projectIdentifier)), + // Map the observable from the stream to a new one that completes when states are loaded + mergeMap((query:QueryResource) => { + // load the form if needed + this.conditionallyLoadForm(query); + + + // Project the loaded query into the table states and confirm the query is fully loaded + return this.tableState.ready + .doAndTransition('Query loaded', () => { + this.wpStatesInitialization.initialize(query, query.results); + return this.tableState.tableRendering.onQueryUpdated.valuesPromise(); + }) + .then(() => query); + }), + // Share any consecutive requests to the same resource, this is due to switchMap + // diverting observables to the LATEST emitted. + share() ); private queryChanges = new BehaviorSubject(''); @@ -80,35 +97,28 @@ export class WorkPackagesListService { protected states:States, protected tableState:TableState, protected wpTablePagination:WorkPackageTablePaginationService, - protected wpListChecksumService:WorkPackagesListChecksumService, protected wpStatesInitialization:WorkPackageStatesInitializationService, protected wpListInvalidQueryService:WorkPackagesListInvalidQueryService) { } - private handleQueryRequest(queryParams:{ query_id?:number, query_props?:string }, projectIdentifier ?:string):Observable { + /** + * Stream a query request as a HTTP observable. Each request to this method will + * result in a new HTTP request. + * + * @param queryParams + * @param projectIdentifier + */ + private streamQueryRequest(queryParams:{ query_id?:number, query_props?:string }, projectIdentifier ?:string):Observable { const decodedProps = this.getCurrentQueryProps(queryParams); const queryData = this.UrlParamsHelper.buildV3GetQueryFromJsonParams(decodedProps); const stream = this.QueryDm.stream(queryData, queryParams.query_id, projectIdentifier); return stream.pipe( - map((query:QueryResource) => { - - // Project the loaded query into the table states and confirm the query is fully loaded - this.tableState.ready.doAndTransition('Query loaded', () => { - this.wpStatesInitialization.initialize(query, query.results); - return this.tableState.tableRendering.onQueryUpdated.valuesPromise(); - }); - - // load the form if needed - this.conditionallyLoadForm(query); - - return query; - }), catchError((error) => { // Load a default query const queryProps = this.UrlParamsHelper.buildV3GetQueryFromJsonParams(decodedProps); return from(this.handleQueryLoadingError(error, queryProps, queryParams.query_id, projectIdentifier)); - }) + }), ); } @@ -123,6 +133,7 @@ export class WorkPackagesListService { return this .queryLoading .pipe( + tap((val:any) => console.error("WTF %O", val)), take(1) ); } @@ -202,8 +213,8 @@ export class WorkPackagesListService { * Load the query from the given state params */ public loadCurrentQueryFromParams(projectIdentifier?:string) { - this.wpListChecksumService.clear(); - return this.fromQueryParams(this.$state.params as any, projectIdentifier) + return this + .fromQueryParams(this.$state.params as any, projectIdentifier) .toPromise(); } diff --git a/frontend/src/app/components/wp-query-select/wp-query-select-dropdown.component.ts b/frontend/src/app/components/wp-query-select/wp-query-select-dropdown.component.ts index 2722cabc6c..30e89d8925 100644 --- a/frontend/src/app/components/wp-query-select/wp-query-select-dropdown.component.ts +++ b/frontend/src/app/components/wp-query-select/wp-query-select-dropdown.component.ts @@ -437,12 +437,9 @@ export class WorkPackageQuerySelectDropdownComponent implements OnInit, OnDestro const isSameItem = params.query_id && params.query_id === currentId.toString(); - // Ensure we're loading the query - if (isStaticItem || isSameItem) { - this.wpListChecksumService.clear(); - } - + // Ensure we're reloading the query if (isSameItem) { + this.wpListChecksumService.clear(); opts.reload = true; }