Skip to content

Commit a6e9939

Browse files
improve collapse by adding persistence and trigger on certain actions (#1196)
## User-Facing Changes - Layout Browser sections ("Personal" and "Organization") now remember their collapsed/expanded state across sessions. - Expanding the Personal section is automatically triggered when creating a new layout or making a personal copy of a shared layout. - Expanding the Organization section is automatically triggered when sharing a layout. ## Description The `LayoutSection` component previously managed its own collapsed/expanded state via local `useState`, which was lost on re-render or navigation. This PR lifts that state up to persisted app configuration (`AppSetting`) so it survives across sessions. Two new settings keys were added: - `LAYOUT_SECTION_PERSONAL_EXPANDED` (`ui.layout-section-personal-expanded`) - `LAYOUT_SECTION_SHARED_EXPANDED` (`ui.layout-section-shared-expanded`) The `LayoutBrowser` now reads and writes these values via `useAppConfigurationValue`, passing `expanded` and `onToggleExpanded` down to each `LayoutSection`. Additionally, relevant actions (create layout, share layout, make personal copy) automatically expand the corresponding section so the newly added layout is immediately visible. ## Checklist - [ ] The web version was tested and it is running ok - [ ] The desktop version was tested and it is running ok - [x] This change is covered by unit tests (`LayoutSection.test.tsx` updated) - [x] Files constants.ts, types.ts and *.style.ts have been checked and relevant code snippets have been relocated (`AppSetting.ts` extended; no style changes required) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * “Personal” and “Organization” layout sections now remember their expanded/collapsed state and persist it across refreshes and app restarts. * **Bug Fixes** * After creating, sharing, or copying a layout, the correct section now reliably opens automatically. * Clicking a section header now consistently triggers a single toggle, keeping the UI and stored expansion state in sync. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 269e9eb commit a6e9939

9 files changed

Lines changed: 705 additions & 28 deletions

File tree

‎packages/suite-base/src/components/LayoutBrowser/LayoutSection.test.tsx‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -128,14 +128,12 @@ describe("LayoutSection", () => {
128128
expect(screen.getByTestId("layout-row-3").dataset.selected).toBe("false");
129129
});
130130

131-
it("collapses the list when the section header is clicked", async () => {
131+
it("collapses the list when expanded is false", async () => {
132132
// GIVEN
133133
const title = BasicBuilder.string();
134-
const user = userEvent.setup();
135-
render(<LayoutSection {...defaultProps} title={title} />);
136134

137135
// WHEN
138-
await user.click(screen.getByTestId(`layout-section-header-${title}`));
136+
render(<LayoutSection {...defaultProps} title={title} expanded={false} />);
139137

140138
// THEN
141139
await waitFor(() => {
@@ -145,16 +143,27 @@ describe("LayoutSection", () => {
145143
expect(screen.queryByTestId("layout-row-3")).not.toBeInTheDocument();
146144
});
147145

148-
it("expands the list when the collapsed section header is clicked again", async () => {
146+
it("calls onToggleExpanded when the section header is clicked", async () => {
149147
// GIVEN
150148
const title = BasicBuilder.string();
149+
const onToggleExpanded = jest.fn();
151150
const user = userEvent.setup();
152-
render(<LayoutSection {...defaultProps} title={title} />);
151+
render(<LayoutSection {...defaultProps} title={title} onToggleExpanded={onToggleExpanded} />);
153152

154-
// WHEN - collapse then expand
155-
await user.click(screen.getByTestId(`layout-section-header-${title}`));
153+
// WHEN
156154
await user.click(screen.getByTestId(`layout-section-header-${title}`));
157155

156+
// THEN
157+
expect(onToggleExpanded).toHaveBeenCalledTimes(1);
158+
});
159+
160+
it("shows items when expanded is true", () => {
161+
// GIVEN
162+
const title = BasicBuilder.string();
163+
164+
// WHEN
165+
render(<LayoutSection {...defaultProps} title={title} expanded={true} />);
166+
158167
// THEN
159168
expect(screen.getByTestId("layout-row-1")).toBeInTheDocument();
160169
expect(screen.getByTestId("layout-row-2")).toBeInTheDocument();

‎packages/suite-base/src/components/LayoutBrowser/LayoutSection.tsx‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
import ArrowDropDownIcon from "@mui/icons-material/ArrowDropDown";
99
import { ButtonBase, Collapse, Typography, List } from "@mui/material";
10-
import { MouseEvent, useCallback, useState } from "react";
10+
import { MouseEvent } from "react";
1111

1212
import Stack from "@lichtblick/suite-base/components/Stack";
1313
import { Layout } from "@lichtblick/suite-base/services/ILayoutStorage";
@@ -18,11 +18,13 @@ import { useLayoutSectionStyles } from "./LayoutSection.style";
1818
export default function LayoutSection({
1919
title,
2020
disablePadding = false,
21+
expanded = true,
2122
emptyText,
2223
items,
2324
anySelectedModifiedLayouts,
2425
multiSelectedIds,
2526
selectedId,
27+
onToggleExpanded,
2628
onSelect,
2729
onRename,
2830
onDuplicate,
@@ -35,11 +37,13 @@ export default function LayoutSection({
3537
}: Readonly<{
3638
title: string | undefined;
3739
disablePadding?: boolean;
40+
expanded?: boolean;
3841
emptyText: string | undefined;
3942
items: readonly Layout[] | undefined;
4043
anySelectedModifiedLayouts: boolean;
4144
multiSelectedIds: readonly string[];
4245
selectedId?: string;
46+
onToggleExpanded?: () => void;
4347
onSelect: (item: Layout, params?: { selectedViaClick?: boolean; event?: MouseEvent }) => void;
4448
onRename: (item: Layout, newName: string) => void;
4549
onDuplicate: (item: Layout) => void;
@@ -51,11 +55,6 @@ export default function LayoutSection({
5155
onMakePersonalCopy: (item: Layout) => void;
5256
}>): React.JSX.Element {
5357
const { classes, cx } = useLayoutSectionStyles();
54-
const [expanded, setExpanded] = useState(true);
55-
56-
const toggleExpanded = useCallback(() => {
57-
setExpanded((prev) => !prev);
58-
}, []);
5958

6059
const isCollapsible = title != undefined;
6160

@@ -64,7 +63,7 @@ export default function LayoutSection({
6463
{title != undefined && (
6564
<ButtonBase
6665
className={classes.sectionHeader}
67-
onClick={toggleExpanded}
66+
onClick={onToggleExpanded}
6867
disableRipple
6968
data-testid={`layout-section-header-${title}`}
7069
>

‎packages/suite-base/src/components/LayoutBrowser/index.test.tsx‎

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,13 @@ import {
1515
} from "@lichtblick/suite-base/context/CurrentLayoutContext";
1616
import { useCurrentUser } from "@lichtblick/suite-base/context/CurrentUserContext";
1717
import { useLayoutManager } from "@lichtblick/suite-base/context/LayoutManagerContext";
18+
import { useWorkspaceStore } from "@lichtblick/suite-base/context/Workspace/WorkspaceContext";
19+
import { useWorkspaceActions } from "@lichtblick/suite-base/context/Workspace/useWorkspaceActions";
1820
import { useAppConfigurationValue } from "@lichtblick/suite-base/hooks/useAppConfigurationValue";
1921
import { useConfirm } from "@lichtblick/suite-base/hooks/useConfirm";
2022
import { useLayoutNavigation } from "@lichtblick/suite-base/hooks/useLayoutNavigation";
2123
import { usePrompt } from "@lichtblick/suite-base/hooks/usePrompt";
24+
import { Layout } from "@lichtblick/suite-base/services/ILayoutStorage";
2225
import MockLayoutManager from "@lichtblick/suite-base/services/LayoutManager/MockLayoutManager";
2326
import LayoutBuilder from "@lichtblick/suite-base/testing/builders/LayoutBuilder";
2427
import { BasicBuilder } from "@lichtblick/test-builders";
@@ -62,6 +65,17 @@ jest.mock("@lichtblick/suite-base/hooks/useAppConfigurationValue", () => ({
6265
useAppConfigurationValue: jest.fn(),
6366
}));
6467

68+
jest.mock("@lichtblick/suite-base/context/Workspace/WorkspaceContext", () => ({
69+
useWorkspaceStore: jest.fn(),
70+
WorkspaceStoreSelectors: {
71+
selectLayoutSectionExpanded: jest.fn(),
72+
},
73+
}));
74+
75+
jest.mock("@lichtblick/suite-base/context/Workspace/useWorkspaceActions", () => ({
76+
useWorkspaceActions: jest.fn(),
77+
}));
78+
6579
jest.mock("@lichtblick/suite-base/hooks/useLayoutTransfer", () => ({
6680
useLayoutTransfer: jest.fn().mockReturnValue({
6781
importLayout: jest.fn(),
@@ -115,6 +129,13 @@ describe("LayoutBrowser", () => {
115129
(useConfirm as jest.Mock).mockReturnValue([jest.fn(), undefined]);
116130
(usePrompt as jest.Mock).mockReturnValue([jest.fn(), undefined]);
117131
(useAppConfigurationValue as jest.Mock).mockReturnValue([true, jest.fn()]);
132+
(useWorkspaceStore as jest.Mock).mockReturnValue({ personal: true, shared: true });
133+
(useWorkspaceActions as jest.Mock).mockReturnValue({
134+
layoutBrowserActions: {
135+
setPersonalSectionExpanded: jest.fn(),
136+
setSharedSectionExpanded: jest.fn(),
137+
},
138+
});
118139
(useLayoutNavigation as jest.Mock).mockReturnValue({
119140
onSelectLayout: jest.fn(),
120141
state: {
@@ -251,4 +272,180 @@ describe("LayoutBrowser", () => {
251272
expect(dispatchMock).toHaveBeenCalledWith({ type: "clear-multi-action" });
252273
});
253274
});
275+
276+
describe("section collapse persistence", () => {
277+
let setPersonalExpandedMock: jest.Mock;
278+
let setSharedExpandedMock: jest.Mock;
279+
let onSelectLayoutMock: jest.Mock;
280+
let logEventMock: jest.Mock;
281+
282+
const originalLayoutSectionMock = jest.requireMock("./LayoutSection").default;
283+
284+
beforeEach(() => {
285+
setPersonalExpandedMock = jest.fn();
286+
setSharedExpandedMock = jest.fn();
287+
onSelectLayoutMock = jest.fn().mockResolvedValue(undefined);
288+
logEventMock = jest.fn().mockResolvedValue(undefined);
289+
290+
(useAnalytics as jest.Mock).mockReturnValue({ logEvent: logEventMock });
291+
(useWorkspaceStore as jest.Mock).mockReturnValue({ personal: true, shared: true });
292+
(useWorkspaceActions as jest.Mock).mockReturnValue({
293+
layoutBrowserActions: {
294+
setPersonalSectionExpanded: setPersonalExpandedMock,
295+
setSharedSectionExpanded: setSharedExpandedMock,
296+
},
297+
});
298+
(useLayoutNavigation as jest.Mock).mockReturnValue({
299+
onSelectLayout: onSelectLayoutMock,
300+
state: {
301+
busy: false,
302+
error: undefined,
303+
online: true,
304+
lastSelectedId: undefined,
305+
multiAction: undefined,
306+
selectedIds: [],
307+
},
308+
dispatch: dispatchMock,
309+
});
310+
});
311+
312+
afterEach(() => {
313+
jest.requireMock("./LayoutSection").default = originalLayoutSectionMock;
314+
});
315+
316+
it("passes expanded state and toggle handlers to LayoutSection", () => {
317+
// GIVEN
318+
(useWorkspaceStore as jest.Mock).mockReturnValue({ personal: false, shared: true });
319+
320+
const capturedProps: Record<string, unknown>[] = [];
321+
jest.requireMock("./LayoutSection").default = jest
322+
.fn()
323+
.mockImplementation((props: Record<string, unknown>) => {
324+
capturedProps.push(props);
325+
return <div data-testid="layout-section" />;
326+
});
327+
328+
// WHEN
329+
render(<LayoutBrowser />);
330+
331+
// THEN
332+
expect(capturedProps[0]?.expanded).toBe(false);
333+
expect(capturedProps[0]?.onToggleExpanded).toBeDefined();
334+
});
335+
336+
it("calls setPersonalSectionExpanded with toggler when togglePersonalExpanded is invoked", () => {
337+
// GIVEN
338+
let capturedOnToggle: (() => void) | undefined;
339+
jest.requireMock("./LayoutSection").default = jest
340+
.fn()
341+
.mockImplementation((props: { onToggleExpanded?: () => void }) => {
342+
if (!capturedOnToggle && props.onToggleExpanded) {
343+
capturedOnToggle = props.onToggleExpanded;
344+
}
345+
return <div data-testid="layout-section" />;
346+
});
347+
348+
render(<LayoutBrowser />);
349+
350+
// WHEN
351+
capturedOnToggle!();
352+
353+
// THEN
354+
expect(setPersonalExpandedMock).toHaveBeenCalledTimes(1);
355+
expect(setPersonalExpandedMock).toHaveBeenCalledWith(expect.any(Function));
356+
});
357+
358+
it("calls setSharedSectionExpanded with toggler when toggleSharedExpanded is invoked", () => {
359+
// GIVEN
360+
mockLayoutManager.supportsSharing = true;
361+
362+
const capturedOnToggles: (() => void)[] = [];
363+
jest.requireMock("./LayoutSection").default = jest
364+
.fn()
365+
.mockImplementation((props: { onToggleExpanded?: () => void }) => {
366+
if (props.onToggleExpanded) {
367+
capturedOnToggles.push(props.onToggleExpanded);
368+
}
369+
return <div data-testid="layout-section" />;
370+
});
371+
372+
render(<LayoutBrowser />);
373+
374+
// WHEN - second LayoutSection is the shared one
375+
const sharedToggle = capturedOnToggles[1];
376+
sharedToggle!();
377+
378+
// THEN
379+
expect(setSharedExpandedMock).toHaveBeenCalledTimes(1);
380+
expect(setSharedExpandedMock).toHaveBeenCalledWith(expect.any(Function));
381+
});
382+
383+
it("expands personal section when creating a new layout", async () => {
384+
// GIVEN
385+
const newLayout = LayoutBuilder.layout();
386+
mockLayoutManager.saveNewLayout = jest.fn().mockResolvedValue(newLayout);
387+
render(<LayoutBrowser currentDateForStorybook={new Date("2025-01-01")} />);
388+
389+
// WHEN - simulate createNewLayout by clicking the button
390+
const createBtn = screen.getByTestId("create-new-layout");
391+
createBtn.click();
392+
393+
// THEN
394+
await waitFor(() => {
395+
expect(setPersonalExpandedMock).toHaveBeenCalledWith(true);
396+
});
397+
});
398+
399+
it("expands shared section when sharing a layout", async () => {
400+
// GIVEN
401+
const layout = LayoutBuilder.layout();
402+
const newLayout = LayoutBuilder.layout();
403+
const promptMock = jest.fn().mockResolvedValue("Shared Layout");
404+
(usePrompt as jest.Mock).mockReturnValue([promptMock, undefined]);
405+
mockLayoutManager.saveNewLayout = jest.fn().mockResolvedValue(newLayout);
406+
407+
let capturedOnShare: ((item: Layout) => void) | undefined;
408+
jest.requireMock("./LayoutSection").default = jest
409+
.fn()
410+
.mockImplementation((props: { onShare: (item: Layout) => void }) => {
411+
capturedOnShare = props.onShare;
412+
return <div data-testid="layout-section" />;
413+
});
414+
415+
render(<LayoutBrowser />);
416+
417+
// WHEN
418+
capturedOnShare!(layout);
419+
420+
// THEN
421+
await waitFor(() => {
422+
expect(setSharedExpandedMock).toHaveBeenCalledWith(true);
423+
});
424+
});
425+
426+
it("expands personal section when making a personal copy", async () => {
427+
// GIVEN
428+
const layout = LayoutBuilder.layout();
429+
const newLayout = LayoutBuilder.layout();
430+
mockLayoutManager.makePersonalCopy = jest.fn().mockResolvedValue(newLayout);
431+
432+
let capturedOnMakePersonalCopy: ((item: Layout) => void) | undefined;
433+
jest.requireMock("./LayoutSection").default = jest
434+
.fn()
435+
.mockImplementation((props: { onMakePersonalCopy: (item: Layout) => void }) => {
436+
capturedOnMakePersonalCopy = props.onMakePersonalCopy;
437+
return <div data-testid="layout-section" />;
438+
});
439+
440+
render(<LayoutBrowser />);
441+
442+
// WHEN
443+
capturedOnMakePersonalCopy!(layout);
444+
445+
// THEN
446+
await waitFor(() => {
447+
expect(setPersonalExpandedMock).toHaveBeenCalledWith(true);
448+
});
449+
});
450+
});
254451
});

0 commit comments

Comments
 (0)