From 0ee3c37ee81a29d98dbc9ee9986447807d3fe1b7 Mon Sep 17 00:00:00 2001 From: Misha Holtz <36575242+mnholtz@users.noreply.github.com> Date: Wed, 2 Oct 2024 13:17:07 -0700 Subject: [PATCH 01/25] add hideSidebarLogo to manatgedStorageSchema --- static/managedStorageSchema.json | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/static/managedStorageSchema.json b/static/managedStorageSchema.json index 390c014296..3ee5a86c47 100644 --- a/static/managedStorageSchema.json +++ b/static/managedStorageSchema.json @@ -49,6 +49,11 @@ "type": "boolean", "description": "Disable the browser warning for non-Chrome browsers, e.g., Microsoft Edge", "default": false + }, + "hideSidebarLogo": { + "type": "boolean", + "description": "If set, hide the logo on the PixieBrix sidebar, overriding the team theme.", + "default": false } } } From 2bcc02eb7a06caaeba8f634bc18737ac57aff2dc Mon Sep 17 00:00:00 2001 From: Misha Holtz <36575242+mnholtz@users.noreply.github.com> Date: Wed, 2 Oct 2024 14:16:01 -0700 Subject: [PATCH 02/25] override showSidebarLogo in useTheme --- src/hooks/useTheme.ts | 23 ++++++++++++++++----- src/store/enterprise/managedStorageTypes.ts | 5 +++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/hooks/useTheme.ts b/src/hooks/useTheme.ts index 9d81018f5c..63479c96f1 100644 --- a/src/hooks/useTheme.ts +++ b/src/hooks/useTheme.ts @@ -25,6 +25,8 @@ import { import { initialTheme } from "@/themes/themeStore"; import useAsyncExternalStore from "@/hooks/useAsyncExternalStore"; import { activateTheme } from "@/background/messenger/api"; +import useAsyncState from "@/hooks/useAsyncState"; +import { readManagedStorageByKey } from "@/store/enterprise/managedStorage"; const themeStorageSubscribe = (callback: () => void) => { const abortController = new AbortController(); @@ -42,10 +44,13 @@ const themeStorageSubscribe = (callback: () => void) => { function useTheme(): { activeTheme: ThemeAssets; isLoading: boolean } { // The active theme is fetched with `getActiveTheme` in the background script and cached in the themeStorage, // This hook subscribes to changes in themeStorage to retrieve the latest current activeTheme - const { data: cachedTheme, isLoading } = useAsyncExternalStore( - themeStorageSubscribe, - themeStorage.get, - ); + const { data: cachedTheme, isLoading: isCachedThemeLoading } = + useAsyncExternalStore(themeStorageSubscribe, themeStorage.get); + + const { data: hideSidebarLogo, isLoading: isManagedStorageLoading } = + useAsyncState(async () => readManagedStorageByKey("hideSidebarLogo"), []); + + const isLoading = isManagedStorageLoading || isCachedThemeLoading; useEffect(() => { if ( @@ -70,7 +75,15 @@ function useTheme(): { activeTheme: ThemeAssets; isLoading: boolean } { setThemeFavicon(activeTheme.themeName); }, [activeTheme, cachedTheme, isLoading]); - return { activeTheme, isLoading }; + return { + activeTheme: { + ...activeTheme, + // There is a managed storage policy to hide the sidebar logo, overriding the team theme + // See managedStorageSchema.json + showSidebarLogo: hideSidebarLogo ? false : activeTheme.showSidebarLogo, + }, + isLoading, + }; } export default useTheme; diff --git a/src/store/enterprise/managedStorageTypes.ts b/src/store/enterprise/managedStorageTypes.ts index 85cd62d776..0d94ad5f56 100644 --- a/src/store/enterprise/managedStorageTypes.ts +++ b/src/store/enterprise/managedStorageTypes.ts @@ -73,4 +73,9 @@ export type ManagedStorageState = { * @since 1.7.36 */ disableBrowserWarning?: boolean; + /** + * Hides the logo in the PixieBrix sidebar, overriding the team theme. + * @since 2.1.4 + */ + hideSidebarLogo?: boolean; }; From 5d60f92278b9820fb7244b5896e96f68a4cf60e3 Mon Sep 17 00:00:00 2001 From: Misha Holtz <36575242+mnholtz@users.noreply.github.com> Date: Wed, 2 Oct 2024 14:40:30 -0700 Subject: [PATCH 03/25] add unit tests to useTheme --- src/hooks/useTheme.test.ts | 71 +++++++++++++++++++++++++++++++++++--- 1 file changed, 66 insertions(+), 5 deletions(-) diff --git a/src/hooks/useTheme.test.ts b/src/hooks/useTheme.test.ts index 2b8cb20689..6fe6a26e50 100644 --- a/src/hooks/useTheme.test.ts +++ b/src/hooks/useTheme.test.ts @@ -22,21 +22,31 @@ import { initialTheme } from "@/themes/themeStore"; import { type AsyncState } from "@/types/sliceTypes"; import { themeStorage } from "@/themes/themeUtils"; import { activateTheme } from "@/background/messenger/api"; +import { readManagedStorageByKey } from "@/store/enterprise/managedStorage"; afterEach(() => { jest.clearAllMocks(); }); jest.mock("@/hooks/useAsyncExternalStore"); +jest.mock("@/background/messenger/api"); +jest.mock("@/store/enterprise/managedStorage"); describe("useTheme", () => { - beforeEach(() => { + beforeEach(async () => { jest .mocked(useAsyncExternalStore) .mockReturnValue({ data: initialTheme, isLoading: false } as AsyncState); + // eslint-disable-next-line no-restricted-syntax -- this func requires a parameter + jest.mocked(readManagedStorageByKey).mockResolvedValue(undefined); }); + test("calls useAsyncExternalStore and gets current theme state", async () => { - const { result: themeResult } = renderHook(() => useTheme()); + const { result: themeResult, waitForNextUpdate } = renderHook(() => + useTheme(), + ); + + await waitForNextUpdate(); expect(useAsyncExternalStore).toHaveBeenNthCalledWith( 1, @@ -57,7 +67,7 @@ describe("useTheme", () => { }); }); - it("calls activateTheme after loading is done and it hasn't been called recently", () => { + it("calls activateTheme after loading is done and it hasn't been called recently", async () => { jest.useFakeTimers(); jest.mocked(useAsyncExternalStore).mockReturnValue({ @@ -65,12 +75,63 @@ describe("useTheme", () => { isLoading: false, } as AsyncState); - renderHook(() => useTheme()); + let result = renderHook(() => useTheme()); + await result.waitForNextUpdate(); expect(activateTheme).not.toHaveBeenCalled(); jest.advanceTimersByTime(125_000); - renderHook(() => useTheme()); + result = renderHook(() => useTheme()); + await result.waitForNextUpdate(); expect(activateTheme).toHaveBeenCalledOnce(); }); + + it("overrides showSidebarLogo when hideSidebarLogo is true in managed storage", async () => { + jest.mocked(readManagedStorageByKey).mockResolvedValue(true); + + const { result, waitForNextUpdate } = renderHook(() => useTheme()); + + await waitForNextUpdate(); + + expect(result.current.activeTheme.showSidebarLogo).toBe(false); + }); + + it("uses default activeTheme when hideSidebarLogo is false in managed storage", async () => { + jest.mocked(readManagedStorageByKey).mockResolvedValue(false); + + const { result, waitForNextUpdate } = renderHook(() => useTheme()); + + await waitForNextUpdate(); + + expect(result.current.activeTheme.showSidebarLogo).toBe( + initialTheme.showSidebarLogo, + ); + }); + + it("uses default activeTheme when hideSidebarLogo is undefined in managed storage", async () => { + // eslint-disable-next-line no-restricted-syntax -- this func requires a parameter + jest.mocked(readManagedStorageByKey).mockResolvedValue(undefined); + + const { result, waitForNextUpdate } = renderHook(() => useTheme()); + + await waitForNextUpdate(); + + expect(result.current.activeTheme.showSidebarLogo).toBe( + initialTheme.showSidebarLogo, + ); + }); + + it("uses default activeTheme when an error occurs while fetching from managed storage", async () => { + jest + .mocked(readManagedStorageByKey) + .mockRejectedValue(new Error("Managed storage error")); + + const { result, waitForNextUpdate } = renderHook(() => useTheme()); + + await waitForNextUpdate(); + + expect(result.current.activeTheme.showSidebarLogo).toBe( + initialTheme.showSidebarLogo, + ); + }); }); From 7b1e90e0a960d151fd9bba53780cb296735953a5 Mon Sep 17 00:00:00 2001 From: Misha Holtz <36575242+mnholtz@users.noreply.github.com> Date: Wed, 2 Oct 2024 15:06:33 -0700 Subject: [PATCH 04/25] fix header button alignment when missing sidebar logo --- src/sidebar/Header.tsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/sidebar/Header.tsx b/src/sidebar/Header.tsx index 1b83cc830e..01c986c9fb 100644 --- a/src/sidebar/Header.tsx +++ b/src/sidebar/Header.tsx @@ -53,17 +53,16 @@ const Header: React.FunctionComponent = () => { return (
- {showSidebarLogo && ( - // `mx-auto` centers the logo -
+
+ {showSidebarLogo && ( {customSidebarLogo -
- )} + )} +
{showDeveloperUI && (