From 15c301d39f94b71b08a6310c88b036f51609d4d9 Mon Sep 17 00:00:00 2001 From: Jonathan Prusik Date: Thu, 24 Oct 2024 10:15:24 -0400 Subject: [PATCH] Do not redirect after saving changes to excluded domains (#11676) --- .../settings/excluded-domains.component.html | 10 ++- .../settings/excluded-domains.component.ts | 81 ++++++++++++------- 2 files changed, 61 insertions(+), 30 deletions(-) diff --git a/apps/browser/src/autofill/popup/settings/excluded-domains.component.html b/apps/browser/src/autofill/popup/settings/excluded-domains.component.html index 1dec438fdd..e3b6bf5f80 100644 --- a/apps/browser/src/autofill/popup/settings/excluded-domains.component.html +++ b/apps/browser/src/autofill/popup/settings/excluded-domains.component.html @@ -11,7 +11,7 @@ accountSwitcherEnabled ? ("excludedDomainsDescAlt" | i18n) : ("excludedDomainsDesc" | i18n) }}

- +

{{ "domainsTitle" | i18n }}

{{ excludedDomainsState?.length || 0 }} @@ -57,7 +57,13 @@ - diff --git a/apps/browser/src/autofill/popup/settings/excluded-domains.component.ts b/apps/browser/src/autofill/popup/settings/excluded-domains.component.ts index 381ba90342..f622312ce0 100644 --- a/apps/browser/src/autofill/popup/settings/excluded-domains.component.ts +++ b/apps/browser/src/autofill/popup/settings/excluded-domains.component.ts @@ -1,13 +1,19 @@ import { CommonModule } from "@angular/common"; -import { QueryList, Component, ElementRef, OnDestroy, OnInit, ViewChildren } from "@angular/core"; +import { + QueryList, + Component, + ElementRef, + OnDestroy, + AfterViewInit, + ViewChildren, +} from "@angular/core"; import { FormsModule } from "@angular/forms"; -import { Router, RouterModule } from "@angular/router"; -import { firstValueFrom, Subject, takeUntil } from "rxjs"; +import { RouterModule } from "@angular/router"; +import { Subject, takeUntil } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service"; import { NeverDomains } from "@bitwarden/common/models/domain/domain-service"; -import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; @@ -29,8 +35,6 @@ import { PopupFooterComponent } from "../../../platform/popup/layout/popup-foote import { PopupHeaderComponent } from "../../../platform/popup/layout/popup-header.component"; import { PopupPageComponent } from "../../../platform/popup/layout/popup-page.component"; -const BroadcasterSubscriptionId = "excludedDomainsState"; - @Component({ selector: "app-excluded-domains", templateUrl: "excluded-domains.component.html", @@ -55,11 +59,12 @@ const BroadcasterSubscriptionId = "excludedDomainsState"; TypographyModule, ], }) -export class ExcludedDomainsComponent implements OnInit, OnDestroy { +export class ExcludedDomainsComponent implements AfterViewInit, OnDestroy { @ViewChildren("uriInput") uriInputElements: QueryList>; accountSwitcherEnabled = false; dataIsPristine = true; + isLoading = false; excludedDomainsState: string[] = []; storedExcludedDomains: string[] = []; // How many fields should be non-editable before editable fields @@ -70,16 +75,27 @@ export class ExcludedDomainsComponent implements OnInit, OnDestroy { constructor( private domainSettingsService: DomainSettingsService, private i18nService: I18nService, - private router: Router, - private broadcasterService: BroadcasterService, private platformUtilsService: PlatformUtilsService, ) { this.accountSwitcherEnabled = enableAccountSwitching(); } - async ngOnInit() { - const neverDomains = await firstValueFrom(this.domainSettingsService.neverDomains$); + async ngAfterViewInit() { + this.domainSettingsService.neverDomains$ + .pipe(takeUntil(this.destroy$)) + .subscribe((neverDomains: NeverDomains) => this.handleStateUpdate(neverDomains)); + this.uriInputElements.changes.pipe(takeUntil(this.destroy$)).subscribe(({ last }) => { + this.focusNewUriInput(last); + }); + } + + ngOnDestroy() { + this.destroy$.next(); + this.destroy$.complete(); + } + + handleStateUpdate(neverDomains: NeverDomains) { if (neverDomains) { this.storedExcludedDomains = Object.keys(neverDomains); } @@ -89,15 +105,8 @@ export class ExcludedDomainsComponent implements OnInit, OnDestroy { // Do not allow the first x (pre-existing) fields to be edited this.fieldsEditThreshold = this.storedExcludedDomains.length; - this.uriInputElements.changes.pipe(takeUntil(this.destroy$)).subscribe(({ last }) => { - this.focusNewUriInput(last); - }); - } - - ngOnDestroy() { - this.broadcasterService.unsubscribe(BroadcasterSubscriptionId); - this.destroy$.next(); - this.destroy$.complete(); + this.dataIsPristine = true; + this.isLoading = false; } focusNewUriInput(elementRef: ElementRef) { @@ -116,7 +125,7 @@ export class ExcludedDomainsComponent implements OnInit, OnDestroy { async removeDomain(i: number) { this.excludedDomainsState.splice(i, 1); - // if a pre-existing field was dropped, lower the edit threshold + // If a pre-existing field was dropped, lower the edit threshold if (i < this.fieldsEditThreshold) { this.fieldsEditThreshold--; } @@ -132,11 +141,11 @@ export class ExcludedDomainsComponent implements OnInit, OnDestroy { async saveChanges() { if (this.dataIsPristine) { - await this.router.navigate(["/notifications"]); - return; } + this.isLoading = true; + const newExcludedDomainsSaveState: NeverDomains = {}; const uniqueExcludedDomains = new Set(this.excludedDomainsState); @@ -151,6 +160,8 @@ export class ExcludedDomainsComponent implements OnInit, OnDestroy { this.i18nService.t("excludedDomainsInvalidDomain", uri), ); + // Don't reset via `handleStateUpdate` to allow existing input value correction + this.isLoading = false; return; } @@ -159,7 +170,23 @@ export class ExcludedDomainsComponent implements OnInit, OnDestroy { } try { - await this.domainSettingsService.setNeverDomains(newExcludedDomainsSaveState); + const existingState = new Set(this.storedExcludedDomains); + const newState = new Set(Object.keys(newExcludedDomainsSaveState)); + const stateIsUnchanged = + existingState.size === newState.size && + new Set([...existingState, ...newState]).size === existingState.size; + + // The subscriber updates don't trigger if `setNeverDomains` sets an equivalent state + if (stateIsUnchanged) { + // Reset UI state directly + const constructedNeverDomainsState = this.storedExcludedDomains.reduce( + (neverDomains, uri) => ({ ...neverDomains, [uri]: null }), + {}, + ); + this.handleStateUpdate(constructedNeverDomainsState); + } else { + await this.domainSettingsService.setNeverDomains(newExcludedDomainsSaveState); + } this.platformUtilsService.showToast( "success", @@ -169,11 +196,9 @@ export class ExcludedDomainsComponent implements OnInit, OnDestroy { } catch { this.platformUtilsService.showToast("error", null, this.i18nService.t("unexpectedError")); - // Do not navigate on error - return; + // Don't reset via `handleStateUpdate` to preserve input values + this.isLoading = false; } - - await this.router.navigate(["/notifications"]); } trackByFunction(index: number, _: string) {