diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 9f15bfd840f..5026f5e2799 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -6123,6 +6123,12 @@ "whyAmISeeingThis": { "message": "Why am I seeing this?" }, + "items": { + "message": "Items" + }, + "searchResults": { + "message": "Search results" + }, "resizeSideNavigation": { "message": "Resize side navigation" }, diff --git a/apps/browser/src/autofill/content/autofill-init.spec.ts b/apps/browser/src/autofill/content/autofill-init.spec.ts index d612e63f82c..eef7fe32dd0 100644 --- a/apps/browser/src/autofill/content/autofill-init.spec.ts +++ b/apps/browser/src/autofill/content/autofill-init.spec.ts @@ -347,6 +347,18 @@ describe("AutofillInit", () => { ); }); + it("removes the LOAD event listener", () => { + jest.spyOn(window, "removeEventListener"); + + autofillInit.init(); + autofillInit.destroy(); + + expect(window.removeEventListener).toHaveBeenCalledWith( + "load", + autofillInit["sendCollectDetailsMessage"], + ); + }); + it("removes the extension message listeners", () => { autofillInit.destroy(); diff --git a/apps/browser/src/autofill/content/autofill-init.ts b/apps/browser/src/autofill/content/autofill-init.ts index b6fc6c3392e..80cfe5de49f 100644 --- a/apps/browser/src/autofill/content/autofill-init.ts +++ b/apps/browser/src/autofill/content/autofill-init.ts @@ -72,21 +72,24 @@ class AutofillInit implements AutofillInitInterface { * to act on the page. */ private collectPageDetailsOnLoad() { - const sendCollectDetailsMessage = () => { - this.clearCollectPageDetailsOnLoadTimeout(); - this.collectPageDetailsOnLoadTimeout = setTimeout( - () => this.sendExtensionMessage("bgCollectPageDetails", { sender: "autofillInit" }), - 750, - ); - }; - if (globalThis.document.readyState === "complete") { - sendCollectDetailsMessage(); + this.sendCollectDetailsMessage(); } - globalThis.addEventListener(EVENTS.LOAD, sendCollectDetailsMessage); + globalThis.addEventListener(EVENTS.LOAD, this.sendCollectDetailsMessage); } + /** + * Sends a message to collect page details after a short delay. + */ + private sendCollectDetailsMessage = () => { + this.clearCollectPageDetailsOnLoadTimeout(); + this.collectPageDetailsOnLoadTimeout = setTimeout( + () => this.sendExtensionMessage("bgCollectPageDetails", { sender: "autofillInit" }), + 750, + ); + }; + /** * Collects the page details and sends them to the * extension background script. If the `sendDetailsInResponse` @@ -218,6 +221,7 @@ class AutofillInit implements AutofillInitInterface { */ destroy() { this.clearCollectPageDetailsOnLoadTimeout(); + globalThis.removeEventListener(EVENTS.LOAD, this.sendCollectDetailsMessage); chrome.runtime.onMessage.removeListener(this.handleExtensionMessage); this.collectAutofillContentService.destroy(); this.autofillOverlayContentService?.destroy(); diff --git a/apps/browser/src/autofill/overlay/inline-menu/abstractions/autofill-inline-menu-iframe.service.ts b/apps/browser/src/autofill/overlay/inline-menu/abstractions/autofill-inline-menu-iframe.service.ts index f55faec887a..ab8b0e2553e 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/abstractions/autofill-inline-menu-iframe.service.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/abstractions/autofill-inline-menu-iframe.service.ts @@ -32,4 +32,5 @@ export type BackgroundPortMessageHandlers = { export interface AutofillInlineMenuIframeService { initMenuIframe(): void; + destroy(): void; } diff --git a/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.spec.ts b/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.spec.ts index b7bd24c537b..00c214c32e7 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.spec.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.spec.ts @@ -645,6 +645,292 @@ describe("AutofillInlineMenuContentService", () => { expect(disconnectSpy).toHaveBeenCalled(); }); + + it("unobserves custom elements", () => { + const disconnectSpy = jest.spyOn( + autofillInlineMenuContentService["inlineMenuElementsMutationObserver"], + "disconnect", + ); + + autofillInlineMenuContentService.destroy(); + + expect(disconnectSpy).toHaveBeenCalled(); + }); + + it("unobserves the container element", () => { + const disconnectSpy = jest.spyOn( + autofillInlineMenuContentService["containerElementMutationObserver"], + "disconnect", + ); + + autofillInlineMenuContentService.destroy(); + + expect(disconnectSpy).toHaveBeenCalled(); + }); + + it("clears the mutation observer iterations reset timeout", () => { + jest.useFakeTimers(); + const clearTimeoutSpy = jest.spyOn(globalThis, "clearTimeout"); + autofillInlineMenuContentService["mutationObserverIterationsResetTimeout"] = setTimeout( + jest.fn(), + 1000, + ); + + autofillInlineMenuContentService.destroy(); + + expect(clearTimeoutSpy).toHaveBeenCalled(); + expect(autofillInlineMenuContentService["mutationObserverIterationsResetTimeout"]).toBeNull(); + }); + + it("destroys the button iframe", () => { + const mockButtonIframe = { destroy: jest.fn() }; + autofillInlineMenuContentService["buttonIframe"] = mockButtonIframe as any; + + autofillInlineMenuContentService.destroy(); + + expect(mockButtonIframe.destroy).toHaveBeenCalled(); + }); + + it("destroys the list iframe", () => { + const mockListIframe = { destroy: jest.fn() }; + autofillInlineMenuContentService["listIframe"] = mockListIframe as any; + + autofillInlineMenuContentService.destroy(); + + expect(mockListIframe.destroy).toHaveBeenCalled(); + }); + }); + + describe("observeCustomElements", () => { + it("observes the button element for attribute mutations", () => { + const buttonElement = document.createElement("div"); + autofillInlineMenuContentService["buttonElement"] = buttonElement; + const observeSpy = jest.spyOn( + autofillInlineMenuContentService["inlineMenuElementsMutationObserver"], + "observe", + ); + + autofillInlineMenuContentService["observeCustomElements"](); + + expect(observeSpy).toHaveBeenCalledWith(buttonElement, { attributes: true }); + }); + + it("observes the list element for attribute mutations", () => { + const listElement = document.createElement("div"); + autofillInlineMenuContentService["listElement"] = listElement; + const observeSpy = jest.spyOn( + autofillInlineMenuContentService["inlineMenuElementsMutationObserver"], + "observe", + ); + + autofillInlineMenuContentService["observeCustomElements"](); + + expect(observeSpy).toHaveBeenCalledWith(listElement, { attributes: true }); + }); + + it("does not observe when no elements exist", () => { + autofillInlineMenuContentService["buttonElement"] = undefined; + autofillInlineMenuContentService["listElement"] = undefined; + const observeSpy = jest.spyOn( + autofillInlineMenuContentService["inlineMenuElementsMutationObserver"], + "observe", + ); + + autofillInlineMenuContentService["observeCustomElements"](); + + expect(observeSpy).not.toHaveBeenCalled(); + }); + }); + + describe("observeContainerElement", () => { + it("observes the container element for child list mutations", () => { + const containerElement = document.createElement("div"); + const observeSpy = jest.spyOn( + autofillInlineMenuContentService["containerElementMutationObserver"], + "observe", + ); + + autofillInlineMenuContentService["observeContainerElement"](containerElement); + + expect(observeSpy).toHaveBeenCalledWith(containerElement, { childList: true }); + }); + }); + + describe("unobserveContainerElement", () => { + it("disconnects the container element mutation observer", () => { + const disconnectSpy = jest.spyOn( + autofillInlineMenuContentService["containerElementMutationObserver"], + "disconnect", + ); + + autofillInlineMenuContentService["unobserveContainerElement"](); + + expect(disconnectSpy).toHaveBeenCalled(); + }); + + it("handles the case when the mutation observer is undefined", () => { + autofillInlineMenuContentService["containerElementMutationObserver"] = undefined as any; + + expect(() => autofillInlineMenuContentService["unobserveContainerElement"]()).not.toThrow(); + }); + }); + + describe("observePageAttributes", () => { + it("observes the document element for attribute mutations", () => { + const observeSpy = jest.spyOn( + autofillInlineMenuContentService["htmlMutationObserver"], + "observe", + ); + + autofillInlineMenuContentService["observePageAttributes"](); + + expect(observeSpy).toHaveBeenCalledWith(document.documentElement, { attributes: true }); + }); + + it("observes the body element for attribute mutations", () => { + const observeSpy = jest.spyOn( + autofillInlineMenuContentService["bodyMutationObserver"], + "observe", + ); + + autofillInlineMenuContentService["observePageAttributes"](); + + expect(observeSpy).toHaveBeenCalledWith(document.body, { attributes: true }); + }); + }); + + describe("unobservePageAttributes", () => { + it("disconnects the html mutation observer", () => { + const disconnectSpy = jest.spyOn( + autofillInlineMenuContentService["htmlMutationObserver"], + "disconnect", + ); + + autofillInlineMenuContentService["unobservePageAttributes"](); + + expect(disconnectSpy).toHaveBeenCalled(); + }); + + it("disconnects the body mutation observer", () => { + const disconnectSpy = jest.spyOn( + autofillInlineMenuContentService["bodyMutationObserver"], + "disconnect", + ); + + autofillInlineMenuContentService["unobservePageAttributes"](); + + expect(disconnectSpy).toHaveBeenCalled(); + }); + }); + + describe("checkPageRisks", () => { + it("returns true and closes inline menu when page is not opaque", async () => { + jest.spyOn(autofillInlineMenuContentService as any, "getPageIsOpaque").mockReturnValue(false); + const closeInlineMenuSpy = jest.spyOn( + autofillInlineMenuContentService as any, + "closeInlineMenu", + ); + + const result = await autofillInlineMenuContentService["checkPageRisks"](); + + expect(result).toBe(true); + expect(closeInlineMenuSpy).toHaveBeenCalled(); + }); + + it("returns true and closes inline menu when inline menu is disabled", async () => { + jest.spyOn(autofillInlineMenuContentService as any, "getPageIsOpaque").mockReturnValue(true); + autofillInlineMenuContentService["inlineMenuEnabled"] = false; + const closeInlineMenuSpy = jest.spyOn( + autofillInlineMenuContentService as any, + "closeInlineMenu", + ); + + const result = await autofillInlineMenuContentService["checkPageRisks"](); + + expect(result).toBe(true); + expect(closeInlineMenuSpy).toHaveBeenCalled(); + }); + + it("returns false when page is opaque and inline menu is enabled", async () => { + jest.spyOn(autofillInlineMenuContentService as any, "getPageIsOpaque").mockReturnValue(true); + autofillInlineMenuContentService["inlineMenuEnabled"] = true; + const closeInlineMenuSpy = jest.spyOn( + autofillInlineMenuContentService as any, + "closeInlineMenu", + ); + + const result = await autofillInlineMenuContentService["checkPageRisks"](); + + expect(result).toBe(false); + expect(closeInlineMenuSpy).not.toHaveBeenCalled(); + }); + }); + + describe("handlePageMutations", () => { + it("checks page risks when mutations include attribute changes", async () => { + const checkPageRisksSpy = jest.spyOn( + autofillInlineMenuContentService as any, + "checkPageRisks", + ); + const mutations = [{ type: "attributes" } as MutationRecord]; + + await autofillInlineMenuContentService["handlePageMutations"](mutations); + + expect(checkPageRisksSpy).toHaveBeenCalled(); + }); + + it("does not check page risks when mutations do not include attribute changes", async () => { + const checkPageRisksSpy = jest.spyOn( + autofillInlineMenuContentService as any, + "checkPageRisks", + ); + const mutations = [{ type: "childList" } as MutationRecord]; + + await autofillInlineMenuContentService["handlePageMutations"](mutations); + + expect(checkPageRisksSpy).not.toHaveBeenCalled(); + }); + }); + + describe("clearPersistentLastChildOverrideTimeout", () => { + it("clears the timeout when it exists", () => { + jest.useFakeTimers(); + const clearTimeoutSpy = jest.spyOn(globalThis, "clearTimeout"); + autofillInlineMenuContentService["handlePersistentLastChildOverrideTimeout"] = setTimeout( + jest.fn(), + 1000, + ); + + autofillInlineMenuContentService["clearPersistentLastChildOverrideTimeout"](); + + expect(clearTimeoutSpy).toHaveBeenCalled(); + }); + + it("does nothing when the timeout is null", () => { + const clearTimeoutSpy = jest.spyOn(globalThis, "clearTimeout"); + autofillInlineMenuContentService["handlePersistentLastChildOverrideTimeout"] = null; + + autofillInlineMenuContentService["clearPersistentLastChildOverrideTimeout"](); + + expect(clearTimeoutSpy).not.toHaveBeenCalled(); + }); + }); + + describe("elementAtCenterOfInlineMenuPosition", () => { + it("returns the element at the center of the given position", () => { + const mockElement = document.createElement("div"); + jest.spyOn(globalThis.document, "elementFromPoint").mockReturnValue(mockElement); + + const result = autofillInlineMenuContentService["elementAtCenterOfInlineMenuPosition"]({ + top: 100, + left: 200, + width: 50, + height: 30, + }); + + expect(globalThis.document.elementFromPoint).toHaveBeenCalledWith(225, 115); + expect(result).toBe(mockElement); + }); }); describe("getOwnedTagNames", () => { @@ -975,6 +1261,25 @@ describe("AutofillInlineMenuContentService", () => { }); }); + describe("unobserveCustomElements", () => { + it("disconnects the inline menu elements mutation observer", () => { + const disconnectSpy = jest.spyOn( + autofillInlineMenuContentService["inlineMenuElementsMutationObserver"], + "disconnect", + ); + + autofillInlineMenuContentService["unobserveCustomElements"](); + + expect(disconnectSpy).toHaveBeenCalled(); + }); + + it("handles the case when the mutation observer is undefined", () => { + autofillInlineMenuContentService["inlineMenuElementsMutationObserver"] = undefined as any; + + expect(() => autofillInlineMenuContentService["unobserveCustomElements"]()).not.toThrow(); + }); + }); + describe("getPageIsOpaque", () => { it("returns false when no page elements exist", () => { jest.spyOn(globalThis.document, "querySelectorAll").mockReturnValue([] as any); diff --git a/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.ts b/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.ts index c2f872d7ba5..24e6f34df4b 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/content/autofill-inline-menu-content.service.ts @@ -41,7 +41,9 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte globalThis.navigator.userAgent.indexOf(" Firefox/") !== -1 || globalThis.navigator.userAgent.indexOf(" Gecko/") !== -1; private buttonElement?: HTMLElement; + private buttonIframe?: AutofillInlineMenuButtonIframe; private listElement?: HTMLElement; + private listIframe?: AutofillInlineMenuListIframe; private htmlMutationObserver: MutationObserver; private bodyMutationObserver: MutationObserver; private inlineMenuElementsMutationObserver: MutationObserver; @@ -264,18 +266,19 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte if (this.isFirefoxBrowser) { this.buttonElement = globalThis.document.createElement("div"); this.buttonElement.setAttribute("popover", "manual"); - new AutofillInlineMenuButtonIframe(this.buttonElement); + this.buttonIframe = new AutofillInlineMenuButtonIframe(this.buttonElement); return this.buttonElement; } const customElementName = this.generateRandomCustomElementName(); + const self = this; globalThis.customElements?.define( customElementName, class extends HTMLElement { constructor() { super(); - new AutofillInlineMenuButtonIframe(this); + self.buttonIframe = new AutofillInlineMenuButtonIframe(this); } }, ); @@ -293,18 +296,19 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte if (this.isFirefoxBrowser) { this.listElement = globalThis.document.createElement("div"); this.listElement.setAttribute("popover", "manual"); - new AutofillInlineMenuListIframe(this.listElement); + this.listIframe = new AutofillInlineMenuListIframe(this.listElement); return this.listElement; } const customElementName = this.generateRandomCustomElementName(); + const self = this; globalThis.customElements?.define( customElementName, class extends HTMLElement { constructor() { super(); - new AutofillInlineMenuListIframe(this); + self.listIframe = new AutofillInlineMenuListIframe(this); } }, ); @@ -778,5 +782,13 @@ export class AutofillInlineMenuContentService implements AutofillInlineMenuConte this.closeInlineMenu(); this.clearPersistentLastChildOverrideTimeout(); this.unobservePageAttributes(); + this.unobserveCustomElements(); + this.unobserveContainerElement(); + if (this.mutationObserverIterationsResetTimeout) { + clearTimeout(this.mutationObserverIterationsResetTimeout); + this.mutationObserverIterationsResetTimeout = null; + } + this.buttonIframe?.destroy(); + this.listIframe?.destroy(); } } diff --git a/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe-element.ts b/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe-element.ts index 3e2b364b17b..e26b6ba9ccc 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe-element.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe-element.ts @@ -1,6 +1,8 @@ import { AutofillInlineMenuIframeService } from "./autofill-inline-menu-iframe.service"; export class AutofillInlineMenuIframeElement { + private autofillInlineMenuIframeService: AutofillInlineMenuIframeService; + constructor( element: HTMLElement, portName: string, @@ -12,14 +14,14 @@ export class AutofillInlineMenuIframeElement { const shadow: ShadowRoot = element.attachShadow({ mode: "closed" }); shadow.prepend(style); - const autofillInlineMenuIframeService = new AutofillInlineMenuIframeService( + this.autofillInlineMenuIframeService = new AutofillInlineMenuIframeService( shadow, portName, initStyles, iframeTitle, ariaAlert, ); - autofillInlineMenuIframeService.initMenuIframe(); + this.autofillInlineMenuIframeService.initMenuIframe(); } /** @@ -67,4 +69,11 @@ export class AutofillInlineMenuIframeElement { return style; } + + /** + * Cleans up the iframe service to prevent memory leaks. + */ + destroy() { + this.autofillInlineMenuIframeService?.destroy(); + } } diff --git a/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.spec.ts b/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.spec.ts index f1ed6875f90..5e9d7c1da48 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.spec.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.spec.ts @@ -752,4 +752,164 @@ describe("AutofillInlineMenuIframeService", () => { expect(autofillInlineMenuIframeService["iframe"].title).toBe("title"); }); }); + + describe("destroy", () => { + beforeEach(() => { + autofillInlineMenuIframeService.initMenuIframe(); + autofillInlineMenuIframeService["iframe"].dispatchEvent(new Event(EVENTS.LOAD)); + portSpy = autofillInlineMenuIframeService["port"]; + }); + + it("removes the LOAD event listener from the iframe", () => { + const removeEventListenerSpy = jest.spyOn( + autofillInlineMenuIframeService["iframe"], + "removeEventListener", + ); + + autofillInlineMenuIframeService.destroy(); + + expect(removeEventListenerSpy).toHaveBeenCalledWith( + EVENTS.LOAD, + autofillInlineMenuIframeService["setupPortMessageListener"], + ); + }); + + it("clears the aria alert timeout", () => { + jest.spyOn(autofillInlineMenuIframeService, "clearAriaAlert"); + autofillInlineMenuIframeService["ariaAlertTimeout"] = setTimeout(jest.fn(), 1000); + + autofillInlineMenuIframeService.destroy(); + + expect(autofillInlineMenuIframeService.clearAriaAlert).toHaveBeenCalled(); + }); + + it("clears the fade in timeout", () => { + jest.useFakeTimers(); + jest.spyOn(globalThis, "clearTimeout"); + autofillInlineMenuIframeService["fadeInTimeout"] = setTimeout(jest.fn(), 1000); + + autofillInlineMenuIframeService.destroy(); + + expect(globalThis.clearTimeout).toHaveBeenCalled(); + expect(autofillInlineMenuIframeService["fadeInTimeout"]).toBeNull(); + }); + + it("clears the delayed close timeout", () => { + jest.useFakeTimers(); + jest.spyOn(globalThis, "clearTimeout"); + autofillInlineMenuIframeService["delayedCloseTimeout"] = setTimeout(jest.fn(), 1000); + + autofillInlineMenuIframeService.destroy(); + + expect(globalThis.clearTimeout).toHaveBeenCalled(); + expect(autofillInlineMenuIframeService["delayedCloseTimeout"]).toBeNull(); + }); + + it("clears the mutation observer iterations reset timeout", () => { + jest.useFakeTimers(); + jest.spyOn(globalThis, "clearTimeout"); + autofillInlineMenuIframeService["mutationObserverIterationsResetTimeout"] = setTimeout( + jest.fn(), + 1000, + ); + + autofillInlineMenuIframeService.destroy(); + + expect(globalThis.clearTimeout).toHaveBeenCalled(); + expect(autofillInlineMenuIframeService["mutationObserverIterationsResetTimeout"]).toBeNull(); + }); + + it("unobserves the iframe mutation observer", () => { + const disconnectSpy = jest.spyOn( + autofillInlineMenuIframeService["iframeMutationObserver"], + "disconnect", + ); + + autofillInlineMenuIframeService.destroy(); + + expect(disconnectSpy).toHaveBeenCalled(); + }); + + it("removes the port message listeners and disconnects the port", () => { + autofillInlineMenuIframeService.destroy(); + + expect(portSpy.onMessage.removeListener).toHaveBeenCalledWith(handlePortMessageSpy); + expect(portSpy.onDisconnect.removeListener).toHaveBeenCalledWith(handlePortDisconnectSpy); + expect(portSpy.disconnect).toHaveBeenCalled(); + expect(autofillInlineMenuIframeService["port"]).toBeNull(); + }); + + it("handles the case when the port is null", () => { + autofillInlineMenuIframeService["port"] = null; + + expect(() => autofillInlineMenuIframeService.destroy()).not.toThrow(); + }); + + it("handles the case when the iframe is undefined", () => { + autofillInlineMenuIframeService["iframe"] = undefined as any; + + expect(() => autofillInlineMenuIframeService.destroy()).not.toThrow(); + }); + }); + + describe("clearAriaAlert", () => { + it("clears the aria alert timeout when it exists", () => { + jest.useFakeTimers(); + jest.spyOn(globalThis, "clearTimeout"); + autofillInlineMenuIframeService["ariaAlertTimeout"] = setTimeout(jest.fn(), 1000); + + autofillInlineMenuIframeService.clearAriaAlert(); + + expect(globalThis.clearTimeout).toHaveBeenCalled(); + expect(autofillInlineMenuIframeService["ariaAlertTimeout"]).toBeNull(); + }); + + it("does nothing when the aria alert timeout is null", () => { + jest.spyOn(globalThis, "clearTimeout"); + autofillInlineMenuIframeService["ariaAlertTimeout"] = null; + + autofillInlineMenuIframeService.clearAriaAlert(); + + expect(globalThis.clearTimeout).not.toHaveBeenCalled(); + }); + }); + + describe("unobserveIframe", () => { + it("disconnects the iframe mutation observer", () => { + autofillInlineMenuIframeService.initMenuIframe(); + const disconnectSpy = jest.spyOn( + autofillInlineMenuIframeService["iframeMutationObserver"], + "disconnect", + ); + + autofillInlineMenuIframeService["unobserveIframe"](); + + expect(disconnectSpy).toHaveBeenCalled(); + }); + + it("handles the case when the mutation observer is undefined", () => { + autofillInlineMenuIframeService["iframeMutationObserver"] = undefined as any; + + expect(() => autofillInlineMenuIframeService["unobserveIframe"]()).not.toThrow(); + }); + }); + + describe("observeIframe", () => { + beforeEach(() => { + autofillInlineMenuIframeService.initMenuIframe(); + }); + + it("observes the iframe for attribute mutations", () => { + const observeSpy = jest.spyOn( + autofillInlineMenuIframeService["iframeMutationObserver"], + "observe", + ); + + autofillInlineMenuIframeService["observeIframe"](); + + expect(observeSpy).toHaveBeenCalledWith(autofillInlineMenuIframeService["iframe"], { + attributes: true, + }); + }); + }); }); diff --git a/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.ts b/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.ts index ad1241e98d2..40db2eef9fd 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.ts @@ -555,4 +555,26 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe return false; } + + /** + * Cleans up all event listeners, timeouts, and observers to prevent memory leaks. + */ + destroy() { + this.iframe?.removeEventListener(EVENTS.LOAD, this.setupPortMessageListener); + this.clearAriaAlert(); + this.clearFadeInTimeout(); + if (this.delayedCloseTimeout) { + clearTimeout(this.delayedCloseTimeout); + this.delayedCloseTimeout = null; + } + if (this.mutationObserverIterationsResetTimeout) { + clearTimeout(this.mutationObserverIterationsResetTimeout); + this.mutationObserverIterationsResetTimeout = null; + } + this.unobserveIframe(); + this.port?.onMessage.removeListener(this.handlePortMessage); + this.port?.onDisconnect.removeListener(this.handlePortDisconnect); + this.port?.disconnect(); + this.port = null; + } } diff --git a/apps/browser/src/vault/popup/components/at-risk-callout/at-risk-password-callout.component.ts b/apps/browser/src/vault/popup/components/at-risk-callout/at-risk-password-callout.component.ts index c37131b3ff1..f1614f800f2 100644 --- a/apps/browser/src/vault/popup/components/at-risk-callout/at-risk-password-callout.component.ts +++ b/apps/browser/src/vault/popup/components/at-risk-callout/at-risk-password-callout.component.ts @@ -6,7 +6,7 @@ import { firstValueFrom, switchMap } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; -import { AnchorLinkDirective, CalloutModule, BannerModule } from "@bitwarden/components"; +import { LinkComponent, CalloutModule, BannerModule } from "@bitwarden/components"; import { I18nPipe } from "@bitwarden/ui-common"; import { AtRiskPasswordCalloutData, AtRiskPasswordCalloutService } from "@bitwarden/vault"; @@ -15,7 +15,7 @@ import { AtRiskPasswordCalloutData, AtRiskPasswordCalloutService } from "@bitwar @Component({ selector: "vault-at-risk-password-callout", imports: [ - AnchorLinkDirective, + LinkComponent, CommonModule, RouterModule, CalloutModule, diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html index 20871b4b134..f0a6b0d6000 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html @@ -107,20 +107,32 @@ @if (vaultState === null) { @if (!(loading$ | async)) { - - - + + @if (hasSearchText$ | async) { + + } @else { + + + + + } } } diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts index d7824f3df58..a322fbc53dd 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.spec.ts @@ -44,6 +44,7 @@ import { VaultPopupAutofillService } from "../../services/vault-popup-autofill.s import { VaultPopupCopyButtonsService } from "../../services/vault-popup-copy-buttons.service"; import { VaultPopupItemsService } from "../../services/vault-popup-items.service"; import { VaultPopupListFiltersService } from "../../services/vault-popup-list-filters.service"; +import { VaultPopupLoadingService } from "../../services/vault-popup-loading.service"; import { VaultPopupScrollPositionService } from "../../services/vault-popup-scroll-position.service"; import { AtRiskPasswordCalloutComponent } from "../at-risk-callout/at-risk-password-callout.component"; @@ -174,15 +175,21 @@ describe("VaultV2Component", () => { showDeactivatedOrg$: new BehaviorSubject(false), favoriteCiphers$: new BehaviorSubject([]), remainingCiphers$: new BehaviorSubject([]), + filteredCiphers$: new BehaviorSubject([]), cipherCount$: new BehaviorSubject(0), - loading$: new BehaviorSubject(true), + hasSearchText$: new BehaviorSubject(false), } as Partial; - const filtersSvc = { + const filtersSvc: any = { allFilters$: new Subject(), filters$: new BehaviorSubject({}), filterVisibilityState$: new BehaviorSubject({}), - } as Partial; + numberOfAppliedFilters$: new BehaviorSubject(0), + }; + + const loadingSvc: any = { + loading$: new BehaviorSubject(false), + }; const activeAccount$ = new BehaviorSubject({ id: "user-1" }); @@ -240,6 +247,7 @@ describe("VaultV2Component", () => { provideNoopAnimations(), { provide: VaultPopupItemsService, useValue: itemsSvc }, { provide: VaultPopupListFiltersService, useValue: filtersSvc }, + { provide: VaultPopupLoadingService, useValue: loadingSvc }, { provide: VaultPopupScrollPositionService, useValue: scrollSvc }, { provide: AccountService, @@ -366,18 +374,18 @@ describe("VaultV2Component", () => { }); it("loading$ is true when items loading or filters missing; false when both ready", () => { - const itemsLoading$ = itemsSvc.loading$ as unknown as BehaviorSubject; + const vaultLoading$ = loadingSvc.loading$ as unknown as BehaviorSubject; const allFilters$ = filtersSvc.allFilters$ as unknown as Subject; const readySubject$ = component["readySubject"] as unknown as BehaviorSubject; const values: boolean[] = []; getObs(component, "loading$").subscribe((v) => values.push(!!v)); - itemsLoading$.next(true); + vaultLoading$.next(true); allFilters$.next({}); - itemsLoading$.next(false); + vaultLoading$.next(false); readySubject$.next(true); @@ -389,7 +397,7 @@ describe("VaultV2Component", () => { const component = fixture.componentInstance; const readySubject$ = component["readySubject"] as unknown as BehaviorSubject; - const itemsLoading$ = itemsSvc.loading$ as unknown as BehaviorSubject; + const vaultLoading$ = loadingSvc.loading$ as unknown as BehaviorSubject; const allFilters$ = filtersSvc.allFilters$ as unknown as Subject; fixture.detectChanges(); @@ -400,7 +408,7 @@ describe("VaultV2Component", () => { ) as HTMLElement; // Unblock loading - itemsLoading$.next(false); + vaultLoading$.next(false); readySubject$.next(true); allFilters$.next({}); tick(); @@ -607,6 +615,127 @@ describe("VaultV2Component", () => { expect(spotlights.length).toBe(0); })); + it("does not render app-autofill-vault-list-items or favorites item container when hasSearchText$ is true", () => { + itemsSvc.hasSearchText$.next(true); + + const fixture = TestBed.createComponent(VaultV2Component); + component = fixture.componentInstance; + + const readySubject$ = component["readySubject"]; + const allFilters$ = filtersSvc.allFilters$ as unknown as Subject; + + // Unblock loading + readySubject$.next(true); + allFilters$.next({}); + fixture.detectChanges(); + + const autofillElement = fixture.debugElement.query(By.css("app-autofill-vault-list-items")); + expect(autofillElement).toBeFalsy(); + + const favoritesElement = fixture.debugElement.query(By.css("#favorites")); + expect(favoritesElement).toBeFalsy(); + }); + + it("does render app-autofill-vault-list-items and favorites item container when hasSearchText$ is false", () => { + // Ensure vaultState is null (not Empty, NoResults, or DeactivatedOrg) + itemsSvc.emptyVault$.next(false); + itemsSvc.noFilteredResults$.next(false); + itemsSvc.showDeactivatedOrg$.next(false); + itemsSvc.hasSearchText$.next(false); + loadingSvc.loading$.next(false); + + const fixture = TestBed.createComponent(VaultV2Component); + component = fixture.componentInstance; + + const readySubject$ = component["readySubject"]; + const allFilters$ = filtersSvc.allFilters$ as unknown as Subject; + + // Unblock loading + readySubject$.next(true); + allFilters$.next({}); + fixture.detectChanges(); + + const autofillElement = fixture.debugElement.query(By.css("app-autofill-vault-list-items")); + expect(autofillElement).toBeTruthy(); + + const favoritesElement = fixture.debugElement.query(By.css("#favorites")); + expect(favoritesElement).toBeTruthy(); + }); + + it("does set the title for allItems container to allItems when hasSearchText$ and numberOfAppliedFilters$ are false and 0 respectively", () => { + // Ensure vaultState is null (not Empty, NoResults, or DeactivatedOrg) + itemsSvc.emptyVault$.next(false); + itemsSvc.noFilteredResults$.next(false); + itemsSvc.showDeactivatedOrg$.next(false); + itemsSvc.hasSearchText$.next(false); + filtersSvc.numberOfAppliedFilters$.next(0); + loadingSvc.loading$.next(false); + + const fixture = TestBed.createComponent(VaultV2Component); + component = fixture.componentInstance; + + const readySubject$ = component["readySubject"]; + const allFilters$ = filtersSvc.allFilters$ as unknown as Subject; + + // Unblock loading + readySubject$.next(true); + allFilters$.next({}); + fixture.detectChanges(); + + const allItemsElement = fixture.debugElement.query(By.css("#allItems")); + const allItemsTitle = allItemsElement.componentInstance.title(); + expect(allItemsTitle).toBe("allItems"); + }); + + it("does set the title for allItems container to searchResults when hasSearchText$ is true", () => { + // Ensure vaultState is null (not Empty, NoResults, or DeactivatedOrg) + itemsSvc.emptyVault$.next(false); + itemsSvc.noFilteredResults$.next(false); + itemsSvc.showDeactivatedOrg$.next(false); + itemsSvc.hasSearchText$.next(true); + loadingSvc.loading$.next(false); + + const fixture = TestBed.createComponent(VaultV2Component); + component = fixture.componentInstance; + + const readySubject$ = component["readySubject"]; + const allFilters$ = filtersSvc.allFilters$ as unknown as Subject; + + // Unblock loading + readySubject$.next(true); + allFilters$.next({}); + fixture.detectChanges(); + + const allItemsElement = fixture.debugElement.query(By.css("#allItems")); + const allItemsTitle = allItemsElement.componentInstance.title(); + expect(allItemsTitle).toBe("searchResults"); + }); + + it("does set the title for allItems container to items when numberOfAppliedFilters$ is > 0", fakeAsync(() => { + // Ensure vaultState is null (not Empty, NoResults, or DeactivatedOrg) + itemsSvc.emptyVault$.next(false); + itemsSvc.noFilteredResults$.next(false); + itemsSvc.showDeactivatedOrg$.next(false); + itemsSvc.hasSearchText$.next(false); + filtersSvc.numberOfAppliedFilters$.next(1); + loadingSvc.loading$.next(false); + + const fixture = TestBed.createComponent(VaultV2Component); + component = fixture.componentInstance; + + const readySubject$ = component["readySubject"]; + const allFilters$ = filtersSvc.allFilters$ as unknown as Subject; + + // Unblock loading + readySubject$.next(true); + allFilters$.next({}); + fixture.detectChanges(); + + const allItemsElement = fixture.debugElement.query(By.css("#allItems")); + const allItemsTitle = allItemsElement.componentInstance.title(); + expect(allItemsTitle).toBe("items"); + })); + describe("AutoConfirmExtensionSetupDialog", () => { beforeEach(() => { autoConfirmDialogSpy.mockClear(); diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts index 51e735fb1ef..fce084542a9 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts @@ -160,6 +160,11 @@ export class VaultV2Component implements OnInit, OnDestroy { FeatureFlag.BrowserPremiumSpotlight, ); + protected readonly hasSearchText$ = this.vaultPopupItemsService.hasSearchText$; + protected readonly numberOfAppliedFilters$ = + this.vaultPopupListFiltersService.numberOfAppliedFilters$; + + protected filteredCiphers$ = this.vaultPopupItemsService.filteredCiphers$; protected favoriteCiphers$ = this.vaultPopupItemsService.favoriteCiphers$; protected remainingCiphers$ = this.vaultPopupItemsService.remainingCiphers$; protected allFilters$ = this.vaultPopupListFiltersService.allFilters$; diff --git a/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts index 7cd73279c3d..093fdbfb66d 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts @@ -323,6 +323,25 @@ describe("VaultPopupItemsService", () => { }); }); + describe("filteredCiphers$", () => { + it("should filter filteredCipher$ down to search term", (done) => { + const cipherList = Object.values(allCiphers); + const searchText = "Login"; + + searchService.searchCiphers.mockImplementation(async () => { + return cipherList.filter((cipher) => { + return cipher.name.includes(searchText); + }); + }); + + service.filteredCiphers$.subscribe((ciphers) => { + // There are 10 ciphers but only 3 with "Login" in the name + expect(ciphers.length).toBe(3); + done(); + }); + }); + }); + describe("favoriteCiphers$", () => { it("should exclude autofill ciphers", (done) => { service.favoriteCiphers$.subscribe((ciphers) => { diff --git a/apps/browser/src/vault/popup/services/vault-popup-items.service.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts index 321d7936806..7ccfc834c87 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-items.service.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts @@ -201,6 +201,15 @@ export class VaultPopupItemsService { shareReplay({ refCount: true, bufferSize: 1 }), ); + /** + * List of ciphers that are filtered using filters and search. + * Includes favorite ciphers and ciphers currently suggested for autofill. + * Ciphers are sorted by name. + */ + filteredCiphers$: Observable = this._filteredCipherList$.pipe( + shareReplay({ refCount: false, bufferSize: 1 }), + ); + /** * List of ciphers that can be used for autofill on the current tab. Includes cards and/or identities * if enabled in the vault settings. Ciphers are sorted by type, then by last used date, then by name. diff --git a/apps/browser/src/vault/popup/settings/archive.component.html b/apps/browser/src/vault/popup/settings/archive.component.html index 01ac799ba29..5024a22ff16 100644 --- a/apps/browser/src/vault/popup/settings/archive.component.html +++ b/apps/browser/src/vault/popup/settings/archive.component.html @@ -74,9 +74,11 @@ - + @if (userHasPremium$ | async) { + + } @if (canAssignCollections$ | async) { } + @if (desktopMigrationMilestone1()) { + + + + } diff --git a/apps/desktop/src/vault/app/vault/vault-items-v2.component.ts b/apps/desktop/src/vault/app/vault/vault-items-v2.component.ts index a6582f6de58..50f00025238 100644 --- a/apps/desktop/src/vault/app/vault/vault-items-v2.component.ts +++ b/apps/desktop/src/vault/app/vault/vault-items-v2.component.ts @@ -1,12 +1,13 @@ import { ScrollingModule } from "@angular/cdk/scrolling"; import { CommonModule } from "@angular/common"; -import { Component, input } from "@angular/core"; -import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; +import { Component, input, output } from "@angular/core"; +import { takeUntilDestroyed, toSignal } from "@angular/core/rxjs-interop"; import { distinctUntilChanged, debounceTime } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { VaultItemsComponent as BaseVaultItemsComponent } from "@bitwarden/angular/vault/components/vault-items.component"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { uuidAsString } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; @@ -32,6 +33,12 @@ import { SearchBarService } from "../../../app/layout/search/search-bar.service" export class VaultItemsV2Component extends BaseVaultItemsComponent { readonly showPremiumCallout = input(false); + readonly onAddFolder = output(); + + protected readonly desktopMigrationMilestone1 = toSignal( + this.configService.getFeatureFlag$(FeatureFlag.DesktopUiMigrationMilestone1), + ); + protected CipherViewLikeUtils = CipherViewLikeUtils; constructor( diff --git a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.spec.ts b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.spec.ts index 276c0c2e6a3..08931c68900 100644 --- a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.spec.ts +++ b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.spec.ts @@ -363,7 +363,7 @@ describe("VaultItemDialogComponent", () => { }); it("refocuses the dialog header", async () => { - const focusOnHeaderSpy = jest.spyOn(component["dialogComponent"](), "focusOnHeader"); + const focusOnHeaderSpy = jest.spyOn(component["dialogComponent"](), "handleAutofocus"); await component["changeMode"]("view"); diff --git a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts index ef861b7cab3..df73aacfdde 100644 --- a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts +++ b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.ts @@ -692,7 +692,7 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy { this.dialogContent().nativeElement.parentElement.scrollTop = 0; // Refocus on title element, the built-in focus management of the dialog only works for the initial open. - this.dialogComponent().focusOnHeader(); + this.dialogComponent().handleAutofocus(); // Update the URL query params to reflect the new mode. await this.router.navigate([], { diff --git a/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.html b/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.html index 88719b93643..62b23fc580d 100644 --- a/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.html +++ b/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.html @@ -46,17 +46,21 @@ bitMenuItem (click)="unlinkSso(organization)" > + {{ "unlinkSso" | i18n }} diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 8d5801cb5bf..0e2c11054fb 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -38,6 +38,9 @@ "accessIntelligence": { "message": "Access Intelligence" }, + "noApplicationsMatchTheseFilters": { + "message": "No applications match these filters" + }, "passwordRisk": { "message": "Password Risk" }, diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.html b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.html index 7c4f6d04a6b..2fa9fabf73d 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.html +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.html @@ -3,12 +3,10 @@ } @else { @let drawerDetails = dataService.drawerDetails$ | async;
-

{{ "allApplications" | i18n }}

-
@@ -20,7 +18,8 @@ (ngModelChange)="setFilterApplicationsByStatus($event)" fullWidth="false" class="tw-min-w-48" - > + > +
} diff --git a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.ts b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.ts index 8962980c872..8cd0c2640f5 100644 --- a/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.ts +++ b/bitwarden_license/bit-web/src/app/dirt/access-intelligence/all-applications/applications.component.ts @@ -96,12 +96,15 @@ export class ApplicationsComponent implements OnInit { { label: this.i18nService.t("critical", this.criticalApplicationsCount()), value: ApplicationFilterOption.Critical, + icon: " ", }, { label: this.i18nService.t("notCritical", this.nonCriticalApplicationsCount()), value: ApplicationFilterOption.NonCritical, + icon: " ", }, ]); + protected readonly emptyTableExplanation = signal(""); constructor( protected i18nService: I18nService, @@ -162,6 +165,12 @@ export class ApplicationsComponent implements OnInit { this.dataSource.filter = (app) => filterFunction(app) && app.applicationName.toLowerCase().includes(searchText.toLowerCase()); + + if (this.dataSource?.filteredData?.length === 0) { + this.emptyTableExplanation.set(this.i18nService.t("noApplicationsMatchTheseFilters")); + } else { + this.emptyTableExplanation.set(""); + } }); } diff --git a/libs/angular/src/vault/components/vault-items.component.ts b/libs/angular/src/vault/components/vault-items.component.ts index c4fe2741e11..6058955788e 100644 --- a/libs/angular/src/vault/components/vault-items.component.ts +++ b/libs/angular/src/vault/components/vault-items.component.ts @@ -94,7 +94,7 @@ export class VaultItemsComponent implements OnDestroy protected cipherService: CipherService, protected accountService: AccountService, protected restrictedItemTypesService: RestrictedItemTypesService, - private configService: ConfigService, + protected configService: ConfigService, ) { this.subscribeToCiphers(); diff --git a/libs/components/src/button/button.component.html b/libs/components/src/button/button.component.html index 26e0c3b4d3d..d8718340217 100644 --- a/libs/components/src/button/button.component.html +++ b/libs/components/src/button/button.component.html @@ -1,6 +1,14 @@ - - - + + + @if (startIcon()) { + + } +
+ +
+ @if (endIcon()) { + + } @if (showLoadingStyle()) { diff --git a/libs/components/src/button/button.component.ts b/libs/components/src/button/button.component.ts index 7cae8fe974d..1055d134e53 100644 --- a/libs/components/src/button/button.component.ts +++ b/libs/components/src/button/button.component.ts @@ -1,4 +1,4 @@ -import { NgClass } from "@angular/common"; +import { NgClass, NgTemplateOutlet } from "@angular/common"; import { input, HostBinding, @@ -14,6 +14,7 @@ import { debounce, interval } from "rxjs"; import { AriaDisableDirective } from "../a11y"; import { ButtonLikeAbstraction, ButtonType, ButtonSize } from "../shared/button-like.abstraction"; +import { BitwardenIcon } from "../shared/icon"; import { SpinnerComponent } from "../spinner"; import { ariaDisableElement } from "../utils"; @@ -71,7 +72,7 @@ const buttonStyles: Record = { selector: "button[bitButton], a[bitButton]", templateUrl: "button.component.html", providers: [{ provide: ButtonLikeAbstraction, useExisting: ButtonComponent }], - imports: [NgClass, SpinnerComponent], + imports: [NgClass, NgTemplateOutlet, SpinnerComponent], hostDirectives: [AriaDisableDirective], }) export class ButtonComponent implements ButtonLikeAbstraction { @@ -125,12 +126,23 @@ export class ButtonComponent implements ButtonLikeAbstraction { readonly buttonType = input("secondary"); + readonly startIcon = input(undefined); + + readonly endIcon = input(undefined); + readonly size = input("default"); readonly block = input(false, { transform: booleanAttribute }); readonly loading = model(false); + readonly startIconClasses = computed(() => { + return ["bwi", this.startIcon()]; + }); + + readonly endIconClasses = computed(() => { + return ["bwi", this.endIcon()]; + }); /** * Determine whether it is appropriate to display a loading spinner. We only want to show * a spinner if it's been more than 75 ms since the `loading` state began. This prevents diff --git a/libs/components/src/button/button.stories.ts b/libs/components/src/button/button.stories.ts index 24c263f240a..9e8d23611ff 100644 --- a/libs/components/src/button/button.stories.ts +++ b/libs/components/src/button/button.stories.ts @@ -152,15 +152,13 @@ export const WithIcon: Story = { template: /*html*/ `
-
-
diff --git a/libs/components/src/callout/callout.stories.ts b/libs/components/src/callout/callout.stories.ts index ff1a8c16d5f..fb1a2d67a40 100644 --- a/libs/components/src/callout/callout.stories.ts +++ b/libs/components/src/callout/callout.stories.ts @@ -113,7 +113,7 @@ export const WithTextButton: Story = { template: ` (args)}>

The content of the callout

- Visit the help center + Visit the help center `, }), diff --git a/libs/components/src/card/base-card/base-card.stories.ts b/libs/components/src/card/base-card/base-card.stories.ts index bae07dd1468..98814c1f9f4 100644 --- a/libs/components/src/card/base-card/base-card.stories.ts +++ b/libs/components/src/card/base-card/base-card.stories.ts @@ -1,6 +1,6 @@ import { Meta, StoryObj, moduleMetadata } from "@storybook/angular"; -import { AnchorLinkDirective } from "../../link"; +import { LinkComponent } from "../../link"; import { TypographyModule } from "../../typography"; import { BaseCardComponent } from "./base-card.component"; @@ -10,7 +10,7 @@ export default { component: BaseCardComponent, decorators: [ moduleMetadata({ - imports: [AnchorLinkDirective, TypographyModule], + imports: [LinkComponent, TypographyModule], }), ], parameters: { diff --git a/libs/components/src/dialog/dialog/dialog.component.ts b/libs/components/src/dialog/dialog/dialog.component.ts index 2ce19a9f9e0..63fbb69399d 100644 --- a/libs/components/src/dialog/dialog/dialog.component.ts +++ b/libs/components/src/dialog/dialog/dialog.component.ts @@ -12,9 +12,10 @@ import { computed, signal, AfterViewInit, + NgZone, } from "@angular/core"; import { toObservable } from "@angular/core/rxjs-interop"; -import { combineLatest, switchMap } from "rxjs"; +import { combineLatest, firstValueFrom, switchMap } from "rxjs"; import { I18nPipe } from "@bitwarden/ui-common"; @@ -65,6 +66,9 @@ const drawerSizeToWidth = { }) export class DialogComponent implements AfterViewInit { private readonly destroyRef = inject(DestroyRef); + private readonly ngZone = inject(NgZone); + private readonly el = inject(ElementRef); + private readonly dialogHeader = viewChild.required>("dialogHeader"); private readonly scrollableBody = viewChild.required(CdkScrollable); @@ -144,10 +148,6 @@ export class DialogComponent implements AfterViewInit { return [...baseClasses, this.width(), ...sizeClasses, ...animationClasses]; }); - ngAfterViewInit() { - this.focusOnHeader(); - } - handleEsc(event: Event) { if (!this.dialogRef?.disableClose) { this.dialogRef?.close(); @@ -159,24 +159,54 @@ export class DialogComponent implements AfterViewInit { this.animationCompleted.set(true); } - /** - * Moves focus to the dialog header element. - * This is done automatically when the dialog is opened but can be called manually - * when the contents of the dialog change and focus should be reset. - */ - focusOnHeader(): void { + async ngAfterViewInit() { /** - * Wait a tick for any focus management to occur on the trigger element before moving focus to - * the dialog header. We choose the dialog header because it is always present, unlike possible - * interactive elements. - * - * We are doing this manually instead of using `cdkTrapFocusAutoCapture` and `cdkFocusInitial` - * because we need this delay behavior. + * Wait for the zone to stabilize before performing any focus behaviors. This ensures that all + * child elements are rendered and stable. */ - const headerFocusTimeout = setTimeout(() => { - this.dialogHeader().nativeElement.focus(); - }, 0); + if (this.ngZone.isStable) { + this.handleAutofocus(); + } else { + await firstValueFrom(this.ngZone.onStable); + this.handleAutofocus(); + } + } - this.destroyRef.onDestroy(() => clearTimeout(headerFocusTimeout)); + /** + * Ensure that the user's focus is in the dialog by autofocusing the appropriate element. + * + * If there is a descendant of the dialog with the AutofocusDirective applied, we defer to that. + * If not, we want to fallback to a default behavior of focusing the dialog's header element. We + * choose the dialog header as the default fallback for dialog focus because it is always present, + * unlike possible interactive elements. + */ + handleAutofocus() { + /** + * Angular's contentChildren query cannot see into the internal templates of child components. + * We need to use a regular DOM query instead to see if there are descendants using the + * AutofocusDirective. + */ + const dialogRef = this.el.nativeElement; + // Must match selectors of AutofocusDirective + const autofocusDescendants = dialogRef.querySelectorAll("[appAutofocus], [bitAutofocus]"); + const hasAutofocusDescendants = autofocusDescendants.length > 0; + + if (!hasAutofocusDescendants) { + /** + * Wait a tick for any focus management to occur on the trigger element before moving focus + * to the dialog header. + * + * We are doing this manually instead of using Angular's built-in focus management + * directives (`cdkTrapFocusAutoCapture` and `cdkFocusInitial`) because we need this delay + * behavior. + * + * And yes, we need the timeout even though we are already waiting for ngZone to stabilize. + */ + const headerFocusTimeout = setTimeout(() => { + this.dialogHeader().nativeElement.focus(); + }, 0); + + this.destroyRef.onDestroy(() => clearTimeout(headerFocusTimeout)); + } } } diff --git a/libs/components/src/input/autofocus.directive.ts b/libs/components/src/input/autofocus.directive.ts index a4791a51f01..bffac8eb757 100644 --- a/libs/components/src/input/autofocus.directive.ts +++ b/libs/components/src/input/autofocus.directive.ts @@ -22,6 +22,8 @@ import { FocusableElement } from "../shared/focusable-element"; * * If the component provides the `FocusableElement` interface, the `focus` * method will be called. Otherwise, the native element will be focused. + * + * If selector changes, `dialog.component.ts` must also be updated */ @Directive({ selector: "[appAutofocus], [bitAutofocus]", diff --git a/libs/components/src/layout/layout.component.ts b/libs/components/src/layout/layout.component.ts index da30b76a9f0..c71c4e73c6e 100644 --- a/libs/components/src/layout/layout.component.ts +++ b/libs/components/src/layout/layout.component.ts @@ -5,7 +5,7 @@ import { booleanAttribute, Component, ElementRef, inject, input, viewChild } fro import { RouterModule } from "@angular/router"; import { DrawerService } from "../dialog/drawer.service"; -import { LinkModule } from "../link"; +import { LinkComponent, LinkModule } from "../link"; import { SideNavService } from "../navigation/side-nav.service"; import { SharedModule } from "../shared"; @@ -52,11 +52,11 @@ export class LayoutComponent { * * @see https://github.com/angular/components/issues/10247#issuecomment-384060265 **/ - private readonly skipLink = viewChild.required>("skipLink"); + private readonly skipLink = viewChild.required("skipLink"); handleKeydown(ev: KeyboardEvent) { if (isNothingFocused()) { ev.preventDefault(); - this.skipLink().nativeElement.focus(); + this.skipLink().el.nativeElement.focus(); } } } diff --git a/libs/components/src/link/index.ts b/libs/components/src/link/index.ts index 480f5396de7..08617e813f5 100644 --- a/libs/components/src/link/index.ts +++ b/libs/components/src/link/index.ts @@ -1,2 +1,2 @@ -export * from "./link.directive"; +export * from "./link.component"; export * from "./link.module"; diff --git a/libs/components/src/link/link.component.html b/libs/components/src/link/link.component.html new file mode 100644 index 00000000000..810b65db519 --- /dev/null +++ b/libs/components/src/link/link.component.html @@ -0,0 +1,11 @@ +
+ @if (startIcon()) { + + } + + + + @if (endIcon()) { + + } +
diff --git a/libs/components/src/link/link.directive.ts b/libs/components/src/link/link.component.ts similarity index 59% rename from libs/components/src/link/link.directive.ts rename to libs/components/src/link/link.component.ts index 62f0d8b878f..d826a4633a9 100644 --- a/libs/components/src/link/link.directive.ts +++ b/libs/components/src/link/link.component.ts @@ -1,6 +1,14 @@ -import { input, HostBinding, Directive, inject, ElementRef, booleanAttribute } from "@angular/core"; +import { + ChangeDetectionStrategy, + Component, + computed, + input, + booleanAttribute, + inject, + ElementRef, +} from "@angular/core"; -import { AriaDisableDirective } from "../a11y"; +import { BitwardenIcon } from "../shared/icon"; import { ariaDisableElement } from "../utils"; export const LinkTypes = [ @@ -46,16 +54,16 @@ const commonStyles = [ "tw-transition", "tw-no-underline", "tw-cursor-pointer", - "hover:tw-underline", - "hover:tw-decoration-1", + "[&:hover_span]:tw-underline", + "[&.tw-test-hover_span]:tw-underline", + "[&:hover_span]:tw-decoration-[.125em]", + "[&.tw-test-hover_span]:tw-decoration-[.125em]", "disabled:tw-no-underline", "disabled:tw-cursor-not-allowed", "disabled:!tw-text-fg-disabled", "disabled:hover:!tw-text-fg-disabled", "disabled:hover:tw-no-underline", "focus-visible:tw-outline-none", - "focus-visible:tw-underline", - "focus-visible:tw-decoration-1", "focus-visible:before:tw-ring-border-focus", // Workaround for html button tag not being able to be set to `display: inline` @@ -72,8 +80,12 @@ const commonStyles = [ "before:tw-block", "before:tw-absolute", "before:-tw-inset-x-[0.1em]", + "before:-tw-inset-y-[0]", "before:tw-rounded-md", "before:tw-transition", + "before:tw-h-full", + "before:tw-w-[calc(100%_+_.25rem)]", + "before:tw-pointer-events-none", "focus-visible:before:tw-ring-2", "focus-visible:tw-z-10", "aria-disabled:tw-no-underline", @@ -83,47 +95,57 @@ const commonStyles = [ "aria-disabled:hover:tw-no-underline", ]; -@Directive() -abstract class LinkDirective { - readonly linkType = input("default"); -} - -/** - * Text Links and Buttons can use either the `` or `
-
-
@@ -203,7 +201,7 @@ export const Buttons: Story = { }, }; -export const Anchors: StoryObj = { +export const Anchors: StoryObj = { render: (args) => ({ props: { linkType: args.linkType, @@ -220,14 +218,12 @@ export const Anchors: StoryObj = { Anchor
- - + Add Icon Anchor
- - + Chevron Icon Anchor
@@ -247,20 +243,57 @@ export const Inline: Story = { props: args, template: /*html*/ ` - On the internet paragraphs often contain inline links, but few know that can be used for similar purposes. + On the internet paragraphs often contain inline links with very long text that might break, but few know that can be used for similar purposes. `, }), }; -export const Inactive: Story = { +export const WithIcons: Story = { render: (args) => ({ props: args, template: /*html*/ ` - - -
- +
+
+ Start icon link +
+
+ External link +
+
+ Both icons +
+
+ +
+
+ +
+
+ +
+
+ `, + }), + args: { + linkType: "primary", + }, +}; + +export const Inactive: Story = { + render: (args) => ({ + props: { + ...args, + onClick: () => { + alert("Button clicked! (This should not appear when disabled)"); + }, + }, + template: /*html*/ ` + + Links can not be inactive + +
+
`, }), diff --git a/libs/vault/src/cipher-form/components/autofill-options/advanced-uri-option-dialog.component.ts b/libs/vault/src/cipher-form/components/autofill-options/advanced-uri-option-dialog.component.ts index 3580b1fada8..04545730172 100644 --- a/libs/vault/src/cipher-form/components/autofill-options/advanced-uri-option-dialog.component.ts +++ b/libs/vault/src/cipher-form/components/autofill-options/advanced-uri-option-dialog.component.ts @@ -3,13 +3,13 @@ import { Component, inject } from "@angular/core"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { - ButtonLinkDirective, ButtonModule, + CenterPositionStrategy, DialogModule, + DialogRef, DialogService, DIALOG_DATA, - DialogRef, - CenterPositionStrategy, + LinkComponent, } from "@bitwarden/components"; export type AdvancedUriOptionDialogParams = { @@ -22,7 +22,7 @@ export type AdvancedUriOptionDialogParams = { // eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @Component({ templateUrl: "advanced-uri-option-dialog.component.html", - imports: [ButtonLinkDirective, ButtonModule, DialogModule, JslibModule], + imports: [LinkComponent, ButtonModule, DialogModule, JslibModule], }) export class AdvancedUriOptionDialogComponent { constructor(private dialogRef: DialogRef) {} diff --git a/libs/vault/src/cipher-form/services/default-cipher-form.service.ts b/libs/vault/src/cipher-form/services/default-cipher-form.service.ts index 8566e51d74f..63a9b3091e2 100644 --- a/libs/vault/src/cipher-form/services/default-cipher-form.service.ts +++ b/libs/vault/src/cipher-form/services/default-cipher-form.service.ts @@ -76,6 +76,9 @@ export class DefaultCipherFormService implements CipherFormService { .then((res) => res.cipher); } else { // Updating a cipher with collection changes is not supported with a single request currently + // Save the new collectionIds before overwriting + const newCollectionIdsToSave = cipher.collectionIds; + // First update the cipher with the original collectionIds cipher.collectionIds = config.originalCipher.collectionIds; const newCipher = await this.cipherService.updateWithServer( @@ -86,7 +89,7 @@ export class DefaultCipherFormService implements CipherFormService { ); // Then save the new collection changes separately - newCipher.collectionIds = cipher.collectionIds; + newCipher.collectionIds = newCollectionIdsToSave; // TODO: Remove after migrating all SDK ops const { cipher: encryptedCipher } = await this.cipherService.encrypt(newCipher, activeUserId); diff --git a/libs/vault/src/cipher-view/cipher-view.component.html b/libs/vault/src/cipher-view/cipher-view.component.html index 3d0cc4c4414..05d2ecede72 100644 --- a/libs/vault/src/cipher-view/cipher-view.component.html +++ b/libs/vault/src/cipher-view/cipher-view.component.html @@ -12,9 +12,15 @@ - + {{ "changeAtRiskPassword" | i18n }} - diff --git a/libs/vault/src/cipher-view/cipher-view.component.ts b/libs/vault/src/cipher-view/cipher-view.component.ts index 26e3f18b542..24713d3f612 100644 --- a/libs/vault/src/cipher-view/cipher-view.component.ts +++ b/libs/vault/src/cipher-view/cipher-view.component.ts @@ -30,7 +30,7 @@ import { CalloutModule, SearchModule, TypographyModule, - AnchorLinkDirective, + LinkComponent, } from "@bitwarden/components"; import { ChangeLoginPasswordService } from "../abstractions/change-login-password.service"; @@ -66,7 +66,7 @@ import { ViewIdentitySectionsComponent } from "./view-identity-sections/view-ide ViewIdentitySectionsComponent, LoginCredentialsViewComponent, AutofillOptionsViewComponent, - AnchorLinkDirective, + LinkComponent, TypographyModule, ], }) diff --git a/libs/vault/src/cipher-view/item-details/item-details-v2.component.ts b/libs/vault/src/cipher-view/item-details/item-details-v2.component.ts index eb0e468fa4f..73e7c2706be 100644 --- a/libs/vault/src/cipher-view/item-details/item-details-v2.component.ts +++ b/libs/vault/src/cipher-view/item-details/item-details-v2.component.ts @@ -19,9 +19,9 @@ import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; import { BadgeModule, - ButtonLinkDirective, CardComponent, FormFieldModule, + LinkComponent, TypographyModule, } from "@bitwarden/components"; @@ -39,7 +39,7 @@ import { OrgIconDirective } from "../../components/org-icon.directive"; TypographyModule, OrgIconDirective, FormFieldModule, - ButtonLinkDirective, + LinkComponent, BadgeModule, ], }) diff --git a/libs/vault/src/components/decryption-failure-dialog/decryption-failure-dialog.component.ts b/libs/vault/src/components/decryption-failure-dialog/decryption-failure-dialog.component.ts index 6b1a0e0d8aa..e829c003c5a 100644 --- a/libs/vault/src/components/decryption-failure-dialog/decryption-failure-dialog.component.ts +++ b/libs/vault/src/components/decryption-failure-dialog/decryption-failure-dialog.component.ts @@ -7,7 +7,7 @@ import { CipherId } from "@bitwarden/common/types/guid"; import { DIALOG_DATA, DialogRef, - AnchorLinkDirective, + LinkComponent, AsyncActionsModule, ButtonModule, DialogModule, @@ -32,7 +32,7 @@ export type DecryptionFailureDialogParams = { JslibModule, AsyncActionsModule, ButtonModule, - AnchorLinkDirective, + LinkComponent, ], }) export class DecryptionFailureDialogComponent { diff --git a/package-lock.json b/package-lock.json index 55873bdb40c..6352675d718 100644 --- a/package-lock.json +++ b/package-lock.json @@ -38,7 +38,7 @@ "bufferutil": "4.1.0", "chalk": "4.1.2", "commander": "14.0.0", - "core-js": "3.47.0", + "core-js": "3.48.0", "form-data": "4.0.4", "https-proxy-agent": "7.0.6", "inquirer": "8.2.6", @@ -205,7 +205,7 @@ "browser-hrtime": "1.1.8", "chalk": "4.1.2", "commander": "14.0.0", - "core-js": "3.47.0", + "core-js": "3.48.0", "form-data": "4.0.4", "https-proxy-agent": "7.0.6", "inquirer": "8.2.6", @@ -20879,9 +20879,9 @@ } }, "node_modules/core-js": { - "version": "3.47.0", - "resolved": "https://registry.npmjs.org/core-js/-/core-js-3.47.0.tgz", - "integrity": "sha512-c3Q2VVkGAUyupsjRnaNX6u8Dq2vAdzm9iuPj5FW0fRxzlxgq9Q39MDq10IvmQSpLgHQNyQzQmOo6bgGHmH3NNg==", + "version": "3.48.0", + "resolved": "https://registry.npmjs.org/core-js/-/core-js-3.48.0.tgz", + "integrity": "sha512-zpEHTy1fjTMZCKLHUZoVeylt9XrzaIN2rbPXEt0k+q7JE5CkCZdo6bNq55bn24a69CH7ErAVLKijxJja4fw+UQ==", "hasInstallScript": true, "license": "MIT", "funding": { diff --git a/package.json b/package.json index 1a72c49d263..2cc60c08c9d 100644 --- a/package.json +++ b/package.json @@ -177,7 +177,7 @@ "bufferutil": "4.1.0", "chalk": "4.1.2", "commander": "14.0.0", - "core-js": "3.47.0", + "core-js": "3.48.0", "form-data": "4.0.4", "https-proxy-agent": "7.0.6", "inquirer": "8.2.6",