[PM-9455] Back Navigation in extension (#10496)

* refactor open-attachments component to push attachments route before opening the popout

* use popupRouterCache for navigating the user back

* use the PopupRouterCache for navigating the user after an attachment
This commit is contained in:
Nick Krantz 2024-08-20 09:30:11 -05:00 committed by GitHub
parent 59aa7ebc57
commit 8f07c78cc5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 29 additions and 80 deletions

View File

@ -1,4 +1,4 @@
import { CommonModule, Location } from "@angular/common"; import { CommonModule } from "@angular/common";
import { Component, OnInit } from "@angular/core"; import { Component, OnInit } from "@angular/core";
import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
import { FormsModule } from "@angular/forms"; import { FormsModule } from "@angular/forms";
@ -27,6 +27,7 @@ import { PopOutComponent } from "../../../../../platform/popup/components/pop-ou
import { PopupFooterComponent } from "../../../../../platform/popup/layout/popup-footer.component"; import { PopupFooterComponent } from "../../../../../platform/popup/layout/popup-footer.component";
import { PopupHeaderComponent } from "../../../../../platform/popup/layout/popup-header.component"; import { PopupHeaderComponent } from "../../../../../platform/popup/layout/popup-header.component";
import { PopupPageComponent } from "../../../../../platform/popup/layout/popup-page.component"; import { PopupPageComponent } from "../../../../../platform/popup/layout/popup-page.component";
import { PopupRouterCacheService } from "../../../../../platform/popup/view-cache/popup-router-cache.service";
import { PopupCloseWarningService } from "../../../../../popup/services/popup-close-warning.service"; import { PopupCloseWarningService } from "../../../../../popup/services/popup-close-warning.service";
import { BrowserCipherFormGenerationService } from "../../../services/browser-cipher-form-generation.service"; import { BrowserCipherFormGenerationService } from "../../../services/browser-cipher-form-generation.service";
import { BrowserTotpCaptureService } from "../../../services/browser-totp-capture.service"; import { BrowserTotpCaptureService } from "../../../services/browser-totp-capture.service";
@ -150,11 +151,11 @@ export class AddEditV2Component implements OnInit {
constructor( constructor(
private route: ActivatedRoute, private route: ActivatedRoute,
private location: Location, private popupRouterCacheService: PopupRouterCacheService,
private i18nService: I18nService, private i18nService: I18nService,
private addEditFormConfigService: CipherFormConfigService, private addEditFormConfigService: CipherFormConfigService,
private router: Router,
private popupCloseWarningService: PopupCloseWarningService, private popupCloseWarningService: PopupCloseWarningService,
private router: Router,
) { ) {
this.subscribeToParams(); this.subscribeToParams();
} }
@ -200,13 +201,7 @@ export class AddEditV2Component implements OnInit {
return; return;
} }
if (history.length === 1) { await this.popupRouterCacheService.back();
await this.router.navigate(["/view-cipher"], {
queryParams: { cipherId: this.originalCipherId },
});
} else {
this.location.back();
}
} }
async onCipherSaved(cipher: CipherView) { async onCipherSaved(cipher: CipherView) {

View File

@ -1,10 +1,5 @@
<popup-page> <popup-page>
<popup-header <popup-header slot="header" [pageTitle]="'attachments' | i18n" showBackButton>
slot="header"
[pageTitle]="'attachments' | i18n"
showBackButton
[backAction]="handleBackButton.bind(this)"
>
<app-pop-out slot="end" /> <app-pop-out slot="end" />
</popup-header> </popup-header>
@ -12,7 +7,7 @@
*ngIf="cipherId" *ngIf="cipherId"
[cipherId]="cipherId" [cipherId]="cipherId"
[submitBtn]="submitButton" [submitBtn]="submitButton"
(onUploadSuccess)="navigateToEditScreen()" (onUploadSuccess)="navigateBack()"
></app-cipher-attachments> ></app-cipher-attachments>
<popup-footer slot="footer"> <popup-footer slot="footer">

View File

@ -10,12 +10,12 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic
import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CipherType } from "@bitwarden/common/vault/enums";
import { ButtonComponent } from "@bitwarden/components"; import { ButtonComponent } from "@bitwarden/components";
import { CipherAttachmentsComponent } from "@bitwarden/vault"; import { CipherAttachmentsComponent } from "@bitwarden/vault";
import { PopupFooterComponent } from "../../../../../platform/popup/layout/popup-footer.component"; import { PopupFooterComponent } from "../../../../../platform/popup/layout/popup-footer.component";
import { PopupHeaderComponent } from "../../../../../platform/popup/layout/popup-header.component"; import { PopupHeaderComponent } from "../../../../../platform/popup/layout/popup-header.component";
import { PopupRouterCacheService } from "../../../../../platform/popup/view-cache/popup-router-cache.service";
import { AttachmentsV2Component } from "./attachments-v2.component"; import { AttachmentsV2Component } from "./attachments-v2.component";
@ -44,16 +44,10 @@ describe("AttachmentsV2Component", () => {
const queryParams = new BehaviorSubject<{ cipherId: string }>({ cipherId: "5555-444-3333" }); const queryParams = new BehaviorSubject<{ cipherId: string }>({ cipherId: "5555-444-3333" });
let cipherAttachment: CipherAttachmentsComponent; let cipherAttachment: CipherAttachmentsComponent;
const navigate = jest.fn(); const navigate = jest.fn();
const back = jest.fn().mockResolvedValue(undefined);
const cipherDomain = {
type: CipherType.Login,
name: "Test Login",
};
const cipherServiceGet = jest.fn().mockResolvedValue(cipherDomain);
beforeEach(async () => { beforeEach(async () => {
cipherServiceGet.mockClear(); back.mockClear();
navigate.mockClear(); navigate.mockClear();
await TestBed.configureTestingModule({ await TestBed.configureTestingModule({
@ -62,6 +56,8 @@ describe("AttachmentsV2Component", () => {
{ provide: LogService, useValue: mock<LogService>() }, { provide: LogService, useValue: mock<LogService>() },
{ provide: ConfigService, useValue: mock<ConfigService>() }, { provide: ConfigService, useValue: mock<ConfigService>() },
{ provide: PlatformUtilsService, useValue: mock<PlatformUtilsService>() }, { provide: PlatformUtilsService, useValue: mock<PlatformUtilsService>() },
{ provide: CipherService, useValue: mock<CipherService>() },
{ provide: PopupRouterCacheService, useValue: { back } },
{ provide: I18nService, useValue: { t: (key: string) => key } }, { provide: I18nService, useValue: { t: (key: string) => key } },
{ provide: Router, useValue: { navigate } }, { provide: Router, useValue: { navigate } },
{ {
@ -70,12 +66,6 @@ describe("AttachmentsV2Component", () => {
queryParams, queryParams,
}, },
}, },
{
provide: CipherService,
useValue: {
get: cipherServiceGet,
},
},
], ],
}) })
.overrideComponent(AttachmentsV2Component, { .overrideComponent(AttachmentsV2Component, {
@ -115,9 +105,6 @@ describe("AttachmentsV2Component", () => {
tick(); tick();
expect(navigate).toHaveBeenCalledWith(["/edit-cipher"], { expect(back).toHaveBeenCalledTimes(1);
queryParams: { cipherId: "5555-444-3333", type: CipherType.Login },
replaceUrl: true,
});
})); }));
}); });

View File

@ -1,12 +1,11 @@
import { CommonModule, Location } from "@angular/common"; import { CommonModule } from "@angular/common";
import { Component } from "@angular/core"; import { Component } from "@angular/core";
import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
import { ActivatedRoute, Router } from "@angular/router"; import { ActivatedRoute } from "@angular/router";
import { first } from "rxjs"; import { first } from "rxjs";
import { JslibModule } from "@bitwarden/angular/jslib.module"; import { JslibModule } from "@bitwarden/angular/jslib.module";
import { CipherId } from "@bitwarden/common/types/guid"; import { CipherId } from "@bitwarden/common/types/guid";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { ButtonModule } from "@bitwarden/components"; import { ButtonModule } from "@bitwarden/components";
import { CipherAttachmentsComponent } from "@bitwarden/vault"; import { CipherAttachmentsComponent } from "@bitwarden/vault";
@ -14,6 +13,7 @@ import { PopOutComponent } from "../../../../../platform/popup/components/pop-ou
import { PopupFooterComponent } from "../../../../../platform/popup/layout/popup-footer.component"; import { PopupFooterComponent } from "../../../../../platform/popup/layout/popup-footer.component";
import { PopupHeaderComponent } from "../../../../../platform/popup/layout/popup-header.component"; import { PopupHeaderComponent } from "../../../../../platform/popup/layout/popup-header.component";
import { PopupPageComponent } from "../../../../../platform/popup/layout/popup-page.component"; import { PopupPageComponent } from "../../../../../platform/popup/layout/popup-page.component";
import { PopupRouterCacheService } from "../../../../../platform/popup/view-cache/popup-router-cache.service";
@Component({ @Component({
standalone: true, standalone: true,
@ -38,9 +38,7 @@ export class AttachmentsV2Component {
cipherId: CipherId; cipherId: CipherId;
constructor( constructor(
private router: Router, private popupRouterCacheService: PopupRouterCacheService,
private cipherService: CipherService,
private location: Location,
route: ActivatedRoute, route: ActivatedRoute,
) { ) {
route.queryParams.pipe(takeUntilDestroyed(), first()).subscribe(({ cipherId }) => { route.queryParams.pipe(takeUntilDestroyed(), first()).subscribe(({ cipherId }) => {
@ -48,30 +46,8 @@ export class AttachmentsV2Component {
}); });
} }
/**
* Navigates to previous view or edit-cipher path
* depending on the history length.
*
* This can happen when history is lost due to the extension being
* forced into a popout window.
*/
async handleBackButton() {
if (history.length === 1) {
await this.navigateToEditScreen();
} else {
this.location.back();
}
}
/** Navigate the user back to the edit screen after uploading an attachment */ /** Navigate the user back to the edit screen after uploading an attachment */
async navigateToEditScreen() { async navigateBack() {
const cipherDomain = await this.cipherService.get(this.cipherId); await this.popupRouterCacheService.back();
void this.router.navigate(["/edit-cipher"], {
queryParams: { cipherId: this.cipherId, type: cipherDomain.type },
// "replaceUrl" so the /attachments route is not in the history, thus when a back button
// is clicked, the user is taken to the view screen instead of the attachments screen
replaceUrl: true,
});
} }
} }

View File

@ -102,11 +102,10 @@ describe("OpenAttachmentsComponent", () => {
await component.openAttachments(); await component.openAttachments();
expect(router.navigate).not.toHaveBeenCalled(); expect(router.navigate).toHaveBeenCalledWith(["/attachments"], {
expect(openCurrentPagePopout).toHaveBeenCalledWith( queryParams: { cipherId: "5555-444-3333" },
window, });
"http:/localhost//attachments?cipherId=5555-444-3333", expect(openCurrentPagePopout).toHaveBeenCalledWith(window);
);
}); });
it("opens attachments in same window", async () => { it("opens attachments in same window", async () => {

View File

@ -94,16 +94,13 @@ export class OpenAttachmentsComponent implements OnInit {
return; return;
} }
await this.router.navigate(["/attachments"], { queryParams: { cipherId: this.cipherId } });
// Open the attachments page in a popout
// This is done after the router navigation to ensure that the navigation
// is included in the `PopupRouterCacheService` history
if (this.openAttachmentsInPopout) { if (this.openAttachmentsInPopout) {
const destinationUrl = this.router await BrowserPopupUtils.openCurrentPagePopout(window);
.createUrlTree(["/attachments"], { queryParams: { cipherId: this.cipherId } })
.toString();
const currentBaseUrl = window.location.href.replace(this.router.url, "");
await BrowserPopupUtils.openCurrentPagePopout(window, currentBaseUrl + destinationUrl);
} else {
await this.router.navigate(["/attachments"], { queryParams: { cipherId: this.cipherId } });
} }
} }
} }