From 320d4f65fa6ce0fd3bb9cdc526e6c39ddb66f13a Mon Sep 17 00:00:00 2001 From: Daniel Riera Date: Wed, 23 Apr 2025 14:47:09 -0400 Subject: [PATCH] PM-20396 open view item vault pop out (#14342) * PM-20396 open view item vault pop out * add aria and clean up * format json * clean naming in messages * revert feature-flag.enum.ts * change username to item name * return nullish operator removed in testing * update tests to account for itemName * revert to anchor tag --- apps/browser/src/_locales/en/messages.json | 43 +++++++++++-------- .../abstractions/notification.background.ts | 4 ++ .../notification.background.spec.ts | 11 +++-- .../background/notification.background.ts | 28 ++++++++++-- .../notification/confirmation/body.ts | 5 ++- .../notification/confirmation/container.ts | 19 ++++---- .../notification/confirmation/message.ts | 17 +++++++- .../abstractions/notification-bar.ts | 2 +- apps/browser/src/autofill/notification/bar.ts | 16 +++---- 9 files changed, 99 insertions(+), 46 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 462af12a352..d1d05a4e852 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -192,7 +192,7 @@ "message": "Copy", "description": "Copy to clipboard" }, - "fill":{ + "fill": { "message": "Fill", "description": "This string is used on the vault page to indicate autofilling. Horizontal space is limited in the interface here so try and keep translations as concise as possible." }, @@ -1062,6 +1062,15 @@ "notificationAddSave": { "message": "Save" }, + "notificationViewAria": { + "message": "View $ITEMNAME$, opens in new window", + "placeholders": { + "itemName": { + "content": "$1" + } + }, + "description": "Aria label for the view button in notification bar confirmation message" + }, "newNotification": { "message": "New notification" }, @@ -1075,23 +1084,23 @@ } } }, - "loginSaveSuccessDetails": { - "message": "$USERNAME$ saved to Bitwarden.", - "placeholders": { - "username": { - "content": "$1" - } - }, - "description": "Shown to user after login is saved." + "loginSaveConfirmation": { + "message": "$ITEMNAME$ saved to Bitwarden.", + "placeholders": { + "itemName": { + "content": "$1" + } + }, + "description": "Shown to user after item is saved." }, - "loginUpdatedSuccessDetails": { - "message": "$USERNAME$ updated in Bitwarden.", - "placeholders": { - "username": { - "content": "$1" - } - }, - "description": "Shown to user after login is updated." + "loginUpdatedConfirmation": { + "message": "$ITEMNAME$ updated in Bitwarden.", + "placeholders": { + "itemName": { + "content": "$1" + } + }, + "description": "Shown to user after item is updated." }, "saveAsNewLoginAction": { "message": "Save as new login", diff --git a/apps/browser/src/autofill/background/abstractions/notification.background.ts b/apps/browser/src/autofill/background/abstractions/notification.background.ts index 6b3c91a109c..c93fd9a3acf 100644 --- a/apps/browser/src/autofill/background/abstractions/notification.background.ts +++ b/apps/browser/src/autofill/background/abstractions/notification.background.ts @@ -101,6 +101,10 @@ type NotificationBackgroundExtensionMessageHandlers = { bgChangedPassword: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; bgRemoveTabFromNotificationQueue: ({ sender }: BackgroundSenderParam) => void; bgSaveCipher: ({ message, sender }: BackgroundOnMessageHandlerParams) => void; + bgOpenViewVaultItemPopout: ({ + message, + sender, + }: BackgroundOnMessageHandlerParams) => Promise; bgOpenVault: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; bgNeverSave: ({ sender }: BackgroundSenderParam) => Promise; bgUnlockPopoutOpened: ({ message, sender }: BackgroundOnMessageHandlerParams) => Promise; diff --git a/apps/browser/src/autofill/background/notification.background.spec.ts b/apps/browser/src/autofill/background/notification.background.spec.ts index ffc416ab62a..bb993fcf94b 100644 --- a/apps/browser/src/autofill/background/notification.background.spec.ts +++ b/apps/browser/src/autofill/background/notification.background.spec.ts @@ -823,6 +823,7 @@ describe("NotificationBackground", () => { notificationBackground["notificationQueue"] = [queueMessage]; const cipherView = mock({ id: "testId", + name: "testItemName", login: { username: "testUser" }, }); getDecryptedCipherByIdSpy.mockResolvedValueOnce(cipherView); @@ -844,8 +845,9 @@ describe("NotificationBackground", () => { sender.tab, "saveCipherAttemptCompleted", { - username: cipherView.login.username, + itemName: "testItemName", cipherId: cipherView.id, + task: undefined, }, ); }); @@ -899,7 +901,7 @@ describe("NotificationBackground", () => { const cipherView = mock({ id: mockCipherId, organizationId: mockOrgId, - login: { username: "testUser" }, + name: "Test Item", }); getDecryptedCipherByIdSpy.mockResolvedValueOnce(cipherView); @@ -921,11 +923,11 @@ describe("NotificationBackground", () => { "saveCipherAttemptCompleted", { cipherId: "testId", + itemName: "Test Item", task: { orgName: "Org Name, LLC", remainingTasksCount: 1, }, - username: "testUser", }, ); }); @@ -1074,6 +1076,7 @@ describe("NotificationBackground", () => { notificationBackground["notificationQueue"] = [queueMessage]; const cipherView = mock({ id: "testId", + name: "testName", login: { username: "test", password: "password" }, }); folderExistsSpy.mockResolvedValueOnce(false); @@ -1097,8 +1100,8 @@ describe("NotificationBackground", () => { sender.tab, "saveCipherAttemptCompleted", { - username: cipherView.login.username, cipherId: cipherView.id, + itemName: cipherView.name, }, ); expect(tabSendMessageSpy).toHaveBeenCalledWith(sender.tab, { command: "addedCipher" }); diff --git a/apps/browser/src/autofill/background/notification.background.ts b/apps/browser/src/autofill/background/notification.background.ts index 00d24300d78..dabb75b97b6 100644 --- a/apps/browser/src/autofill/background/notification.background.ts +++ b/apps/browser/src/autofill/background/notification.background.ts @@ -41,7 +41,10 @@ import { SecurityTask } from "@bitwarden/common/vault/tasks/models/security-task import { openUnlockPopout } from "../../auth/popup/utils/auth-popout-window"; import { BrowserApi } from "../../platform/browser/browser-api"; -import { openAddEditVaultItemPopout } from "../../vault/popup/utils/vault-popout-window"; +import { + openAddEditVaultItemPopout, + openViewVaultItemPopout, +} from "../../vault/popup/utils/vault-popout-window"; import { OrganizationCategory, OrganizationCategories, @@ -67,6 +70,7 @@ import { OverlayBackgroundExtensionMessage } from "./abstractions/overlay.backgr export default class NotificationBackground { private openUnlockPopout = openUnlockPopout; private openAddEditVaultItemPopout = openAddEditVaultItemPopout; + private openViewVaultItemPopout = openViewVaultItemPopout; private notificationQueue: NotificationQueueMessageItem[] = []; private allowedRetryCommands: Set = new Set([ ExtensionCommand.AutofillLogin, @@ -91,6 +95,7 @@ export default class NotificationBackground { bgGetOrgData: () => this.getOrgData(), bgNeverSave: ({ sender }) => this.saveNever(sender.tab), bgOpenVault: ({ message, sender }) => this.openVault(message, sender.tab), + bgOpenViewVaultItemPopout: ({ message, sender }) => this.viewItem(message, sender.tab), bgRemoveTabFromNotificationQueue: ({ sender }) => this.removeTabFromNotificationQueue(sender.tab), bgReopenUnlockPopout: ({ sender }) => this.openUnlockPopout(sender.tab), @@ -638,8 +643,8 @@ export default class NotificationBackground { try { await this.cipherService.createWithServer(cipher); await BrowserApi.tabSendMessageData(tab, "saveCipherAttemptCompleted", { - username: queueMessage?.username && String(queueMessage.username), - cipherId: cipher?.id && String(cipher.id), + itemName: newCipher?.name && String(newCipher?.name), + cipherId: cipher?.id && String(cipher?.id), }); await BrowserApi.tabSendMessage(tab, { command: "addedCipher" }); } catch (error) { @@ -701,7 +706,7 @@ export default class NotificationBackground { await this.cipherService.updateWithServer(cipher); await BrowserApi.tabSendMessageData(tab, "saveCipherAttemptCompleted", { - username: cipherView?.login?.username && String(cipherView.login.username), + itemName: cipherView?.name && String(cipherView?.name), cipherId: cipherView?.id && String(cipherView.id), task: taskData, }); @@ -754,6 +759,21 @@ export default class NotificationBackground { await this.openAddEditVaultItemPopout(senderTab, { cipherId: message.cipherId }); } + private async viewItem( + message: NotificationBackgroundExtensionMessage, + senderTab: chrome.tabs.Tab, + ) { + await Promise.all([ + this.openViewVaultItemPopout(senderTab, { + cipherId: message.cipherId, + action: null, + }), + BrowserApi.tabSendMessageData(senderTab, "closeNotificationBar", { + fadeOutNotification: !!message.fadeOutNotification, + }), + ]); + } + private async folderExists(folderId: string, userId: UserId) { if (Utils.isNullOrWhitespace(folderId) || folderId === "null") { return false; diff --git a/apps/browser/src/autofill/content/components/notification/confirmation/body.ts b/apps/browser/src/autofill/content/components/notification/confirmation/body.ts index d2ac7f36277..0508991c5da 100644 --- a/apps/browser/src/autofill/content/components/notification/confirmation/body.ts +++ b/apps/browser/src/autofill/content/components/notification/confirmation/body.ts @@ -16,16 +16,18 @@ const { css } = createEmotion({ export type NotificationConfirmationBodyProps = { buttonText: string; + itemName: string; confirmationMessage: string; error?: string; messageDetails?: string; tasksAreComplete?: boolean; theme: Theme; - handleOpenVault: (e: Event) => void; + handleOpenVault: () => void; }; export function NotificationConfirmationBody({ buttonText, + itemName, confirmationMessage, error, messageDetails, @@ -43,6 +45,7 @@ export function NotificationConfirmationBody({ ${showConfirmationMessage ? NotificationConfirmationMessage({ buttonText, + itemName, message: confirmationMessage, messageDetails, theme, diff --git a/apps/browser/src/autofill/content/components/notification/confirmation/container.ts b/apps/browser/src/autofill/content/components/notification/confirmation/container.ts index a071338af9a..5cc977cf4cb 100644 --- a/apps/browser/src/autofill/content/components/notification/confirmation/container.ts +++ b/apps/browser/src/autofill/content/components/notification/confirmation/container.ts @@ -20,14 +20,14 @@ import { NotificationConfirmationFooter } from "./footer"; export type NotificationConfirmationContainerProps = NotificationBarIframeInitData & { handleCloseNotification: (e: Event) => void; - handleOpenVault: (e: Event) => void; + handleOpenVault: () => void; handleOpenTasks: (e: Event) => void; } & { error?: string; i18n: { [key: string]: string }; + itemName: string; task?: NotificationTaskInfo; type: NotificationType; - username: string; }; export function NotificationConfirmationContainer({ @@ -36,13 +36,13 @@ export function NotificationConfirmationContainer({ handleOpenVault, handleOpenTasks, i18n, + itemName, task, theme = ThemeTypes.Light, type, - username, }: NotificationConfirmationContainerProps) { const headerMessage = getHeaderMessage(i18n, type, error); - const confirmationMessage = getConfirmationMessage(i18n, username, type, error); + const confirmationMessage = getConfirmationMessage(i18n, itemName, type, error); const buttonText = error ? i18n.newItem : i18n.view; let messageDetails: string | undefined; @@ -71,6 +71,7 @@ export function NotificationConfirmationContainer({ })} ${NotificationConfirmationBody({ buttonText, + itemName, confirmationMessage, tasksAreComplete, messageDetails, @@ -106,19 +107,17 @@ const notificationContainerStyles = (theme: Theme) => css` function getConfirmationMessage( i18n: { [key: string]: string }, - username: string, + itemName: string, type?: NotificationType, error?: string, ) { - const loginSaveSuccessDetails = chrome.i18n.getMessage("loginSaveSuccessDetails", [username]); - const loginUpdatedSuccessDetails = chrome.i18n.getMessage("loginUpdatedSuccessDetails", [ - username, - ]); + const loginSaveConfirmation = chrome.i18n.getMessage("loginSaveConfirmation", [itemName]); + const loginUpdatedConfirmation = chrome.i18n.getMessage("loginUpdatedConfirmation", [itemName]); if (error) { return i18n.saveFailureDetails; } - return type === "add" ? loginSaveSuccessDetails : loginUpdatedSuccessDetails; + return type === "add" ? loginSaveConfirmation : loginUpdatedConfirmation; } function getHeaderMessage( diff --git a/apps/browser/src/autofill/content/components/notification/confirmation/message.ts b/apps/browser/src/autofill/content/components/notification/confirmation/message.ts index c018371caff..3707e628370 100644 --- a/apps/browser/src/autofill/content/components/notification/confirmation/message.ts +++ b/apps/browser/src/autofill/content/components/notification/confirmation/message.ts @@ -7,19 +7,23 @@ import { themes, typography } from "../../constants/styles"; export type NotificationConfirmationMessageProps = { buttonText?: string; + itemName: string; message?: string; messageDetails?: string; - handleClick: (e: Event) => void; + handleClick: () => void; theme: Theme; }; export function NotificationConfirmationMessage({ buttonText, + itemName, message, messageDetails, handleClick, theme, }: NotificationConfirmationMessageProps) { + const buttonAria = chrome.i18n.getMessage("notificationViewAria", [itemName]); + return html`
${message || buttonText @@ -35,6 +39,10 @@ export function NotificationConfirmationMessage({ title=${buttonText} class=${notificationConfirmationButtonTextStyles(theme)} @click=${handleClick} + @keydown=${(e: KeyboardEvent) => handleButtonKeyDown(e, handleClick)} + aria-label=${buttonAria} + tabindex="0" + role="button" > ${buttonText} @@ -81,3 +89,10 @@ const AdditionalMessageStyles = ({ theme }: { theme: Theme }) => css` font-size: 14px; color: ${themes[theme].text.muted}; `; + +function handleButtonKeyDown(event: KeyboardEvent, handleClick: () => void) { + if (event.key === "Enter" || event.key === " ") { + event.preventDefault(); + handleClick(); + } +} diff --git a/apps/browser/src/autofill/notification/abstractions/notification-bar.ts b/apps/browser/src/autofill/notification/abstractions/notification-bar.ts index cbfeffcf2f4..9dd02b64154 100644 --- a/apps/browser/src/autofill/notification/abstractions/notification-bar.ts +++ b/apps/browser/src/autofill/notification/abstractions/notification-bar.ts @@ -33,7 +33,7 @@ type NotificationBarWindowMessage = { data?: { cipherId?: string; task?: NotificationTaskInfo; - username?: string; + itemName?: string; }; error?: string; initData?: NotificationBarIframeInitData; diff --git a/apps/browser/src/autofill/notification/bar.ts b/apps/browser/src/autofill/notification/bar.ts index 4e85d893178..fce05913e5e 100644 --- a/apps/browser/src/autofill/notification/bar.ts +++ b/apps/browser/src/autofill/notification/bar.ts @@ -56,9 +56,9 @@ function getI18n() { collection: chrome.i18n.getMessage("collection"), folder: chrome.i18n.getMessage("folder"), loginSaveSuccess: chrome.i18n.getMessage("loginSaveSuccess"), - loginSaveSuccessDetails: chrome.i18n.getMessage("loginSaveSuccessDetails"), + loginSaveConfirmation: chrome.i18n.getMessage("loginSaveConfirmation"), loginUpdateSuccess: chrome.i18n.getMessage("loginUpdateSuccess"), - loginUpdateSuccessDetails: chrome.i18n.getMessage("loginUpdatedSuccessDetails"), + loginUpdateConfirmation: chrome.i18n.getMessage("loginUpdatedConfirmation"), loginUpdateTaskSuccess: chrome.i18n.getMessage("loginUpdateTaskSuccess"), loginUpdateTaskSuccessAdditional: chrome.i18n.getMessage("loginUpdateTaskSuccessAdditional"), nextSecurityTaskAction: chrome.i18n.getMessage("nextSecurityTaskAction"), @@ -72,6 +72,7 @@ function getI18n() { notificationEdit: chrome.i18n.getMessage("edit"), notificationUnlock: chrome.i18n.getMessage("notificationUnlock"), notificationUnlockDesc: chrome.i18n.getMessage("notificationUnlockDesc"), + notificationViewAria: chrome.i18n.getMessage("notificationViewAria"), saveAction: chrome.i18n.getMessage("notificationAddSave"), saveAsNewLoginAction: chrome.i18n.getMessage("saveAsNewLoginAction"), saveFailure: chrome.i18n.getMessage("saveFailure"), @@ -349,10 +350,9 @@ function handleSaveCipherAttemptCompletedMessage(message: NotificationBarWindowM ); } -function openViewVaultItemPopout(e: Event, cipherId: string) { - e.preventDefault(); +function openViewVaultItemPopout(cipherId: string) { sendPlatformMessage({ - command: "bgOpenVault", + command: "bgOpenViewVaultItemPopout", cipherId, }); } @@ -360,7 +360,7 @@ function openViewVaultItemPopout(e: Event, cipherId: string) { function handleSaveCipherConfirmation(message: NotificationBarWindowMessage) { const { theme, type } = notificationBarIframeInitData; const { error, data } = message; - const { username, cipherId, task } = data || {}; + const { cipherId, task, itemName } = data || {}; const i18n = getI18n(); const resolvedTheme = getResolvedTheme(theme ?? ThemeTypes.Light); @@ -374,9 +374,9 @@ function handleSaveCipherConfirmation(message: NotificationBarWindowMessage) { handleCloseNotification, i18n, error, - username: username ?? i18n.typeLogin, + itemName: itemName ?? i18n.typeLogin, task, - handleOpenVault: (e) => cipherId && openViewVaultItemPopout(e, cipherId), + handleOpenVault: () => cipherId && openViewVaultItemPopout(cipherId), handleOpenTasks: () => sendPlatformMessage({ command: "bgOpenAtRisksPasswords" }), }), document.body,