From 41b948ae05e7643a6ead8ff4823af874e8128892 Mon Sep 17 00:00:00 2001 From: Misha Holtz <36575242+mnholtz@users.noreply.github.com> Date: Thu, 3 Oct 2024 13:17:43 -0700 Subject: [PATCH] #9222 Add policy for hiding the sidebar logo (#9224) * add hideSidebarLogo to manatgedStorageSchema * override showSidebarLogo in useTheme * add unit tests to useTheme * fix header button alignment when missing sidebar logo * refactor rename hideSidebarLogo -> showSidebarLogo * refactor combine test cases * add add custom theme to test cases * deck version number in code comment * make test cases complete --- src/hooks/useTheme.test.ts | 77 +++++++++++++++++++-- src/hooks/useTheme.ts | 26 +++++-- src/sidebar/Header.tsx | 15 ++-- src/store/enterprise/managedStorageTypes.ts | 5 ++ static/managedStorageSchema.json | 4 ++ 5 files changed, 107 insertions(+), 20 deletions(-) diff --git a/src/hooks/useTheme.test.ts b/src/hooks/useTheme.test.ts index 2b8cb20689..0ad8ccce25 100644 --- a/src/hooks/useTheme.test.ts +++ b/src/hooks/useTheme.test.ts @@ -22,21 +22,43 @@ 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"); + +const customTheme = { + themeName: "custom", + showSidebarLogo: true, + customSidebarLogo: "https://example.com/custom-logo.png", + toolbarIcon: "https://example.com/custom-icon.svg", + logo: { + regular: "https://example.com/custom-logo-regular.png", + small: "https://example.com/custom-logo-small.png", + }, + lastFetched: Date.now(), +}; 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 +79,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 +87,57 @@ 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.each([ + { policyValue: true, themeValue: true, expectedValue: true }, + { policyValue: true, themeValue: false, expectedValue: true }, + { policyValue: false, themeValue: true, expectedValue: false }, + { policyValue: false, themeValue: false, expectedValue: false }, + { policyValue: undefined, themeValue: true, expectedValue: true }, + { policyValue: undefined, themeValue: false, expectedValue: false }, + ])( + "handles showSidebarLogo policy (policy: $policyValue, theme: $themeValue, expected: $expectedValue)", + async ({ policyValue, themeValue, expectedValue }) => { + jest.mocked(useAsyncExternalStore).mockReturnValue({ + data: { ...customTheme, showSidebarLogo: themeValue }, + isLoading: false, + } as AsyncState); + jest.mocked(readManagedStorageByKey).mockResolvedValue(policyValue); + + const { result, waitForNextUpdate } = renderHook(() => useTheme()); + + await waitForNextUpdate(); + + expect(result.current.activeTheme.showSidebarLogo).toBe(expectedValue); + }, + ); + + it("uses activeTheme when an error occurs in managed storage", async () => { + jest.mocked(useAsyncExternalStore).mockReturnValue({ + data: customTheme, + isLoading: false, + } as AsyncState); + + jest + .mocked(readManagedStorageByKey) + .mockRejectedValue(new Error("Managed storage error")); + + const { result, waitForNextUpdate } = renderHook(() => useTheme()); + + await waitForNextUpdate(); + + expect(result.current.activeTheme.showSidebarLogo).toBe( + customTheme.showSidebarLogo, + ); + }); }); diff --git a/src/hooks/useTheme.ts b/src/hooks/useTheme.ts index 9d81018f5c..b6f827279a 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: showSidebarLogoOverride, isLoading: isManagedStorageLoading } = + useAsyncState(async () => readManagedStorageByKey("showSidebarLogo"), []); + + const isLoading = isManagedStorageLoading || isCachedThemeLoading; useEffect(() => { if ( @@ -70,7 +75,18 @@ function useTheme(): { activeTheme: ThemeAssets; isLoading: boolean } { setThemeFavicon(activeTheme.themeName); }, [activeTheme, cachedTheme, isLoading]); - return { activeTheme, isLoading }; + return { + activeTheme: { + ...activeTheme, + // There is a managed storage policy that overrides the sidebar logo visibility specified by the team theme + // See managedStorageSchema.json + showSidebarLogo: + showSidebarLogoOverride == null + ? activeTheme.showSidebarLogo + : showSidebarLogoOverride, + }, + isLoading, + }; } export default useTheme; diff --git a/src/sidebar/Header.tsx b/src/sidebar/Header.tsx index 1b83cc830e..4a3df4586e 100644 --- a/src/sidebar/Header.tsx +++ b/src/sidebar/Header.tsx @@ -34,7 +34,7 @@ function reloadSidebar() { const Header: React.FunctionComponent = () => { const { activeTheme: { logo, showSidebarLogo, customSidebarLogo, themeName }, - isLoading, + isLoading: isThemeLoading, } = useTheme(); const { flagOn } = useFlags(); @@ -47,23 +47,18 @@ const Header: React.FunctionComponent = () => { [styles.themeColor || ""]: themeName !== DEFAULT_THEME, }); - if (isLoading) { - return null; - } - return (
- {showSidebarLogo && ( - // `mx-auto` centers the logo -
+
+ {!isThemeLoading && showSidebarLogo && ( {customSidebarLogo -
- )} + )} +
{showDeveloperUI && (