Skip to content

Commit

Permalink
#9222 Add policy for hiding the sidebar logo (#9224)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
mnholtz authored Oct 3, 2024
1 parent cf3b817 commit 41b948a
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 20 deletions.
77 changes: 72 additions & 5 deletions src/hooks/useTheme.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -57,20 +79,65 @@ 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({
data: { ...initialTheme, lastFetched: Date.now() },
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,
);
});
});
26 changes: 21 additions & 5 deletions src/hooks/useTheme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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 (
Expand All @@ -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;
15 changes: 5 additions & 10 deletions src/sidebar/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function reloadSidebar() {
const Header: React.FunctionComponent = () => {
const {
activeTheme: { logo, showSidebarLogo, customSidebarLogo, themeName },
isLoading,
isLoading: isThemeLoading,
} = useTheme();

const { flagOn } = useFlags();
Expand All @@ -47,23 +47,18 @@ const Header: React.FunctionComponent = () => {
[styles.themeColor || ""]: themeName !== DEFAULT_THEME,
});

if (isLoading) {
return null;
}

return (
<div className="d-flex py-2 pl-2 pr-0 align-items-center">
{showSidebarLogo && (
// `mx-auto` centers the logo
<div className="mx-auto">
<div className="mx-auto">
{!isThemeLoading && showSidebarLogo && (
<img
src={customSidebarLogo ?? logo.regular}
alt={customSidebarLogo ? "Custom logo" : "PixieBrix logo"}
className={styles.logo}
data-testid="sidebarHeaderLogo"
/>
</div>
)}
)}
</div>
{showDeveloperUI && (
<Button
type="button"
Expand Down
5 changes: 5 additions & 0 deletions src/store/enterprise/managedStorageTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,9 @@ export type ManagedStorageState = {
* @since 1.7.36
*/
disableBrowserWarning?: boolean;
/**
* Controls the visibility of the PixieBrix sidebar logo. Overrides the team theme, if one exists.
* @since 2.1.3
*/
showSidebarLogo?: boolean;
};
4 changes: 4 additions & 0 deletions static/managedStorageSchema.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@
"type": "boolean",
"description": "Disable the browser warning for non-Chrome browsers, e.g., Microsoft Edge",
"default": false
},
"showSidebarLogo": {
"type": "boolean",
"description": "Controls the visibility the PixieBrix sidebar logo. Overrides the team theme, if one exists."
}
}
}

0 comments on commit 41b948a

Please sign in to comment.