fix(web): prevent settings sidebar from closing on first click (#1709)
* fix(web): prevent settings sidebar from closing on first click * test(web): clarify settings sidebar race assertion
This commit is contained in:
@@ -8,6 +8,9 @@ const navigationMock = vi.hoisted(() => ({
|
||||
const officeRouteMock = vi.hoisted(() => ({
|
||||
inOffice: false,
|
||||
}));
|
||||
const footerMock = vi.hoisted(() => ({
|
||||
onLayout: null as (() => void) | null,
|
||||
}));
|
||||
|
||||
// The AppSidebar pulls in a lot of children that touch the dockview / kanban
|
||||
// data layer. For unit testing the collapse + section toggle behaviour we stub
|
||||
@@ -56,9 +59,16 @@ vi.mock("./sections/office-navigation-section", () => ({
|
||||
<div data-testid={`office-navigation-section-${section ?? "all"}`} />
|
||||
),
|
||||
}));
|
||||
vi.mock("./app-sidebar-footer", () => ({
|
||||
AppSidebarFooter: () => <div data-testid="footer" />,
|
||||
}));
|
||||
vi.mock("./app-sidebar-footer", async () => {
|
||||
const { useLayoutEffect } = await vi.importActual<typeof import("react")>("react");
|
||||
|
||||
return {
|
||||
AppSidebarFooter: () => {
|
||||
useLayoutEffect(() => footerMock.onLayout?.(), []);
|
||||
return <div data-testid="footer" />;
|
||||
},
|
||||
};
|
||||
});
|
||||
vi.mock("./app-sidebar-settings-mode", () => ({
|
||||
AppSidebarSettingsMode: () => <div data-testid="settings-mode" />,
|
||||
}));
|
||||
@@ -95,6 +105,7 @@ const storeState = {
|
||||
toggleAppSidebarSection: vi.fn(),
|
||||
setAppSidebarWidth: vi.fn(),
|
||||
toggleAppSidebarSettingsMode: vi.fn(),
|
||||
setAppSidebarSettingsMode: vi.fn((_settingsMode: boolean) => {}),
|
||||
};
|
||||
|
||||
vi.mock("@/components/state-provider", () => ({
|
||||
@@ -112,6 +123,8 @@ describe("AppSidebar", () => {
|
||||
storeState.toggleAppSidebar = vi.fn();
|
||||
storeState.toggleAppSidebarSection = vi.fn();
|
||||
storeState.toggleAppSidebarSettingsMode = vi.fn();
|
||||
storeState.setAppSidebarSettingsMode = vi.fn((_settingsMode: boolean) => {});
|
||||
footerMock.onLayout = null;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -180,8 +193,28 @@ describe("AppSidebar", () => {
|
||||
render(<AppSidebar />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(storeState.toggleAppSidebarSettingsMode).toHaveBeenCalledOnce();
|
||||
expect(storeState.setAppSidebarSettingsMode).toHaveBeenCalledWith(true);
|
||||
});
|
||||
expect(storeState.toggleAppSidebarSettingsMode).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps settings open when the gear is clicked before route synchronization", async () => {
|
||||
navigationMock.pathname = "/settings";
|
||||
storeState.toggleAppSidebarSettingsMode = vi.fn(() => {
|
||||
storeState.appSidebar.settingsMode = !storeState.appSidebar.settingsMode;
|
||||
});
|
||||
storeState.setAppSidebarSettingsMode = vi.fn((settingsMode: boolean) => {
|
||||
storeState.appSidebar.settingsMode = settingsMode;
|
||||
});
|
||||
footerMock.onLayout = () => storeState.toggleAppSidebarSettingsMode();
|
||||
|
||||
render(<AppSidebar />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(storeState.toggleAppSidebarSettingsMode).toHaveBeenCalledOnce();
|
||||
expect(storeState.setAppSidebarSettingsMode).toHaveBeenCalledOnce();
|
||||
});
|
||||
expect(storeState.appSidebar.settingsMode).toBe(true);
|
||||
});
|
||||
|
||||
it("exits settings mode when navigating from a settings route to a non-settings route", async () => {
|
||||
@@ -194,7 +227,8 @@ describe("AppSidebar", () => {
|
||||
rerender(<AppSidebar />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(storeState.toggleAppSidebarSettingsMode).toHaveBeenCalledOnce();
|
||||
expect(storeState.setAppSidebarSettingsMode).toHaveBeenCalledWith(false);
|
||||
});
|
||||
expect(storeState.toggleAppSidebarSettingsMode).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -103,7 +103,7 @@ export function AppSidebar() {
|
||||
const storedWidth = useAppStore((s) => s.appSidebar.width);
|
||||
const toggleSection = useAppStore((s) => s.toggleAppSidebarSection);
|
||||
const toggleCollapsed = useAppStore((s) => s.toggleAppSidebar);
|
||||
const toggleSettingsMode = useAppStore((s) => s.toggleAppSidebarSettingsMode);
|
||||
const setSettingsMode = useAppStore((s) => s.setAppSidebarSettingsMode);
|
||||
const setWidth = useAppStore((s) => s.setAppSidebarWidth);
|
||||
const pathname = usePathname();
|
||||
const inOffice = useInOffice();
|
||||
@@ -150,13 +150,13 @@ export function AppSidebar() {
|
||||
if (!pathname || prevPathnameRef.current === pathname) return;
|
||||
prevPathnameRef.current = pathname;
|
||||
if (isSettingsRoute(pathname)) {
|
||||
if (!settingsMode) toggleSettingsMode();
|
||||
if (!settingsMode) setSettingsMode(true);
|
||||
return;
|
||||
}
|
||||
if (settingsMode) {
|
||||
toggleSettingsMode();
|
||||
setSettingsMode(false);
|
||||
}
|
||||
}, [pathname, settingsMode, toggleSettingsMode]);
|
||||
}, [pathname, settingsMode, setSettingsMode]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!pathname) return;
|
||||
|
||||
@@ -60,6 +60,14 @@ export function buildAppSidebarActions(set: ImmerSet) {
|
||||
draft.appSidebar.width = width;
|
||||
setStoredAppSidebarWidth(width);
|
||||
}),
|
||||
setAppSidebarSettingsMode: (settingsMode: boolean) =>
|
||||
set((draft) => {
|
||||
draft.appSidebar.settingsMode = settingsMode;
|
||||
if (settingsMode && draft.appSidebar.collapsed) {
|
||||
draft.appSidebar.collapsed = false;
|
||||
setStoredAppSidebarCollapsed(false);
|
||||
}
|
||||
}),
|
||||
toggleAppSidebarSettingsMode: () =>
|
||||
set((draft) => {
|
||||
const next = !draft.appSidebar.settingsMode;
|
||||
|
||||
@@ -252,6 +252,7 @@ export type UISliceActions = {
|
||||
setAppSidebarCollapsed: (collapsed: boolean) => void;
|
||||
toggleAppSidebarSection: (sectionId: string, defaultExpanded?: boolean) => void;
|
||||
setAppSidebarWidth: (width: number) => void;
|
||||
setAppSidebarSettingsMode: (settingsMode: boolean) => void;
|
||||
toggleAppSidebarSettingsMode: () => void;
|
||||
/** Record multiple sidebar badge acknowledgements with one localStorage merge. */
|
||||
acknowledgeAgentErrors: (stamps: Record<string, string>) => void;
|
||||
|
||||
@@ -419,6 +419,23 @@ describe("appSidebar actions", () => {
|
||||
expect(store.getState().appSidebar.settingsMode).toBe(false);
|
||||
});
|
||||
|
||||
it("setAppSidebarSettingsMode applies the requested state idempotently", () => {
|
||||
window.localStorage.setItem(COLLAPSED_KEY, JSON.stringify(true));
|
||||
const store = makeStore();
|
||||
|
||||
store.getState().setAppSidebarSettingsMode(true);
|
||||
store.getState().setAppSidebarSettingsMode(true);
|
||||
expect(store.getState().appSidebar.settingsMode).toBe(true);
|
||||
expect(store.getState().appSidebar.collapsed).toBe(false);
|
||||
expect(JSON.parse(window.localStorage.getItem(COLLAPSED_KEY) ?? "null")).toBe(false);
|
||||
|
||||
store.getState().setAppSidebarSettingsMode(false);
|
||||
store.getState().setAppSidebarSettingsMode(false);
|
||||
expect(store.getState().appSidebar.settingsMode).toBe(false);
|
||||
expect(store.getState().appSidebar.collapsed).toBe(false);
|
||||
expect(window.localStorage.getItem("kandev.appSidebar.settingsMode")).toBeNull();
|
||||
});
|
||||
|
||||
it("entering settings mode while collapsed force-expands the rail", () => {
|
||||
window.localStorage.setItem(COLLAPSED_KEY, JSON.stringify(true));
|
||||
const store = makeStore();
|
||||
|
||||
@@ -542,6 +542,7 @@ export type AppState = {
|
||||
setAppSidebarCollapsed: UIA["setAppSidebarCollapsed"];
|
||||
toggleAppSidebarSection: UIA["toggleAppSidebarSection"];
|
||||
setAppSidebarWidth: UIA["setAppSidebarWidth"];
|
||||
setAppSidebarSettingsMode: UIA["setAppSidebarSettingsMode"];
|
||||
toggleAppSidebarSettingsMode: UIA["toggleAppSidebarSettingsMode"];
|
||||
acknowledgeAgentErrors: UIA["acknowledgeAgentErrors"];
|
||||
dismissAgentError: UIA["dismissAgentError"];
|
||||
|
||||
Reference in New Issue
Block a user