Skip to content

[PM-21791] Nudge UI Bug Fixes #15010

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jun 4, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
[title]="'hasItemsVaultNudgeTitle' | i18n"
(onDismiss)="dismissVaultNudgeSpotlight(NudgeType.HasVaultItems)"
>
<ul class="tw-pl-4 tw-text-main" bitTypography="body2">
<ul class="tw-pl-4 tw-text-main tw-mb-0" bitTypography="body2">
<li>{{ "hasItemsVaultNudgeBodyOne" | i18n }}</li>
<li>{{ "hasItemsVaultNudgeBodyTwo" | i18n }}</li>
<li>{{ "hasItemsVaultNudgeBodyThree" | i18n }}</li>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<popup-page>
<popup-header slot="header" [pageTitle]="'vault' | i18n" showBackButton>
<popup-header slot="header" [pageTitle]="'settingsVaultOptions' | i18n" showBackButton>
<ng-container slot="end">
<app-pop-out></app-pop-out>
</ng-container>
Expand All @@ -14,7 +14,17 @@
</bit-item>
<bit-item>
<button type="button" bit-item-content (click)="import()">
{{ "importItems" | i18n }}
<div class="tw-flex tw-items-center tw-justify-center tw-gap-2">
<p>{{ "importItems" | i18n }}</p>
<span
*ngIf="emptyVaultImportBadge$ | async"
bitBadge
variant="notification"
[attr.aria-label]="'nudgeBadgeAria' | i18n"
>
1
</span>
</div>
<i slot="end" class="bwi bwi-popout" aria-hidden="true"></i>
</button>
</bit-item>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import { CommonModule } from "@angular/common";
import { Component, OnInit } from "@angular/core";
import { Component, OnDestroy, OnInit } from "@angular/core";

Check warning on line 2 in apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts#L2

Added line #L2 was not covered by tests
import { Router, RouterModule } from "@angular/router";
import { firstValueFrom, switchMap } from "rxjs";

Check warning on line 4 in apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts#L4

Added line #L4 was not covered by tests

import { JslibModule } from "@bitwarden/angular/jslib.module";
import { NudgesService, NudgeType } from "@bitwarden/angular/vault";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { getUserId } from "@bitwarden/common/auth/services/account.service";

Check warning on line 9 in apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts#L7-L9

Added lines #L7 - L9 were not covered by tests
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction";
import { ItemModule, ToastOptions, ToastService } from "@bitwarden/components";
import { BadgeComponent, ItemModule, ToastOptions, ToastService } from "@bitwarden/components";

Check warning on line 12 in apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts#L12

Added line #L12 was not covered by tests

import { BrowserApi } from "../../../platform/browser/browser-api";
import BrowserPopupUtils from "../../../platform/popup/browser-popup-utils";
Expand All @@ -24,22 +28,38 @@
PopupHeaderComponent,
PopOutComponent,
ItemModule,
BadgeComponent,
],
})
export class VaultSettingsV2Component implements OnInit {
export class VaultSettingsV2Component implements OnInit, OnDestroy {

Check warning on line 34 in apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts#L34

Added line #L34 was not covered by tests
lastSync = "--";

protected emptyVaultImportBadge$ = this.accountService.activeAccount$.pipe(

Check warning on line 37 in apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts#L37

Added line #L37 was not covered by tests
getUserId,
switchMap((userId) =>
this.nudgeService.showNudgeBadge$(NudgeType.EmptyVaultImportNudge, userId),

Check warning on line 40 in apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts#L40

Added line #L40 was not covered by tests
),
);

constructor(
private router: Router,
private syncService: SyncService,
private toastService: ToastService,
private i18nService: I18nService,
private nudgeService: NudgesService,
private accountService: AccountService,

Check warning on line 50 in apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts#L50

Added line #L50 was not covered by tests
) {}

async ngOnInit() {
await this.setLastSync();
}

async ngOnDestroy(): Promise<void> {
// When a user navigates away from the page, dismiss the empty vault import nudge
const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId));
await this.nudgeService.dismissNudge(NudgeType.EmptyVaultImportNudge, userId);

Check warning on line 60 in apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts#L59-L60

Added lines #L59 - L60 were not covered by tests
}

