From 34c54ef04619d6da8510a921c7bc8096c90515f1 Mon Sep 17 00:00:00 2001 From: Luco Bellic Date: Wed, 7 Oct 2026 11:50:02 +0200 Subject: [PATCH 1/4] test(panel-extension): cover nested converter settings routing --- .../PanelExtensionAdapter.test.tsx | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/packages/suite-base/src/components/PanelExtensionAdapter/PanelExtensionAdapter.test.tsx b/packages/suite-base/src/components/PanelExtensionAdapter/PanelExtensionAdapter.test.tsx index cb0b358421..3d5ac01d0d 100644 --- a/packages/suite-base/src/components/PanelExtensionAdapter/PanelExtensionAdapter.test.tsx +++ b/packages/suite-base/src/components/PanelExtensionAdapter/PanelExtensionAdapter.test.tsx @@ -22,6 +22,7 @@ import { MessageEvent, Immutable, Subscription, + SettingsTreeAction, } from "@lichtblick/suite"; import MockPanelContextProvider from "@lichtblick/suite-base/components/MockPanelContextProvider"; import { AlertsContext, AlertsContextStore } from "@lichtblick/suite-base/context/AlertsContext"; @@ -71,6 +72,83 @@ describe("PanelExtensionAdapter", () => { await sig; }); + it("routes nested converter settings through the panel updater", () => { + // GIVEN a panel that handles converter settings updates + const saveConfig = jest.fn(); + const updateSettingsTree = jest.fn(); + const settingsTreeSpy = jest + .spyOn(PanelStateContextProvider, "usePanelSettingsTreeUpdate") + .mockReturnValue(updateSettingsTree); + const config = { topics: { topic: { child: { enabled: false } } } }; + const panelActionHandler = jest.fn(); + const extensionHandler = jest.fn(); + const updateTopicSettings = jest.fn((updateSettings) => updateSettings(config)); + const initPanel = (context: BuiltinPanelExtensionContext) => { + context.updatePanelSettingsEditor( + { actionHandler: panelActionHandler, nodes: {} }, + updateTopicSettings, + ); + }; + + const view = render( + + ({}), + panelSettings: { + "3D": { + settings: () => ({}), + handler: extensionHandler, + }, + }, + }, + ], + }} + > + + + , + ); + + // WHEN a nested setting is enabled + const action: SettingsTreeAction = { + action: "update", + payload: { path: ["topics", "topic", "child", "enabled"], input: "boolean", value: true }, + }; + act(() => { + updateSettingsTree.mock.calls[0]![0].actionHandler(action); + }); + + // THEN the panel applies the edit once without a duplicate adapter save + expect(updateTopicSettings).toHaveBeenCalledTimes(1); + expect(extensionHandler).toHaveBeenCalledWith(action, config.topics.topic); + expect(extensionHandler).toHaveBeenCalledTimes(1); + expect(saveConfig).not.toHaveBeenCalled(); + + // WHEN a normal panel setting is changed + const panelAction: SettingsTreeAction = { + action: "update", + payload: { path: ["scene", "enableStats"], input: "boolean", value: true }, + }; + act(() => { + updateSettingsTree.mock.calls.at(-1)![0].actionHandler(panelAction); + }); + + // THEN only the panel handler receives the edit without another converter update or save + expect(panelActionHandler).toHaveBeenLastCalledWith(panelAction); + expect(panelActionHandler).toHaveBeenCalledTimes(2); + expect(updateTopicSettings).toHaveBeenCalledTimes(1); + expect(extensionHandler).toHaveBeenCalledTimes(1); + expect(saveConfig).not.toHaveBeenCalled(); + view.unmount(); + settingsTreeSpy.mockRestore(); + }); + it("should expose point-in-time message lookups through the panel context", async () => { // GIVEN - a panel that captures its extension context let panelContext: PanelExtensionContext | undefined; From 00d14cfea0c7b424a3784823527958c73ddeb264 Mon Sep 17 00:00:00 2001 From: Luco Bellic Date: Wed, 7 Oct 2026 11:50:04 +0200 Subject: [PATCH 2/4] test(3d): preserve nested settings after later saves --- .../ThreeDeeRender/ThreeDeeRender.test.tsx | 146 +++++++++++++++++- 1 file changed, 144 insertions(+), 2 deletions(-) diff --git a/packages/suite-base/src/panels/ThreeDeeRender/ThreeDeeRender.test.tsx b/packages/suite-base/src/panels/ThreeDeeRender/ThreeDeeRender.test.tsx index 0ba2cb2c03..96f845e467 100644 --- a/packages/suite-base/src/panels/ThreeDeeRender/ThreeDeeRender.test.tsx +++ b/packages/suite-base/src/panels/ThreeDeeRender/ThreeDeeRender.test.tsx @@ -9,18 +9,27 @@ import "@testing-library/jest-dom"; import { act, render, waitFor } from "@testing-library/react"; +import { produce } from "immer"; -import { Topic } from "@lichtblick/suite"; -import { BuiltinPanelExtensionContext } from "@lichtblick/suite-base/components/PanelExtensionAdapter"; +import { SettingsTreeAction, Topic } from "@lichtblick/suite"; +import MockPanelContextProvider from "@lichtblick/suite-base/components/MockPanelContextProvider"; +import { + BuiltinPanelExtensionContext, + PanelExtensionAdapter, +} from "@lichtblick/suite-base/components/PanelExtensionAdapter"; import { useAnalytics } from "@lichtblick/suite-base/context/AnalyticsContext"; import { DEFAULT_FOLLOW_MODE, MAX_TRANSFORM_MESSAGES, } from "@lichtblick/suite-base/panels/ThreeDeeRender/constants"; import type { MessageEvent } from "@lichtblick/suite-base/players/types"; +import * as PanelStateContextProvider from "@lichtblick/suite-base/providers/PanelStateContextProvider"; +import PanelSetup from "@lichtblick/suite-base/stories/PanelSetup"; import MessageEventBuilder from "@lichtblick/suite-base/testing/builders/MessageEventBuilder"; import RenderStateBuilder from "@lichtblick/suite-base/testing/builders/RenderStateBuilder"; +import { SaveConfig } from "@lichtblick/suite-base/types/panels"; +import { RendererConfig } from "./IRenderer"; import { Renderer } from "./Renderer"; import { ThreeDeeRender } from "./ThreeDeeRender"; import { DEFAULT_CAMERA_STATE } from "./camera"; @@ -225,6 +234,139 @@ describe("ThreeDeeRender", () => { expect(container).toBeInTheDocument(); }); + it("saves direct and nested converter edits immediately and preserves them after a later save", () => { + // GIVEN a panel with a disabled nested setting + jest.useFakeTimers(); + const renderer = { + ...createMockRenderer(), + updateConfig: (handler: (draft: RendererConfig) => void) => { + renderer.config = produce(renderer.config as RendererConfig, handler); + renderer.emit("configChange", renderer); + }, + }; + mockedRenderer.mockImplementationOnce(() => renderer as unknown as Renderer); + const initialConfig = { topics: { topic: { text: "", child: { enabled: false } } } }; + // The scene handler applies direct fields before the converter handles the same edit. + renderer.settings.handleAction.mockImplementation((action: SettingsTreeAction) => { + if (action.action === "update" && action.payload.path.length === 3) { + renderer.updateConfig((draft) => { + (draft.topics.topic as typeof initialConfig.topics.topic).text = action.payload + .value as string; + }); + } + }); + let savedConfig: unknown = initialConfig; + const saveConfig: SaveConfig = (update) => { + savedConfig = typeof update === "function" ? update(savedConfig) : update; + }; + + // AND GIVEN an adapter with a converter that handles nested settings + const updateSettingsTree = jest.fn(); + const settingsTreeSpy = jest + .spyOn(PanelStateContextProvider, "usePanelSettingsTreeUpdate") + .mockReturnValue(updateSettingsTree); + const initPanel = jest.fn(); + const adapterView = render( + + ({}), + panelSettings: { + "3D": { + settings: () => ({}), + handler: (action, config) => { + if (action.action === "update") { + const topicConfig = config as typeof initialConfig.topics.topic; + if (action.payload.path[2] === "text") { + topicConfig.text = action.payload.value as string; + } else { + topicConfig.child.enabled = action.payload.value as boolean; + } + } + }, + }, + }, + }, + ], + }} + > + + + , + ); + + // AND GIVEN a 3D panel using the adapter's settings and save methods + const context = initPanel.mock.calls.at(-1)![0]; + const props = setup(undefined, { + initialState: context.initialState, + saveState: context.saveState.bind(context), + updatePanelSettingsEditor: context.updatePanelSettingsEditor.bind(context), + }); + const view = render(); + + try { + // WHEN the nested setting is enabled + act(() => { + updateSettingsTree.mock.calls.at(-1)![0].actionHandler({ + action: "update", + payload: { + path: ["topics", "topic", "child", "enabled"], + input: "boolean", + value: true, + }, + }); + }); + + // THEN the edit is saved + expect(savedConfig).toMatchObject({ + topics: { topic: { child: { enabled: true } } }, + }); + + // WHEN text is typed in a direct field without waiting for the debounced save + for (const text of ["h", "he", "hello", "hello world"]) { + act(() => { + updateSettingsTree.mock.calls.at(-1)![0].actionHandler({ + action: "update", + payload: { path: ["topics", "topic", "text"], input: "string", value: text }, + }); + }); + + // THEN the settings editor can read the latest text after every edit + expect(savedConfig).toMatchObject({ + topics: { topic: { text, child: { enabled: true } } }, + }); + } + + // WHEN camera movement triggers a later save + act(() => { + renderer.getCameraState.mockReturnValue({ ...DEFAULT_CAMERA_STATE, distance: 10 }); + renderer.emit("cameraMove"); + }); + act(() => { + jest.advanceTimersByTime(100); + }); + + // THEN both converter settings survive the later save + expect(savedConfig).toMatchObject({ + topics: { topic: { text: "hello world", child: { enabled: true } } }, + }); + } finally { + view.unmount(); + adapterView.unmount(); + settingsTreeSpy.mockRestore(); + jest.useRealTimers(); + } + }); + it("renders a canvas element", () => { // Given const props = setup(); From 528a474cecbf7a306fe090996f2e24636df3962e Mon Sep 17 00:00:00 2001 From: Luco Bellic Date: Wed, 7 Oct 2026 11:50:05 +0200 Subject: [PATCH 3/4] fix(panel-extension): delegate converter settings updates to panels Direct layout saves can leave panel config stale and lose nested edits. Add an optional updater to updatePanelSettingsEditor so panels can apply converter edits to their own config. Keep the existing save fallback. --- .../PanelExtensionAdapter.tsx | 52 ++++++++++++------- .../components/PanelExtensionAdapter/types.ts | 19 ++++++- 2 files changed, 49 insertions(+), 22 deletions(-) diff --git a/packages/suite-base/src/components/PanelExtensionAdapter/PanelExtensionAdapter.tsx b/packages/suite-base/src/components/PanelExtensionAdapter/PanelExtensionAdapter.tsx index 5d44bd5384..5d44a3c0ba 100644 --- a/packages/suite-base/src/components/PanelExtensionAdapter/PanelExtensionAdapter.tsx +++ b/packages/suite-base/src/components/PanelExtensionAdapter/PanelExtensionAdapter.tsx @@ -69,7 +69,11 @@ import { maybeCast } from "@lichtblick/suite-base/util/maybeCast"; import { PanelConfigVersionError } from "./PanelConfigVersionError"; import { RenderStateConfig, initRenderStateBuilder } from "./renderState"; -import { BuiltinPanelExtensionContext, MessageConverterAlertHandler } from "./types"; +import { + BuiltinPanelExtensionContext, + MessageConverterAlertHandler, + TopicSettingsUpdater, +} from "./types"; import { useSharedPanelState } from "./useSharedPanelState"; import { useSubscribeMessageRange } from "./useSubscribeMessageRange"; @@ -392,7 +396,12 @@ function PanelExtensionAdapter( }, }; - const extensionSettingsActionHandler = (action: SettingsTreeAction) => { + const extensionSettingsActionHandler = ( + action: SettingsTreeAction, + updateTopicSettings: TopicSettingsUpdater = (updateSettings) => { + saveConfig(produce(updateSettings)); + }, + ) => { if (action.action === "reorder-node") { return; // Extensions don't support reordering } @@ -400,25 +409,25 @@ function PanelExtensionAdapter( payload: { path }, } = action; - saveConfig( - produce<{ topics: Record }>((draft) => { - const [category, topicName] = path; + const [category, topicName] = path; - if (category === "topics" && topicName != undefined) { - const topicToSchemaNameMap = getTopicToSchemaNameMap(messagePipelineState()); - const schemaName = topicToSchemaNameMap[topicName]; + if (category === "topics" && topicName != undefined) { + const topicToSchemaNameMap = getTopicToSchemaNameMap(messagePipelineState()); + const schemaName = topicToSchemaNameMap[topicName]; - if (schemaName == undefined) { - return; - } + if (schemaName == undefined) { + return; + } - extensionsSettings[panelName]?.[schemaName]?.handler(action, draft.topics[topicName]); - setForceConversion((_old) => { - return new Set([topicName]); - }); - } - }), - ); + const settings = extensionsSettings[panelName]?.[schemaName]; + if (settings) { + updateTopicSettings((draft) => { + draft.topics[topicName] ??= {}; + settings.handler(action, draft.topics[topicName]); + }); + setForceConversion(() => new Set([topicName])); + } + } }; return { @@ -643,13 +652,16 @@ function PanelExtensionAdapter( setSubscribedAppSettings(settings); }, - updatePanelSettingsEditor: (settings: SettingsTree) => { + updatePanelSettingsEditor: ( + settings: SettingsTree, + updateTopicSettings?: TopicSettingsUpdater, + ) => { if (!isMounted()) { return; } const actionHandler: typeof settings.actionHandler = (action) => { settings.actionHandler(action); - extensionSettingsActionHandler(action); + extensionSettingsActionHandler(action, updateTopicSettings); }; updatePanelSettingsTree({ ...settings, actionHandler }); }, diff --git a/packages/suite-base/src/components/PanelExtensionAdapter/types.ts b/packages/suite-base/src/components/PanelExtensionAdapter/types.ts index 4bac4ec421..46ac11a8d0 100644 --- a/packages/suite-base/src/components/PanelExtensionAdapter/types.ts +++ b/packages/suite-base/src/components/PanelExtensionAdapter/types.ts @@ -5,7 +5,12 @@ // License, v2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at http://mozilla.org/MPL/2.0/ -import { Immutable, MessageConverterAlert, PanelExtensionContext } from "@lichtblick/suite"; +import { + Immutable, + MessageConverterAlert, + PanelExtensionContext, + SettingsTree, +} from "@lichtblick/suite"; import { IteratorResult } from "@lichtblick/suite-base/players/IterablePlayer/IIterableSource"; import { PlayerAlert, Topic } from "@lichtblick/suite-base/players/types"; import { InstalledMessageConverter } from "@lichtblick/suite-base/types/messageConverters"; @@ -64,6 +69,12 @@ export type MessagePathDropConfig = { * extensions. */ export type BuiltinPanelExtensionContext = { + /** Apply converter settings to the panel's current config before saving. */ + updatePanelSettingsEditor: ( + settings: SettingsTree, + updateTopicSettings?: TopicSettingsUpdater, + ) => void; + /** * Fetch an asset from Studio's asset manager. * @@ -95,7 +106,11 @@ export type BuiltinPanelExtensionContext = { * clears the alert with the given `alertId`. */ unstable_setAlert?: (alertId: string, alert: Immutable | undefined) => void; -} & PanelExtensionContext; +} & Omit; + +export type TopicSettingsUpdater = ( + updateSettings: (config: { topics: Record }) => void, +) => void; export type MessageConverterAlertHandler = ( converter: InstalledMessageConverter, From d90096aa1579845289da52cea8dd840494270b0b Mon Sep 17 00:00:00 2001 From: Luco Bellic Date: Wed, 7 Oct 2026 11:50:07 +0200 Subject: [PATCH 4/4] fix(3d): apply converter settings to renderer config Prevent later camera or panel saves from resetting nested converter edits. Provide a settings updater that applies edits through renderer.updateConfig and immediately saves the resulting config to keep both copies in sync. Detect topic changes across both the scene and converter handlers so direct field edits are saved immediately, keeping displayed values in sync with the renderer config. --- .../panels/ThreeDeeRender/ThreeDeeRender.tsx | 44 +++++++++++++++---- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/packages/suite-base/src/panels/ThreeDeeRender/ThreeDeeRender.tsx b/packages/suite-base/src/panels/ThreeDeeRender/ThreeDeeRender.tsx index 1bf7879add..5aec84f1af 100644 --- a/packages/suite-base/src/panels/ThreeDeeRender/ThreeDeeRender.tsx +++ b/packages/suite-base/src/panels/ThreeDeeRender/ThreeDeeRender.tsx @@ -26,6 +26,7 @@ import { Topic, } from "@lichtblick/suite"; import { AppSetting } from "@lichtblick/suite-base/AppSetting"; +import { TopicSettingsUpdater } from "@lichtblick/suite-base/components/PanelExtensionAdapter/types"; import { useAnalytics } from "@lichtblick/suite-base/context/AnalyticsContext"; import { DEFAULT_SCENE_EXTENSION_CONFIG } from "@lichtblick/suite-base/panels/ThreeDeeRender/SceneExtensionConfig"; import { @@ -132,6 +133,7 @@ export function ThreeDeeRender(props: Readonly): React.JSX. }; }); const configRef = useLatest(config); + const lastSavedConfig = useRef>(); const { cameraState } = config; const backgroundColor = config.scene.backgroundColor; @@ -358,13 +360,35 @@ export function ThreeDeeRender(props: Readonly): React.JSX. // Rebuild the settings sidebar tree as needed useEffect(() => { - context.updatePanelSettingsEditor({ - actionHandler, - enableFilter: true, - focusedPath: focusedSettingsPath, - nodes: settingsTree ?? {}, - }); - }, [actionHandler, context, focusedSettingsPath, settingsTree]); + let topicsBeforeAction = renderer?.config.topics; + + // Detect direct field edits even when the scene handler applies them first. + const handleSettingsAction = (action: SettingsTreeAction) => { + topicsBeforeAction = renderer?.config.topics; + actionHandler(action); + }; + + // Save immediately so stale editor values cannot overwrite the latest edits. + const applyConverterSettings: TopicSettingsUpdater = (updateSettings) => { + if (renderer) { + renderer.updateConfig(updateSettings); + if (renderer.config.topics !== topicsBeforeAction) { + saveState(renderer.config); + lastSavedConfig.current = renderer.config; + } + } + }; + + context.updatePanelSettingsEditor( + { + actionHandler: handleSettingsAction, + enableFilter: true, + focusedPath: focusedSettingsPath, + nodes: settingsTree ?? {}, + }, + applyConverterSettings, + ); + }, [actionHandler, context, focusedSettingsPath, settingsTree, renderer, saveState]); // Update the renderer's reference to `config` when it changes. Note that this does *not* // automatically update the settings tree. @@ -412,7 +436,11 @@ export function ThreeDeeRender(props: Readonly): React.JSX. // Save panel settings whenever they change const throttledSave = useDebouncedCallback( (newConfig: Immutable) => { - saveState(newConfig); + // Immediate converter saves already persist this config. + if (newConfig !== lastSavedConfig.current) { + saveState(newConfig); + lastSavedConfig.current = newConfig; + } }, 100, { leading: false, trailing: true, maxWait: 1000 },