[PM-12743] a11y changes to make new drop down list for send and vault…

… accessible (#11717) * updating new menus to allow tab + enter to submit the link/button * Updating New actions to use button instead of a for accessibiity purposes * refactor * refactor * test fix * fixes * fixing tests * fixing test * fixing tests --------- Co-authored-by: --global <>
bitwarden · Dec 2, 2024 · 0ff48aa · 0ff48aa
1 parent 456c516
commit 0ff48aa
Show file tree

Hide file tree

Showing 5 changed files with 148 additions and 106 deletions.
diff --git a/...src/vault/popup/components/vault-v2/new-item-dropdown/new-item-dropdown-v2.component.html b/...src/vault/popup/components/vault-v2/new-item-dropdown/new-item-dropdown-v2.component.html
@@ -3,19 +3,27 @@
   {{ "new" | i18n }}
 </button>
 <bit-menu #itemOptions>
-  <a bitMenuItem (click)="newItemNavigate(cipherType.Login)">
+  <a bitMenuItem [routerLink]="['/add-cipher']" [queryParams]="buildQueryParams(cipherType.Login)">
     <i class="bwi bwi-globe" slot="start" aria-hidden="true"></i>
     {{ "typeLogin" | i18n }}
   </a>
-  <a bitMenuItem (click)="newItemNavigate(cipherType.Card)">
+  <a bitMenuItem [routerLink]="['/add-cipher']" [queryParams]="buildQueryParams(cipherType.Card)">
     <i class="bwi bwi-credit-card" slot="start" aria-hidden="true"></i>
     {{ "typeCard" | i18n }}
   </a>
-  <a bitMenuItem (click)="newItemNavigate(cipherType.Identity)">
+  <a
+    bitMenuItem
+    [routerLink]="['/add-cipher']"
+    [queryParams]="buildQueryParams(cipherType.Identity)"
+  >
     <i class="bwi bwi-id-card" slot="start" aria-hidden="true"></i>
     {{ "typeIdentity" | i18n }}
   </a>
-  <a bitMenuItem (click)="newItemNavigate(cipherType.SecureNote)">
+  <a
+    bitMenuItem
+    [routerLink]="['/add-cipher']"
+    [queryParams]="buildQueryParams(cipherType.SecureNote)"
+  >
     <i class="bwi bwi-sticky-note" slot="start" aria-hidden="true"></i>
     {{ "note" | i18n }}
   </a>

diff --git a/.../vault/popup/components/vault-v2/new-item-dropdown/new-item-dropdown-v2.component.spec.ts b/.../vault/popup/components/vault-v2/new-item-dropdown/new-item-dropdown-v2.component.spec.ts
@@ -1,141 +1,163 @@
 import { CommonModule } from "@angular/common";
 import { ComponentFixture, TestBed } from "@angular/core/testing";
-import { Router } from "@angular/router";
+import { ActivatedRoute, RouterLink } from "@angular/router";
+import { mock } from "jest-mock-extended";
 
-import { JslibModule } from "@bitwarden/angular/jslib.module";
+import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { Utils } from "@bitwarden/common/platform/misc/utils";
+import { FolderApiServiceAbstraction } from "@bitwarden/common/vault/abstractions/folder/folder-api.service.abstraction";
+import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
 import { CipherType } from "@bitwarden/common/vault/enums";
-import { ButtonModule, DialogService, MenuModule } from "@bitwarden/components";
+import { ButtonModule, DialogService, MenuModule, NoItemsModule } from "@bitwarden/components";
 
 import { BrowserApi } from "../../../../../platform/browser/browser-api";
 import BrowserPopupUtils from "../../../../../platform/popup/browser-popup-utils";
-import { AddEditQueryParams } from "../add-edit/add-edit-v2.component";
-import { AddEditFolderDialogComponent } from "../add-edit-folder-dialog/add-edit-folder-dialog.component";
 
 import { NewItemDropdownV2Component, NewItemInitialValues } from "./new-item-dropdown-v2.component";
 
 describe("NewItemDropdownV2Component", () => {
   let component: NewItemDropdownV2Component;
   let fixture: ComponentFixture<NewItemDropdownV2Component>;
-  const open = jest.fn();
-  const navigate = jest.fn();
+  let dialogServiceMock: jest.Mocked<DialogService>;
+  let browserApiMock: jest.Mocked<typeof BrowserApi>;
 
-  jest
-    .spyOn(BrowserApi, "getTabFromCurrentWindow")
-    .mockResolvedValue({ url: "https://example.com" } as chrome.tabs.Tab);
+  const mockTab = { url: "https://example.com" };
+
+  beforeAll(() => {
+    jest.spyOn(BrowserApi, "getTabFromCurrentWindow").mockResolvedValue(mockTab as chrome.tabs.Tab);
+    jest.spyOn(BrowserPopupUtils, "inPopout").mockReturnValue(false);
+    jest.spyOn(Utils, "getHostname").mockReturnValue("example.com");
+  });
 
   beforeEach(async () => {
-    open.mockClear();
-    navigate.mockClear();
+    dialogServiceMock = mock<DialogService>();
+    dialogServiceMock.open.mockClear();
+
+    const activatedRouteMock = {
+      snapshot: { paramMap: { get: jest.fn() } },
+    };
+
+    const i18nServiceMock = mock<I18nService>();
+    const folderServiceMock = mock<FolderService>();
+    const folderApiServiceAbstractionMock = mock<FolderApiServiceAbstraction>();
+    const accountServiceMock = mock<AccountService>();
 
     await TestBed.configureTestingModule({
-      imports: [NewItemDropdownV2Component, MenuModule, ButtonModule, JslibModule, CommonModule],
+      imports: [
+        CommonModule,
+        RouterLink,
+        ButtonModule,
+        MenuModule,
+        NoItemsModule,
+        NewItemDropdownV2Component,
+      ],
       providers: [
-        { provide: I18nService, useValue: { t: (key: string) => key } },
-        { provide: Router, useValue: { navigate } },
+        { provide: DialogService, useValue: dialogServiceMock },
+        { provide: I18nService, useValue: i18nServiceMock },
+        { provide: ActivatedRoute, useValue: activatedRouteMock },
+        { provide: BrowserApi, useValue: browserApiMock },
+        { provide: FolderService, useValue: folderServiceMock },
+        { provide: FolderApiServiceAbstraction, useValue: folderApiServiceAbstractionMock },
+        { provide: AccountService, useValue: accountServiceMock },
       ],
-    })
-      .overrideProvider(DialogService, { useValue: { open } })
-      .compileComponents();
+    }).compileComponents();
+  });
 
+  beforeEach(() => {
     fixture = TestBed.createComponent(NewItemDropdownV2Component);
     component = fixture.componentInstance;
     fixture.detectChanges();
   });
 
-  it("opens new folder dialog", () => {
-    component.openFolderDialog();
+  describe("buildQueryParams", () => {
+    it("should build query params for a Login cipher when not popped out", async () => {
+      await component.ngOnInit();
+      component.initialValues = {
+        folderId: "222-333-444",
+        organizationId: "444-555-666",
+        collectionId: "777-888-999",
+      } as NewItemInitialValues;
 
-    expect(open).toHaveBeenCalledWith(AddEditFolderDialogComponent);
-  });
+      jest.spyOn(BrowserPopupUtils, "inPopout").mockReturnValue(false);
+      jest.spyOn(Utils, "getHostname").mockReturnValue("example.com");
 
-  describe("new item", () => {
-    const emptyParams: AddEditQueryParams = {
-      collectionId: undefined,
-      organizationId: undefined,
-      folderId: undefined,
-    };
+      const params = component.buildQueryParams(CipherType.Login);
 
-    beforeEach(() => {
-      jest.spyOn(component, "newItemNavigate");
+      expect(params).toEqual({
+        type: CipherType.Login.toString(),
+        collectionId: "777-888-999",
+        organizationId: "444-555-666",
+        folderId: "222-333-444",
+        uri: "https://example.com",
+        name: "example.com",
+      });
     });
 
-    it("navigates to new login", async () => {
-      await component.newItemNavigate(CipherType.Login);
+    it("should build query params for a Login cipher when popped out", () => {
+      component.initialValues = {
+        collectionId: "777-888-999",
+      } as NewItemInitialValues;
 
-      expect(navigate).toHaveBeenCalledWith(["/add-cipher"], {
-        queryParams: {
-          type: CipherType.Login.toString(),
-          name: "example.com",
-          uri: "https://example.com",
-          ...emptyParams,
-        },
-      });
-    });
+      jest.spyOn(BrowserPopupUtils, "inPopout").mockReturnValue(true);
 
-    it("navigates to new card", async () => {
-      await component.newItemNavigate(CipherType.Card);
+      const params = component.buildQueryParams(CipherType.Login);
 
-      expect(navigate).toHaveBeenCalledWith(["/add-cipher"], {
-        queryParams: { type: CipherType.Card.toString(), ...emptyParams },
+      expect(params).toEqual({
+        type: CipherType.Login.toString(),
+        collectionId: "777-888-999",
       });
     });
 
-    it("navigates to new identity", async () => {
-      await component.newItemNavigate(CipherType.Identity);
+    it("should build query params for a secure note", () => {
+      component.initialValues = {
+        collectionId: "777-888-999",
+      } as NewItemInitialValues;
 
-      expect(navigate).toHaveBeenCalledWith(["/add-cipher"], {
-        queryParams: { type: CipherType.Identity.toString(), ...emptyParams },
+      const params = component.buildQueryParams(CipherType.SecureNote);
+
+      expect(params).toEqual({
+        type: CipherType.SecureNote.toString(),
+        collectionId: "777-888-999",
       });
     });
 
-    it("navigates to new note", async () => {
-      await component.newItemNavigate(CipherType.SecureNote);
+    it("should build query params for an Identity", () => {
+      component.initialValues = {
+        collectionId: "777-888-999",
+      } as NewItemInitialValues;
+
+      const params = component.buildQueryParams(CipherType.Identity);
 
-      expect(navigate).toHaveBeenCalledWith(["/add-cipher"], {
-        queryParams: { type: CipherType.SecureNote.toString(), ...emptyParams },
+      expect(params).toEqual({
+        type: CipherType.Identity.toString(),
+        collectionId: "777-888-999",
       });
     });
 
-    it("includes initial values", async () => {
+    it("should build query params for a Card", () => {
       component.initialValues = {
-        folderId: "222-333-444",
-        organizationId: "444-555-666",
         collectionId: "777-888-999",
       } as NewItemInitialValues;
 
-      await component.newItemNavigate(CipherType.Login);
-
-      expect(navigate).toHaveBeenCalledWith(["/add-cipher"], {
-        queryParams: {
-          type: CipherType.Login.toString(),
-          folderId: "222-333-444",
-          organizationId: "444-555-666",
-          collectionId: "777-888-999",
-          uri: "https://example.com",
-          name: "example.com",
-        },
+      const params = component.buildQueryParams(CipherType.Card);
+
+      expect(params).toEqual({
+        type: CipherType.Card.toString(),
+        collectionId: "777-888-999",
       });
     });
 
-    it("does not include name or uri when the extension is popped out", async () => {
-      jest.spyOn(BrowserPopupUtils, "inPopout").mockReturnValue(true);
-
+    it("should build query params for a SshKey", () => {
       component.initialValues = {
-        folderId: "222-333-444",
-        organizationId: "444-555-666",
         collectionId: "777-888-999",
       } as NewItemInitialValues;
 
-      await component.newItemNavigate(CipherType.Login);
+      const params = component.buildQueryParams(CipherType.SshKey);
 
-      expect(navigate).toHaveBeenCalledWith(["/add-cipher"], {
-        queryParams: {
-          type: CipherType.Login.toString(),
-          folderId: "222-333-444",
-          organizationId: "444-555-666",
-          collectionId: "777-888-999",
-        },
+      expect(params).toEqual({
+        type: CipherType.SshKey.toString(),
+        collectionId: "777-888-999",
       });
     });
   });

diff --git a/...r/src/vault/popup/components/vault-v2/new-item-dropdown/new-item-dropdown-v2.component.ts b/...r/src/vault/popup/components/vault-v2/new-item-dropdown/new-item-dropdown-v2.component.ts
@@ -1,6 +1,6 @@
 import { CommonModule } from "@angular/common";
-import { Component, Input } from "@angular/core";
-import { Router, RouterLink } from "@angular/router";
+import { Component, Input, OnInit } from "@angular/core";
+import { RouterLink } from "@angular/router";
 
 import { JslibModule } from "@bitwarden/angular/jslib.module";
 import { Utils } from "@bitwarden/common/platform/misc/utils";
@@ -25,31 +25,31 @@ export interface NewItemInitialValues {
   standalone: true,
   imports: [NoItemsModule, JslibModule, CommonModule, ButtonModule, RouterLink, MenuModule],
 })
-export class NewItemDropdownV2Component {
+export class NewItemDropdownV2Component implements OnInit {
   cipherType = CipherType;
-
+  private tab?: chrome.tabs.Tab;
   /**
    * Optional initial values to pass to the add cipher form
    */
   @Input()
   initialValues: NewItemInitialValues;
 
-  constructor(
-    private router: Router,
-    private dialogService: DialogService,
-  ) {}
+  constructor(private dialogService: DialogService) {}
+
+  async ngOnInit() {
+    this.tab = await BrowserApi.getTabFromCurrentWindow();
+  }
 
-  private async buildQueryParams(type: CipherType): Promise<AddEditQueryParams> {
-    const tab = await BrowserApi.getTabFromCurrentWindow();
+  buildQueryParams(type: CipherType): AddEditQueryParams {
     const poppedOut = BrowserPopupUtils.inPopout(window);
 
     const loginDetails: { uri?: string; name?: string } = {};
 
     // When a Login Cipher is created and the extension is not popped out,
     // pass along the uri and name
-    if (!poppedOut && type === CipherType.Login && tab) {
-      loginDetails.uri = tab.url;
-      loginDetails.name = Utils.getHostname(tab.url);
+    if (!poppedOut && type === CipherType.Login && this.tab) {
+      loginDetails.uri = this.tab.url;
+      loginDetails.name = Utils.getHostname(this.tab.url);
     }
 
     return {
@@ -61,10 +61,6 @@ export class NewItemDropdownV2Component {
     };
   }
 
-  async newItemNavigate(type: CipherType) {
-    await this.router.navigate(["/add-cipher"], { queryParams: await this.buildQueryParams(type) });
-  }
-
   openFolderDialog() {
     this.dialogService.open(AddEditFolderDialogComponent);
   }

diff --git a/libs/tools/send/send-ui/src/new-send-dropdown/new-send-dropdown.component.html b/libs/tools/send/send-ui/src/new-send-dropdown/new-send-dropdown.component.html
@@ -3,11 +3,19 @@
   {{ (hideIcon ? "createSend" : "new") | i18n }}
 </button>
 <bit-menu #itemOptions>
-  <a type="button" bitMenuItem (click)="newItemNavigate(sendType.Text)">
+  <a
+    bitMenuItem
+    [routerLink]="buildRouterLink(sendType.File)"
+    [queryParams]="buildQueryParams(sendType.Text)"
+  >
     <i class="bwi bwi-file-text" slot="start" aria-hidden="true"></i>
     {{ "sendTypeText" | i18n }}
   </a>
-  <a type="button" bitMenuItem (click)="newItemNavigate(sendType.File)">
+  <a
+    bitMenuItem
+    [routerLink]="buildRouterLink(sendType.File)"
+    [queryParams]="buildQueryParams(sendType.File)"
+  >
     <i class="bwi bwi-file" slot="start" aria-hidden="true"></i>
     {{ "sendTypeFile" | i18n }}
     <button type="button" slot="end" *ngIf="hasNoPremium" bitBadge variant="success">

diff --git a/libs/tools/send/send-ui/src/new-send-dropdown/new-send-dropdown.component.ts b/libs/tools/send/send-ui/src/new-send-dropdown/new-send-dropdown.component.ts
@@ -32,10 +32,18 @@ export class NewSendDropdownComponent implements OnInit {
     ));
   }
 
-  newItemNavigate(type: SendType) {
+  buildRouterLink(type: SendType) {
     if (this.hasNoPremium && type === SendType.File) {
-      return this.router.navigate(["/premium"]);
+      return "/premium";
+    } else {
+      return "/add-send";
     }
-    void this.router.navigate(["/add-send"], { queryParams: { type: type, isNew: true } });
+  }
+
+  buildQueryParams(type: SendType) {
+    if (this.hasNoPremium && type === SendType.File) {
+      return null;
+    }
+    return { type: type, isNew: true };
   }
 }