Skip to content
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

[PM-15200] add "generated credential" screen reader notification #12877

Merged
merged 7 commits into from
Jan 24, 2025
12 changes: 12 additions & 0 deletions apps/browser/src/_locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,18 @@
"generatePassphrase": {
"message": "Generate passphrase"
},
"passwordGenerated": {
"message": "Password generated"
},
"passphraseGenerated": {
"message": "Passphrase generated"
},
"usernameGenerated": {
"message": "Username generated"
},
"emailGenerated": {
"message": "Email generated"
},
"regeneratePassword": {
"message": "Regenerate password"
},
Expand Down
11 changes: 6 additions & 5 deletions libs/common/src/tools/dependencies.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Observable } from "rxjs";

import { Policy } from "../admin-console/models/domain/policy";
import { OrganizationId, UserId } from "../types/guid";
import { Policy } from "@bitwarden/common/admin-console/models/domain/policy";
import { OrganizationId, UserId } from "@bitwarden/common/types/guid";

import { OrganizationEncryptor } from "./cryptography/organization-encryptor.abstraction";
import { UserEncryptor } from "./cryptography/user-encryptor.abstraction";
Expand Down Expand Up @@ -152,7 +152,8 @@ export type SingleUserDependency = {
};

/** A pattern for types that emit values exclusively when the dependency
* emits a message.
* emits a message. Set a type parameter when your method requires contextual
* information when the request is issued.
*
* Consumers of this dependency should emit when `on$` emits. If `on$`
* completes, the consumer should also complete. If `on$`
Expand All @@ -161,10 +162,10 @@ export type SingleUserDependency = {
* @remarks This dependency is useful when you have a nondeterministic
* or stateful algorithm that you would like to run when an event occurs.
*/
export type OnDependency = {
export type OnDependency<T = any> = {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

๐Ÿ“ The = any default type preserves existing uses of the OnDependency, which use emissions without using the emitted value. The T type parameter allows consumers to specify a specific type for when the observer requires invocation-specific arguments.

/** The stream that controls emissions
*/
on$: Observable<any>;
on$: Observable<T>;
};

/** A pattern for types that emit when a dependency is `true`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,6 @@ export class CredentialGeneratorHistoryComponent {

protected getGeneratedValueText(credential: GeneratedCredential) {
const info = this.generatorService.algorithm(credential.category);
return info.generatedValue;
return info.credentialType;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
type="button"
bitIconButton="bwi-generate"
buttonType="main"
(click)="generate('user request')"
(click)="generate(USER_REQUEST)"
[appA11yTitle]="credentialTypeGenerateLabel$ | async"
[disabled]="!(algorithm$ | async)"
>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { LiveAnnouncer } from "@angular/cdk/a11y";
import { Component, EventEmitter, Input, NgZone, OnDestroy, OnInit, Output } from "@angular/core";
import { FormBuilder } from "@angular/forms";
import {
Expand Down Expand Up @@ -28,6 +29,7 @@ import {
CredentialAlgorithm,
CredentialCategory,
CredentialGeneratorService,
GenerateRequest,
GeneratedCredential,
Generators,
getForwarderConfiguration,
Expand Down Expand Up @@ -60,6 +62,7 @@ export class CredentialGeneratorComponent implements OnInit, OnDestroy {
private accountService: AccountService,
private zone: NgZone,
private formBuilder: FormBuilder,
private ariaLive: LiveAnnouncer,
) {}

/** Binds the component to a specific user's settings. When this input is not provided,
Expand Down Expand Up @@ -185,10 +188,10 @@ export class CredentialGeneratorComponent implements OnInit, OnDestroy {
// continue with origin stream
return generator;
}),
withLatestFrom(this.userId$),
withLatestFrom(this.userId$, this.algorithm$),
takeUntil(this.destroyed),
)
.subscribe(([generated, userId]) => {
.subscribe(([generated, userId, algorithm]) => {
this.generatorHistoryService
.track(userId, generated.credential, generated.category, generated.generationDate)
.catch((e: unknown) => {
Expand All @@ -198,8 +201,12 @@ export class CredentialGeneratorComponent implements OnInit, OnDestroy {
// update subjects within the angular zone so that the
// template bindings refresh immediately
this.zone.run(() => {
if (generated.source === this.USER_REQUEST) {
this.announce(algorithm.onGeneratedMessage);
}

this.generatedCredential$.next(generated);
this.onGenerated.next(generated);
this.value$.next(generated.credential);
});
});

Expand Down Expand Up @@ -383,14 +390,18 @@ export class CredentialGeneratorComponent implements OnInit, OnDestroy {
this.algorithm$.pipe(takeUntil(this.destroyed)).subscribe((a) => {
this.zone.run(() => {
if (!a || a.onlyOnRequest) {
this.value$.next("-");
this.generatedCredential$.next(null);
} else {
this.generate("autogenerate").catch((e: unknown) => this.logService.error(e));
}
});
});
}

private announce(message: string) {
this.ariaLive.announce(message).catch((e) => this.logService.error(e));
}

private typeToGenerator$(type: CredentialAlgorithm) {
const dependencies = {
on$: this.generate$,
Expand Down Expand Up @@ -473,7 +484,7 @@ export class CredentialGeneratorComponent implements OnInit, OnDestroy {
*/
protected credentialTypeLabel$ = this.algorithm$.pipe(
filter((algorithm) => !!algorithm),
map(({ generatedValue }) => generatedValue),
map(({ credentialType }) => credentialType),
);

/** Emits hint key for the currently selected credential type */
Expand All @@ -482,21 +493,28 @@ export class CredentialGeneratorComponent implements OnInit, OnDestroy {
/** tracks the currently selected credential category */
protected category$ = new ReplaySubject<string>(1);

private readonly generatedCredential$ = new BehaviorSubject<GeneratedCredential>(null);

/** Emits the last generated value. */
protected readonly value$ = new BehaviorSubject<string>("");
protected readonly value$ = this.generatedCredential$.pipe(
map((generated) => generated?.credential ?? "-"),
);

/** Emits when the userId changes */
protected readonly userId$ = new BehaviorSubject<UserId>(null);

/** Identifies generator requests that were requested by the user */
protected readonly USER_REQUEST = "user request";

/** Emits when a new credential is requested */
private readonly generate$ = new Subject<string>();
private readonly generate$ = new Subject<GenerateRequest>();

/** Request a new value from the generator
* @param requestor a label used to trace generation request
* origin in the debugger.
*/
protected async generate(requestor: string) {
this.generate$.next(requestor);
this.generate$.next({ source: requestor });
}

private toOptions(algorithms: AlgorithmInfo[]) {
Expand All @@ -515,7 +533,7 @@ export class CredentialGeneratorComponent implements OnInit, OnDestroy {

// finalize subjects
this.generate$.complete();
this.value$.complete();
this.generatedCredential$.complete();

// finalize component bindings
this.onGenerated.complete();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
type="button"
bitIconButton="bwi-generate"
buttonType="main"
(click)="generate('user request')"
(click)="generate(USER_REQUEST)"
[appA11yTitle]="credentialTypeGenerateLabel$ | async"
[disabled]="!(algorithm$ | async)"
>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { LiveAnnouncer } from "@angular/cdk/a11y";
import { coerceBooleanProperty } from "@angular/cdk/coercion";
import { Component, EventEmitter, Input, NgZone, OnDestroy, OnInit, Output } from "@angular/core";
import {
Expand All @@ -16,7 +17,6 @@ import {
} from "rxjs";

import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { UserId } from "@bitwarden/common/types/guid";
import { ToastService, Option } from "@bitwarden/components";
Expand All @@ -28,6 +28,7 @@ import {
isPasswordAlgorithm,
AlgorithmInfo,
isSameAlgorithm,
GenerateRequest,
} from "@bitwarden/generator-core";
import { GeneratorHistoryService } from "@bitwarden/generator-history";

Expand All @@ -42,9 +43,9 @@ export class PasswordGeneratorComponent implements OnInit, OnDestroy {
private generatorHistoryService: GeneratorHistoryService,
private toastService: ToastService,
private logService: LogService,
private i18nService: I18nService,
private accountService: AccountService,
private zone: NgZone,
private ariaLive: LiveAnnouncer,
) {}

/** Binds the component to a specific user's settings.
Expand All @@ -67,14 +68,17 @@ export class PasswordGeneratorComponent implements OnInit, OnDestroy {
protected readonly userId$ = new BehaviorSubject<UserId>(null);

/** Emits when a new credential is requested */
private readonly generate$ = new Subject<string>();
private readonly generate$ = new Subject<GenerateRequest>();

/** Identifies generator requests that were requested by the user */
protected readonly USER_REQUEST = "user request";

/** Request a new value from the generator
* @param requestor a label used to trace generation request
* origin in the debugger.
*/
protected async generate(requestor: string) {
this.generate$.next(requestor);
this.generate$.next({ source: requestor });
}

/** Tracks changes to the selected credential type
Expand Down Expand Up @@ -137,10 +141,10 @@ export class PasswordGeneratorComponent implements OnInit, OnDestroy {
// continue with origin stream
return generator;
}),
withLatestFrom(this.userId$),
withLatestFrom(this.userId$, this.algorithm$),
takeUntil(this.destroyed),
)
.subscribe(([generated, userId]) => {
.subscribe(([generated, userId, algorithm]) => {
this.generatorHistoryService
.track(userId, generated.credential, generated.category, generated.generationDate)
.catch((e: unknown) => {
Expand All @@ -150,6 +154,10 @@ export class PasswordGeneratorComponent implements OnInit, OnDestroy {
// update subjects within the angular zone so that the
// template bindings refresh immediately
this.zone.run(() => {
if (generated.source === this.USER_REQUEST) {
this.announce(algorithm.onGeneratedMessage);
}

this.onGenerated.next(generated);
this.value$.next(generated.credential);
});
Expand Down Expand Up @@ -205,6 +213,10 @@ export class PasswordGeneratorComponent implements OnInit, OnDestroy {
});
}

private announce(message: string) {
this.ariaLive.announce(message).catch((e) => this.logService.error(e));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

๐Ÿ“ I tried writing this using a custom aria region and it simply wouldn't work. Luckily, @willmartian pointed out the LiveAnnouncer works. I wrapped it in a method so that the promise handling doesn't clutter up the observable bindings, which are already fairly complex.

}

private typeToGenerator$(type: CredentialAlgorithm) {
const dependencies = {
on$: this.generate$,
Expand Down Expand Up @@ -249,7 +261,7 @@ export class PasswordGeneratorComponent implements OnInit, OnDestroy {
*/
protected credentialTypeLabel$ = this.algorithm$.pipe(
filter((algorithm) => !!algorithm),
map(({ generatedValue }) => generatedValue),
map(({ credentialType }) => credentialType),
);

private toOptions(algorithms: AlgorithmInfo[]) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
type="button"
bitIconButton="bwi-generate"
buttonType="main"
(click)="generate('user request')"
(click)="generate(USER_REQUEST)"
[appA11yTitle]="credentialTypeGenerateLabel$ | async"
[disabled]="!(algorithm$ | async)"
>
Expand Down
Loading
Loading