async import() {
await this.router.navigate(["/import"]);
if (await BrowserApi.isPopupOpen()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import { inject, Injectable } from "@angular/core";
import { combineLatest, Observable, of, switchMap } from "rxjs";

// This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
// eslint-disable-next-line no-restricted-imports
import { CollectionService } from "@bitwarden/admin-console/common";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { UserId } from "@bitwarden/common/types/guid";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";

import { DefaultSingleNudgeService } from "../default-single-nudge.service";
import { NudgeStatus, NudgeType } from "../nudges.service";

/**
* Custom Nudge Service Checking Nudge Status For Empty Vault Import
*/
@Injectable({
providedIn: "root",
})
export class EmptyVaultImportNudgeService extends DefaultSingleNudgeService {
cipherService = inject(CipherService);
organizationService = inject(OrganizationService);
collectionService = inject(CollectionService);

Check warning on line 23 in libs/angular/src/vault/services/custom-nudges-services/empty-vault-import-nudge.service.ts

View check run for this annotation

Codecov / codecov/patch

libs/angular/src/vault/services/custom-nudges-services/empty-vault-import-nudge.service.ts#L21-L23

Added lines #L21 - L23 were not covered by tests

nudgeStatus$(nudgeType: NudgeType, userId: UserId): Observable<NudgeStatus> {
return combineLatest([

Check warning on line 26 in libs/angular/src/vault/services/custom-nudges-services/empty-vault-import-nudge.service.ts

View check run for this annotation

Codecov / codecov/patch

libs/angular/src/vault/services/custom-nudges-services/empty-vault-import-nudge.service.ts#L26

Added line #L26 was not covered by tests
this.getNudgeStatus$(nudgeType, userId),
this.cipherService.cipherViews$(userId),
this.organizationService.organizations$(userId),
this.collectionService.decryptedCollections$,
]).pipe(
switchMap(([nudgeStatus, ciphers, orgs, collections]) => {
const filteredCiphers = ciphers?.filter((cipher) => cipher.deletedDate == null);
const vaultHasMoreThanOneItem = (filteredCiphers?.length ?? 0) > 1;
const { hasBadgeDismissed, hasSpotlightDismissed } = nudgeStatus;

Check warning on line 35 in libs/angular/src/vault/services/custom-nudges-services/empty-vault-import-nudge.service.ts

View check run for this annotation

Codecov / codecov/patch

libs/angular/src/vault/services/custom-nudges-services/empty-vault-import-nudge.service.ts#L35

Added line #L35 was not covered by tests

// When the user has no organizations, return the nudge status directly
if ((orgs?.length ?? 0) === 0) {
return hasBadgeDismissed || hasSpotlightDismissed
? of(nudgeStatus)
: of({
hasSpotlightDismissed: vaultHasMoreThanOneItem,
hasBadgeDismissed: vaultHasMoreThanOneItem,
});
}

const orgIds = new Set(orgs.map((org) => org.id));
const canCreateCollections = orgs.some((org) => org.canCreateNewCollections);
const hasManageCollections = collections.some(

Check warning on line 49 in libs/angular/src/vault/services/custom-nudges-services/empty-vault-import-nudge.service.ts

View check run for this annotation

Codecov / codecov/patch

libs/angular/src/vault/services/custom-nudges-services/empty-vault-import-nudge.service.ts#L47-L49

Added lines #L47 - L49 were not covered by tests
(c) => c.manage && orgIds.has(c.organizationId),
);

// When the user has dismissed the nudge or spotlight, return the nudge status directly
if (hasBadgeDismissed || hasSpotlightDismissed) {
return of(nudgeStatus);

Check warning on line 55 in libs/angular/src/vault/services/custom-nudges-services/empty-vault-import-nudge.service.ts

View check run for this annotation

Codecov / codecov/patch

libs/angular/src/vault/services/custom-nudges-services/empty-vault-import-nudge.service.ts#L55

Added line #L55 was not covered by tests
}

// When the user belongs to an organization and cannot create collections or manage collections,
// hide the nudge and spotlight
if (!hasManageCollections && !canCreateCollections) {
return of({

Check warning on line 61 in libs/angular/src/vault/services/custom-nudges-services/empty-vault-import-nudge.service.ts

View check run for this annotation

Codecov / codecov/patch

libs/angular/src/vault/services/custom-nudges-services/empty-vault-import-nudge.service.ts#L61

Added line #L61 was not covered by tests
hasSpotlightDismissed: true,
hasBadgeDismissed: true,
});
}

// Otherwise, return the nudge status based on the vault contents
return of({

Check warning on line 68 in libs/angular/src/vault/services/custom-nudges-services/empty-vault-import-nudge.service.ts

View check run for this annotation

Codecov / codecov/patch

libs/angular/src/vault/services/custom-nudges-services/empty-vault-import-nudge.service.ts#L68

Added line #L68 was not covered by tests
hasSpotlightDismissed: vaultHasMoreThanOneItem,
hasBadgeDismissed: vaultHasMoreThanOneItem,
});
}),
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ export * from "./account-security-nudge.service";
export * from "./has-items-nudge.service";
export * from "./download-bitwarden-nudge.service";
export * from "./empty-vault-nudge.service";
export * from "./empty-vault-import-nudge.service";
export * from "./new-item-nudge.service";
5 changes: 5 additions & 0 deletions libs/angular/src/vault/services/nudges.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
HasItemsNudgeService,
EmptyVaultNudgeService,
DownloadBitwardenNudgeService,
EmptyVaultImportNudgeService,
} from "./custom-nudges-services";
import { DefaultSingleNudgeService } from "./default-single-nudge.service";
import { NudgesService, NudgeType } from "./nudges.service";
Expand Down Expand Up @@ -64,6 +65,10 @@ describe("Vault Nudges Service", () => {
provide: EmptyVaultNudgeService,
useValue: mock<EmptyVaultNudgeService>(),
},
{
provide: EmptyVaultImportNudgeService,
useValue: mock<EmptyVaultImportNudgeService>(),
},
{
provide: ApiService,
useValue: mock<ApiService>(),
Expand Down
3 changes: 3 additions & 0 deletions libs/angular/src/vault/services/nudges.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
DownloadBitwardenNudgeService,
NewItemNudgeService,
AccountSecurityNudgeService,
EmptyVaultImportNudgeService,
} from "./custom-nudges-services";
import { DefaultSingleNudgeService, SingleNudgeService } from "./default-single-nudge.service";

Expand All @@ -31,6 +32,7 @@ export enum NudgeType {
* Add future nudges here
*/
EmptyVaultNudge = "empty-vault-nudge",
EmptyVaultImportNudge = "empty-vault-import-nudge",
HasVaultItems = "has-vault-items",
AutofillNudge = "autofill-nudge",
AccountSecurity = "account-security",
Expand Down Expand Up @@ -65,6 +67,7 @@ export class NudgesService {
private customNudgeServices: Partial<Record<NudgeType, SingleNudgeService>> = {
[NudgeType.HasVaultItems]: inject(HasItemsNudgeService),
[NudgeType.EmptyVaultNudge]: inject(EmptyVaultNudgeService),
[NudgeType.EmptyVaultImportNudge]: inject(EmptyVaultImportNudgeService),
[NudgeType.AccountSecurity]: inject(AccountSecurityNudgeService),
[NudgeType.AutofillNudge]: inject(AutofillNudgeService),
[NudgeType.DownloadBitwarden]: inject(DownloadBitwardenNudgeService),
Expand Down
2 changes: 1 addition & 1 deletion libs/components/src/no-items/no-items.component.html
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<div class="tw-mx-auto tw-flex tw-flex-col tw-items-center tw-justify-center tw-pt-6">
<div class="tw-max-w-sm tw-flex tw-flex-col tw-items-center">
<bit-icon [icon]="icon" aria-hidden="true"></bit-icon>
<h3 class="tw-font-semibold tw-text-center">
<h3 class="tw-font-semibold tw-text-center tw-mt-4">
<ng-content select="[slot=title]"></ng-content>
</h3>
<p class="tw-text-center">
Expand Down
Loading