From 125167ea1411c9ad0238489fb3499a5eeafa904b Mon Sep 17 00:00:00 2001 From: James Chien Date: Thu, 21 Jan 2021 13:08:04 +0800 Subject: [PATCH 1/6] Remove margin at the bottom of PostCaptureTab Signed-off-by: James Chien --- .../home/post-capture-tab/post-capture-tab.component.scss | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/app/features/home/post-capture-tab/post-capture-tab.component.scss b/src/app/features/home/post-capture-tab/post-capture-tab.component.scss index a54f41e25..cfe1043bc 100644 --- a/src/app/features/home/post-capture-tab/post-capture-tab.component.scss +++ b/src/app/features/home/post-capture-tab/post-capture-tab.component.scss @@ -1,6 +1,4 @@ .tab-content-post { - margin-bottom: 128px; - virtual-scroller { width: 100vw; height: 100vh; From 7c739ce7dbfeed2ba3e20c3fac8349e7b3cce2e0 Mon Sep 17 00:00:00 2001 From: James Chien Date: Thu, 21 Jan 2021 15:25:39 +0800 Subject: [PATCH 2/6] Add sharable_copy download fallback; add ShareService Signed-off-by: James Chien --- .../post-capture-card.component.ts | 26 ++------ .../dia-backend-asset-repository.service.ts | 19 ++++++ .../services/share/share.service.spec.ts | 16 +++++ .../shared/services/share/share.service.ts | 66 +++++++++++++++++++ 4 files changed, 105 insertions(+), 22 deletions(-) create mode 100644 src/app/shared/services/share/share.service.spec.ts create mode 100644 src/app/shared/services/share/share.service.ts diff --git a/src/app/shared/core/post-capture-card/post-capture-card.component.ts b/src/app/shared/core/post-capture-card/post-capture-card.component.ts index d428e1e66..d18c07303 100644 --- a/src/app/shared/core/post-capture-card/post-capture-card.component.ts +++ b/src/app/shared/core/post-capture-card/post-capture-card.component.ts @@ -3,15 +3,14 @@ import { MatBottomSheet } from '@angular/material/bottom-sheet'; import { Plugins } from '@capacitor/core'; import { TranslocoService } from '@ngneat/transloco'; import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy'; -import mergeImages from 'merge-images'; import { BehaviorSubject } from 'rxjs'; import { concatMap, first, map, tap } from 'rxjs/operators'; import { DiaBackendAsset, DiaBackendAssetRepository, } from '../../../shared/services/dia-backend/asset/dia-backend-asset-repository.service'; -import { ImageStore } from '../../../shared/services/image-store/image-store.service'; import { isNonNullable } from '../../../utils/rx-operators/rx-operators'; +import { ShareService } from '../../services/share/share.service'; import { Option, OptionsMenuComponent, @@ -46,8 +45,8 @@ export class PostCaptureCardComponent implements OnInit { constructor( private readonly diaBackendAssetRepository: DiaBackendAssetRepository, private readonly translocoService: TranslocoService, - private readonly imageStore: ImageStore, - private readonly bottomSheet: MatBottomSheet + private readonly bottomSheet: MatBottomSheet, + private readonly shareService: ShareService ) {} ngOnInit() { @@ -85,24 +84,7 @@ export class PostCaptureCardComponent implements OnInit { return this.postCapture$ .pipe( first(), - concatMap(postCapture => - mergeImages( - [postCapture.sharable_copy, '/assets/image/new-year-frame.png'], - // @ts-ignore - { format: 'image/jpeg', crossOrigin: 'Anonymous' } - ) - ), - concatMap(async watermarkedUrl => { - const base64 = watermarkedUrl.split(',')[1]; - return this.imageStore.write(base64, 'image/jpeg'); - }), - concatMap(index => this.imageStore.getUri(index)), - concatMap(watermarkedUri => - Share.share({ - text: '#CaptureApp #OnlyTruePhotos', - url: watermarkedUri, - }) - ), + concatMap(postCapture => this.shareService.share(postCapture)), untilDestroyed(this) ) .subscribe(); diff --git a/src/app/shared/services/dia-backend/asset/dia-backend-asset-repository.service.ts b/src/app/shared/services/dia-backend/asset/dia-backend-asset-repository.service.ts index 281528821..17a11acab 100644 --- a/src/app/shared/services/dia-backend/asset/dia-backend-asset-repository.service.ts +++ b/src/app/shared/services/dia-backend/asset/dia-backend-asset-repository.service.ts @@ -100,6 +100,20 @@ export class DiaBackendAssetRepository { ); } + downloadFile$(id: string, field: AssetDownloadField) { + const formData = new FormData(); + formData.append('field', field); + return defer(() => this.authService.getAuthHeaders()).pipe( + concatMap(headers => + this.httpClient.post( + `${BASE_URL}/api/v2/assets/${id}/download/`, + formData, + { headers, responseType: 'blob' } + ) + ) + ); + } + add$(proof: Proof) { return forkJoin([ defer(() => this.authService.getAuthHeaders()), @@ -167,6 +181,11 @@ interface ListAssetResponse { results: DiaBackendAsset[]; } +export type AssetDownloadField = + | 'asset_file' + | 'asset_file_thumbnail' + | 'sharable_copy'; + type CreateAssetResponse = DiaBackendAsset; // tslint:disable-next-line: no-empty-interface diff --git a/src/app/shared/services/share/share.service.spec.ts b/src/app/shared/services/share/share.service.spec.ts new file mode 100644 index 000000000..984398c94 --- /dev/null +++ b/src/app/shared/services/share/share.service.spec.ts @@ -0,0 +1,16 @@ +import { TestBed } from '@angular/core/testing'; +import { SharedTestingModule } from '../../shared-testing.module'; +import { ShareService } from './share.service'; + +describe('ShareService', () => { + let service: ShareService; + + beforeEach(() => { + TestBed.configureTestingModule({ imports: [SharedTestingModule] }); + service = TestBed.inject(ShareService); + }); + + it('should be created', () => { + expect(service).toBeTruthy(); + }); +}); diff --git a/src/app/shared/services/share/share.service.ts b/src/app/shared/services/share/share.service.ts new file mode 100644 index 000000000..b8989ec0f --- /dev/null +++ b/src/app/shared/services/share/share.service.ts @@ -0,0 +1,66 @@ +import { Injectable } from '@angular/core'; +import { Plugins } from '@capacitor/core'; +import mergeImages from 'merge-images'; +import { concatMap, map } from 'rxjs/operators'; +import { blobToBase64 } from '../../../utils/encoding/encoding'; +import { + DiaBackendAsset, + DiaBackendAssetRepository, +} from '../dia-backend/asset/dia-backend-asset-repository.service'; +import { ImageStore } from '../image-store/image-store.service'; +const { Share } = Plugins; + +@Injectable({ + providedIn: 'root', +}) +export class ShareService { + private readonly defaultSharingFrame = '/assets/image/new-year-frame.png'; + private readonly defaultMimetype = 'image/jpeg'; + private readonly defaultShareText = '#CaptureApp #OnlyTruePhotos'; + + constructor( + private readonly diaBackendAssetRepository: DiaBackendAssetRepository, + private readonly imageStore: ImageStore + ) {} + + async share(asset: DiaBackendAsset) { + const dataUri = await this.getSharableCopy(asset).catch(() => + this.getSharableCopyFallback(asset) + ); + const fileUrl = await this.createFileUrl(dataUri); + return Share.share({ + text: this.defaultShareText, + url: fileUrl, + }); + } + + private async createFileUrl(dataUri: string) { + const base64 = dataUri.split(',')[1]; + const index = await this.imageStore.write(base64, this.defaultMimetype); + return this.imageStore.getUri(index); + } + + private async getSharableCopy(asset: DiaBackendAsset) { + return mergeImages( + [asset.sharable_copy, this.defaultSharingFrame], + // @ts-ignore + { format: this.defaultMimetype, crossOrigin: 'Anonymous' } + ); + } + + // WORKAROUND: Use this fallback as a workaround for S3 CORS issue + private async getSharableCopyFallback(asset: DiaBackendAsset) { + const dataUri = await this.diaBackendAssetRepository + .downloadFile$(asset.id, 'sharable_copy') + .pipe( + concatMap(blobToBase64), + map(imageBase64 => `data:image/jpeg;base64,${imageBase64}`) + ) + .toPromise(); + return mergeImages( + [dataUri, this.defaultSharingFrame], + // @ts-ignore + { format: this.defaultMimetype, crossOrigin: 'Anonymous' } + ); + } +} From 3bdad48d0d9927e50fa0c8f3c1332fae6b9750ab Mon Sep 17 00:00:00 2001 From: James Chien Date: Thu, 21 Jan 2021 16:17:25 +0800 Subject: [PATCH 3/6] Improve backButton click event readability Signed-off-by: James Chien --- .../sending-post-capture/sending-post-capture.page.html | 7 +------ .../sending-post-capture/sending-post-capture.page.ts | 8 ++++++++ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/app/features/home/capture-tab/capture-details/sending-post-capture/sending-post-capture.page.html b/src/app/features/home/capture-tab/capture-details/sending-post-capture/sending-post-capture.page.html index d135a32db..c78225d28 100644 --- a/src/app/features/home/capture-tab/capture-details/sending-post-capture/sending-post-capture.page.html +++ b/src/app/features/home/capture-tab/capture-details/sending-post-capture/sending-post-capture.page.html @@ -1,10 +1,5 @@ - {{ t(isPreview ? 'preview' : 'sendPostCapture') }} diff --git a/src/app/features/home/capture-tab/capture-details/sending-post-capture/sending-post-capture.page.ts b/src/app/features/home/capture-tab/capture-details/sending-post-capture/sending-post-capture.page.ts index 4eb884df3..24584a83a 100644 --- a/src/app/features/home/capture-tab/capture-details/sending-post-capture/sending-post-capture.page.ts +++ b/src/app/features/home/capture-tab/capture-details/sending-post-capture/sending-post-capture.page.ts @@ -80,6 +80,14 @@ export class SendingPostCapturePage { this.isPreview = true; } + onBackButtonClick() { + if (this.isPreview) { + this.isPreview = false; + } else { + this.router.navigate(['..'], { relativeTo: this.route }); + } + } + async send(captionText: string) { const action$ = combineLatest([this.asset$, this.contact$]).pipe( first(), From cfdb388d6dcfdb70b57ab5b3b41f692a3c5a24b2 Mon Sep 17 00:00:00 2001 From: James Chien Date: Thu, 21 Jan 2021 16:17:37 +0800 Subject: [PATCH 4/6] Fix broken back navigation from sending-post-capture page Use first() and untilDestroyed() to make sure the subscription terminates after the navigation. Zombie subscription is horrible :zombie: Signed-off-by: James Chien --- .../capture-details/capture-details.page.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/app/features/home/capture-tab/capture-details/capture-details.page.ts b/src/app/features/home/capture-tab/capture-details/capture-details.page.ts index 90ba7c7f4..ad7bed6d0 100644 --- a/src/app/features/home/capture-tab/capture-details/capture-details.page.ts +++ b/src/app/features/home/capture-tab/capture-details/capture-details.page.ts @@ -8,6 +8,7 @@ import { combineLatest, defer } from 'rxjs'; import { concatMap, concatMapTo, + first, map, shareReplay, switchMap, @@ -88,14 +89,16 @@ export class CaptureDetailsPage { data: { email: '' }, }); const contact$ = dialogRef.afterClosed().pipe(isNonNullable()); - combineLatest([contact$, this.proof$]).subscribe(([contact, proof]) => - this.router.navigate( - ['sending-post-capture', { contact, id: proof.diaBackendAssetId }], - { - relativeTo: this.route, - } - ) - ); + combineLatest([contact$, this.proof$]) + .pipe(first(), untilDestroyed(this)) + .subscribe(([contact, proof]) => + this.router.navigate( + ['sending-post-capture', { contact, id: proof.diaBackendAssetId }], + { + relativeTo: this.route, + } + ) + ); } openOptionsMenu() { From 0f1a67a299ba3ff26cf8671eed136538f19d26d5 Mon Sep 17 00:00:00 2001 From: James Chien Date: Thu, 21 Jan 2021 16:46:27 +0800 Subject: [PATCH 5/6] Refresh PostCapture whenever navigated and focused Refresh PostCapture whenever navigated to /home and focus on PostCaptureTab. When the PostCaptureTab is initially created, it doesn't need the navigation event (use startwith to force the first emitting), so landing on home page and go to PostCaptureTab can work correctly, as well as when the PostCaptureTab component is destoryed and re-created. Signed-off-by: James Chien --- .../post-capture-tab/post-capture-tab.component.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/app/features/home/post-capture-tab/post-capture-tab.component.ts b/src/app/features/home/post-capture-tab/post-capture-tab.component.ts index 4efb71f83..430ac1c00 100644 --- a/src/app/features/home/post-capture-tab/post-capture-tab.component.ts +++ b/src/app/features/home/post-capture-tab/post-capture-tab.component.ts @@ -1,13 +1,15 @@ import { Component, Input, OnInit } from '@angular/core'; +import { NavigationEnd, Router } from '@angular/router'; import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy'; import { IPageInfo } from 'ngx-virtual-scroller'; -import { BehaviorSubject } from 'rxjs'; +import { BehaviorSubject, combineLatest } from 'rxjs'; import { concatMap, distinctUntilChanged, filter, first, map, + startWith, tap, } from 'rxjs/operators'; import { @@ -38,14 +40,19 @@ export class PostCaptureTabComponent implements OnInit { readonly postCaptures$ = this._postCaptures$.asObservable(); readonly pagination$ = this._pagination$.asObservable(); readonly networkConnected$ = this.networkService.connected$; + readonly onDidNavigate$ = this.router.events.pipe( + filter(event => event instanceof NavigationEnd && event?.url === '/home'), + startWith(undefined) + ); constructor( private readonly diaBackendAssetRepository: DiaBackendAssetRepository, - private readonly networkService: NetworkService + private readonly networkService: NetworkService, + private readonly router: Router ) {} ngOnInit() { - this.focus$ + combineLatest([this.focus$, this.onDidNavigate$]) .pipe( filter(focus => !!focus), concatMap(() => this.fetchPostCaptures$()), From 2486cc1b9a9f43e2b9e20e8fa98f8d56646cfb3b Mon Sep 17 00:00:00 2001 From: James Chien Date: Thu, 21 Jan 2021 18:18:03 +0800 Subject: [PATCH 6/6] Fix race condition caused by multiple subscription Use single observable to handle next-page data loading. The first batch of fetching is considered as well, so all relevant race conditions should be fixed now. However, the code is a bit messy. Need refactoring later. Signed-off-by: James Chien --- .../post-capture-tab.component.ts | 82 ++++++++++++++----- 1 file changed, 63 insertions(+), 19 deletions(-) diff --git a/src/app/features/home/post-capture-tab/post-capture-tab.component.ts b/src/app/features/home/post-capture-tab/post-capture-tab.component.ts index 430ac1c00..c009b37ac 100644 --- a/src/app/features/home/post-capture-tab/post-capture-tab.component.ts +++ b/src/app/features/home/post-capture-tab/post-capture-tab.component.ts @@ -2,14 +2,16 @@ import { Component, Input, OnInit } from '@angular/core'; import { NavigationEnd, Router } from '@angular/router'; import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy'; import { IPageInfo } from 'ngx-virtual-scroller'; -import { BehaviorSubject, combineLatest } from 'rxjs'; +import { BehaviorSubject, combineLatest, of, Subject } from 'rxjs'; import { + catchError, concatMap, distinctUntilChanged, filter, first, map, startWith, + switchMap, tap, } from 'rxjs/operators'; import { @@ -18,6 +20,7 @@ import { } from '../../../shared/services/dia-backend/asset/dia-backend-asset-repository.service'; import { Pagination } from '../../../shared/services/dia-backend/pagination/pagination'; import { NetworkService } from '../../../shared/services/network/network.service'; +import { isNonNullable, VOID$ } from '../../../utils/rx-operators/rx-operators'; @UntilDestroy({ checkProperties: true }) @Component({ @@ -36,9 +39,32 @@ export class PostCaptureTabComponent implements OnInit { private readonly _pagination$ = new BehaviorSubject< Pagination | undefined >(undefined); + // tslint:disable-next-line: rxjs-no-explicit-generics + private readonly _loadNextPageEvent$ = new Subject(); + private readonly _isLoadingNextPage$ = new BehaviorSubject(false); readonly focus$ = this._focus$.asObservable().pipe(distinctUntilChanged()); - readonly postCaptures$ = this._postCaptures$.asObservable(); - readonly pagination$ = this._pagination$.asObservable(); + readonly postCaptures$ = this._postCaptures$ + .asObservable() + .pipe(distinctUntilChanged()); + readonly pagination$ = this._pagination$ + .asObservable() + .pipe(distinctUntilChanged()); + readonly loadNextPageEvent$ = this._loadNextPageEvent$.asObservable().pipe( + isNonNullable(), + concatMap(event => + combineLatest([of(event), this.postCaptures$, this.pagination$]).pipe( + first() + ) + ), + filter( + ([event, postCaptures, pagination]) => + event.endIndex === postCaptures.length - 1 && !!pagination?.next + ), + map(([e, p, pagination]) => pagination) + ); + readonly isLoadingNextPage$ = this._isLoadingNextPage$ + .asObservable() + .pipe(distinctUntilChanged()); readonly networkConnected$ = this.networkService.connected$; readonly onDidNavigate$ = this.router.events.pipe( filter(event => event instanceof NavigationEnd && event?.url === '/home'), @@ -54,9 +80,13 @@ export class PostCaptureTabComponent implements OnInit { ngOnInit() { combineLatest([this.focus$, this.onDidNavigate$]) .pipe( - filter(focus => !!focus), - concatMap(() => this.fetchPostCaptures$()), - tap(postCapture => this._postCaptures$.next(postCapture)), + filter(([focus, _]) => !!focus), + switchMap(() => + this.fetchPostCaptures$().pipe( + tap(postCapture => this._postCaptures$.next(postCapture)), + concatMap(() => this.loadNextPageEventHandler$()) + ) + ), untilDestroyed(this) ) .subscribe(); @@ -68,23 +98,16 @@ export class PostCaptureTabComponent implements OnInit { .pipe( tap(pagination => this._pagination$.next(pagination)), map(pagination => pagination.results), - map(assets => assets.filter(asset => !!asset.source_transaction)) + map(assets => assets.filter(asset => !!asset.source_transaction)), + catchError(err => { + console.error(err); + return of([]); + }) ); } loadNextPage(event: IPageInfo) { - if (event.endIndex !== this._postCaptures$.value.length - 1) { - return; - } - this.pagination$ - .pipe( - first(), - filter(pagination => !!pagination?.next), - concatMap(pagination => this.fetchPostCaptures$(pagination?.next)), - tap(newPostCaptures => this.concatPostCaptures(newPostCaptures)), - untilDestroyed(this) - ) - .subscribe(); + this._loadNextPageEvent$.next(event); } // tslint:disable-next-line: prefer-function-over-method @@ -95,4 +118,25 @@ export class PostCaptureTabComponent implements OnInit { private concatPostCaptures(postCaptures: DiaBackendAsset[]) { this._postCaptures$.next(this._postCaptures$.value.concat(postCaptures)); } + + private loadNextPageEventHandler$() { + const loadData$ = this.pagination$.pipe( + first(), + concatMap(pagination => this.fetchPostCaptures$(pagination?.next)), + tap(newPostCaptures => this.concatPostCaptures(newPostCaptures)) + ); + return this.loadNextPageEvent$.pipe( + concatMap(() => this.isLoadingNextPage$.pipe(first())), + filter(isLoadingNextPage => !isLoadingNextPage), + tap(() => this._isLoadingNextPage$.next(true)), + concatMap(() => loadData$), + tap(() => this._isLoadingNextPage$.next(false)), + catchError(err => { + this._isLoadingNextPage$.next(false); + console.error(err); + return VOID$; + }), + untilDestroyed(this) + ); + } }