1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-15 15:53:27 +00:00

[26908] improve empty state design (#16832)

* max init

* add mp4 and organize code better

* fix lint errors

* move empty state logic into risk insights component

* replace getter logic

* sub for org name

* checkForVaultItems fix
- need to use cipherservice instead of report results from data service

* fix all critical issues mentioned by claude bot

* resolve empty state logic bug and memory leaks

- Handle zero-results case in empty state logic
- Add takeUntil cleanup to _setupUserId subscription
- Guard console.warn with isDevMode() check

* use tuple arrays for benefits to prevent XSS risk

Replace pipe-separated strings with typed tuple arrays [string, string][]
for benefits data in empty state component. This eliminates potential XSS
risk from string splitting, provides compile-time type safety, and improves
performance by removing runtime string parsing on every change detection.

* fix(dirt): hide empty states during report generation and fix memory leak

Add isGeneratingReport$ to combineLatest, update empty state conditions
to check !isGenerating, simplify run report logic, and fix memory leak
in route.queryParams subscription.

Addresses Claude bot feedback on PR #16832

* refactor(dirt): use signals and OnPush in empty state card component

Convert @Input() to readonly input signals and add OnPush change
detection strategy. Update template to call signals as functions.
Fixes ESLint compliance issues.

* refactor(dirt): remove unused shouldShowRunReportState variable

The shouldShowRunReportState variable was calculated but never used.
The template already uses @else for the run report state, making this
variable redundant.

* refactor(dirt): consolidate duplicate if statements in empty state logic

Merge 5 separate if/else blocks checking shouldShowImportDataState into
single consolidated block. Move constant benefits assignment outside
conditional. Improves readability and reduces duplication.

* remove unnecessary getOrganizationName wrapper method

* remove duplicate runReport method

Remove runReport arrow function and use generateReport consistently.
Both methods called dataService.triggerReport(), but generateReport
includes an organizationId check for defensive programming.
This commit is contained in:
Alex
2025-10-30 15:16:41 -04:00
committed by GitHub
parent 2b009778e8
commit fdfcee4bc5
11 changed files with 455 additions and 201 deletions

View File

@@ -114,6 +114,9 @@ export class RiskInsightsOrchestratorService {
// --------------------------- Critical Application data ---------------------
criticalReportResults$: Observable<RiskInsightsEnrichedData | null> = of(null);
// --------------------------- Vault Items Check ---------------------
hasVaultItems$: Observable<boolean> = of(false);
// --------------------------- Trigger subjects ---------------------
private _initializeOrganizationTriggerSubject = new Subject<OrganizationId>();
private _fetchReportTriggerSubject = new Subject<void>();
@@ -139,6 +142,7 @@ export class RiskInsightsOrchestratorService {
this._setupCriticalApplicationContext();
this._setupCriticalApplicationReport();
this._setupEnrichedReportData();
this._setupHasVaultItems();
this._setupInitializationPipeline();
this._setupMigrationAndCleanup();
this._setupReportState();
@@ -711,6 +715,34 @@ export class RiskInsightsOrchestratorService {
}
// Setup the pipeline to load critical applications when organization or user changes
/**
* Sets up an observable to check if the organization has any vault items (ciphers).
* This is used to determine which empty state to show in the UI.
*/
private _setupHasVaultItems() {
this.hasVaultItems$ = this.organizationDetails$.pipe(
switchMap((orgDetails) => {
if (!orgDetails?.organizationId) {
return of(false);
}
return from(
this.cipherService.getAllFromApiForOrganization(orgDetails.organizationId),
).pipe(
map((ciphers) => ciphers.length > 0),
catchError((error: unknown) => {
this.logService.error(
"[RiskInsightsOrchestratorService] Error checking vault items",
error,
);
return of(false);
}),
);
}),
shareReplay({ bufferSize: 1, refCount: true }),
takeUntil(this._destroy$),
);
}
private _setupCriticalApplicationContext() {
this.organizationDetails$
.pipe(
@@ -926,8 +958,10 @@ export class RiskInsightsOrchestratorService {
// Setup the user ID observable to track the current user
private _setupUserId() {
// Watch userId changes
this.accountService.activeAccount$.pipe(getUserId).subscribe((userId) => {
this._userIdSubject.next(userId);
});
this.accountService.activeAccount$
.pipe(getUserId, takeUntil(this._destroy$))
.subscribe((userId) => {
this._userIdSubject.next(userId);
});
}
}

View File

@@ -28,6 +28,7 @@ export class RiskInsightsDataService {
readonly enrichedReportData$: Observable<RiskInsightsEnrichedData | null> = of(null);
readonly isGeneratingReport$: Observable<boolean> = of(false);
readonly criticalReportResults$: Observable<RiskInsightsEnrichedData | null> = of(null);
readonly hasVaultItems$: Observable<boolean> = of(false);
// New applications that need review (reviewedDate === null)
readonly newApplications$: Observable<string[]> = of([]);
@@ -51,6 +52,7 @@ export class RiskInsightsDataService {
this.organizationDetails$ = this.orchestrator.organizationDetails$;
this.enrichedReportData$ = this.orchestrator.enrichedReportData$;
this.criticalReportResults$ = this.orchestrator.criticalReportResults$;
this.hasVaultItems$ = this.orchestrator.hasVaultItems$;
this.newApplications$ = this.orchestrator.newApplications$;
// Expose the loading state