From 19ee7e550ceeba1249c9e21ed953b804bb850c81 Mon Sep 17 00:00:00 2001 From: Lais Portugal Date: Tue, 21 May 2024 13:28:11 +0100 Subject: [PATCH 1/2] Including missing improvement that was available on the original project --- package.json | 2 +- .../components/MessagePipeline/index.test.tsx | 10 ++++----- .../src/components/MessagePipeline/store.ts | 13 +++++------- .../PanelExtensionAdapter.tsx | 21 ++++++++++++------- packages/studio/package.json | 2 +- 5 files changed, 26 insertions(+), 22 deletions(-) diff --git a/package.json b/package.json index bc8c63ab79..57eda36ef5 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "foxbox", - "version": "1.0.2", + "version": "1.0.3", "license": "MPL-2.0", "private": true, "productName": "Foxbox", diff --git a/packages/studio-base/src/components/MessagePipeline/index.test.tsx b/packages/studio-base/src/components/MessagePipeline/index.test.tsx index 1532e8d882..78bcb8f051 100644 --- a/packages/studio-base/src/components/MessagePipeline/index.test.tsx +++ b/packages/studio-base/src/components/MessagePipeline/index.test.tsx @@ -14,7 +14,7 @@ /* eslint-disable jest/no-conditional-expect */ -import { renderHook, act } from "@testing-library/react"; +import { act, renderHook } from "@testing-library/react"; import { PropsWithChildren, useCallback, useState } from "react"; import { DeepPartial } from "ts-essentials"; @@ -29,7 +29,7 @@ import MockCurrentLayoutProvider from "@foxglove/studio-base/providers/CurrentLa import delay from "@foxglove/studio-base/util/delay"; import { makeMockAppConfiguration } from "@foxglove/studio-base/util/makeMockAppConfiguration"; -import { MessagePipelineProvider, useMessagePipeline, MessagePipelineContext } from "."; +import { MessagePipelineContext, MessagePipelineProvider, useMessagePipeline } from "."; import FakePlayer from "./FakePlayer"; import { MAX_PROMISE_TIMEOUT_TIME_MS } from "./pauseFrameForPromise"; @@ -136,7 +136,7 @@ describe("MessagePipelineProvider/useMessagePipeline", () => { playerState: { activeData: undefined, capabilities: [], - presence: PlayerPresence.NOT_PRESENT, + presence: PlayerPresence.INITIALIZING, playerId: "", progress: {}, }, @@ -199,7 +199,7 @@ describe("MessagePipelineProvider/useMessagePipeline", () => { expect(all[0]!.playerState).toEqual({ activeData: undefined, capabilities: [], - presence: PlayerPresence.NOT_PRESENT, + presence: PlayerPresence.INITIALIZING, playerId: "", progress: {}, }); @@ -259,7 +259,7 @@ describe("MessagePipelineProvider/useMessagePipeline", () => { playerState: { activeData: undefined, capabilities: [], - presence: PlayerPresence.NOT_PRESENT, + presence: PlayerPresence.INITIALIZING, playerId: "", progress: {}, }, diff --git a/packages/studio-base/src/components/MessagePipeline/store.ts b/packages/studio-base/src/components/MessagePipeline/store.ts index 11d7cb8154..afd771712c 100644 --- a/packages/studio-base/src/components/MessagePipeline/store.ts +++ b/packages/studio-base/src/components/MessagePipeline/store.ts @@ -21,15 +21,16 @@ import { PlayerState, SubscribePayload, } from "@foxglove/studio-base/players/types"; -import { assertNever } from "@foxglove/studio-base/util/assertNever"; import isDesktopApp from "@foxglove/studio-base/util/isDesktopApp"; import { FramePromise } from "./pauseFrameForPromise"; import { MessagePipelineContext } from "./types"; -export function defaultPlayerState(): PlayerState { +export function defaultPlayerState(player?: Player): PlayerState { return { - presence: PlayerPresence.NOT_PRESENT, + // when there is a player we default to initializing, to prevent thrashing in the UI when + // the player is initialized. + presence: player ? PlayerPresence.INITIALIZING : PlayerPresence.NOT_PRESENT, progress: {}, capabilities: [], profile: undefined, @@ -138,7 +139,7 @@ export function createMessagePipelineStore({ }, public: { - playerState: defaultPlayerState(), + playerState: defaultPlayerState(initialPlayer), messageEventsBySubscriberId: new Map(), subscriptions: [], sortedTopics: [], @@ -424,10 +425,6 @@ export function reducer( } } - assertNever( - action, - `Unhandled message pipeline action type ${(action as MessagePipelineStateAction).type}`, - ); } async function builtinFetch(url: string, opts?: { signal?: AbortSignal }) { diff --git a/packages/studio-base/src/components/PanelExtensionAdapter/PanelExtensionAdapter.tsx b/packages/studio-base/src/components/PanelExtensionAdapter/PanelExtensionAdapter.tsx index c8a29178e2..d768234a81 100644 --- a/packages/studio-base/src/components/PanelExtensionAdapter/PanelExtensionAdapter.tsx +++ b/packages/studio-base/src/components/PanelExtensionAdapter/PanelExtensionAdapter.tsx @@ -7,7 +7,7 @@ import { CSSProperties, useEffect, useLayoutEffect, useMemo, useRef, useState } import { useLatest } from "react-use"; import { v4 as uuid } from "uuid"; -import { useValueChangedDebugLog, useSynchronousMountedState } from "@foxglove/hooks"; +import { useSynchronousMountedState, useValueChangedDebugLog } from "@foxglove/hooks"; import Logger from "@foxglove/log"; import { fromSec, toSec } from "@foxglove/rostime"; import { @@ -42,11 +42,12 @@ import useGlobalVariables from "@foxglove/studio-base/hooks/useGlobalVariables"; import { AdvertiseOptions, PlayerCapabilities, + PlayerPresence, SubscribePayload, } from "@foxglove/studio-base/players/types"; import { - usePanelSettingsTreeUpdate, useDefaultPanelTitle, + usePanelSettingsTreeUpdate, } from "@foxglove/studio-base/providers/PanelStateContextProvider"; import { PanelConfig, SaveConfig } from "@foxglove/studio-base/types/panels"; import { assertNever } from "@foxglove/studio-base/util/assertNever"; @@ -118,7 +119,7 @@ function PanelExtensionAdapter( const { playerState, pauseFrame, setSubscriptions, seekPlayback, sortedTopics } = messagePipelineContext; - const { capabilities, profile: dataSourceProfile } = playerState; + const { capabilities, profile: dataSourceProfile, presence: playerPresence } = playerState; const { openSiblingPanel, setMessagePathDropConfig } = usePanelContext(); @@ -547,6 +548,9 @@ function PanelExtensionAdapter( ); }, [initialState, highestSupportedConfigVersion]); + + const playerIsInitializing = playerPresence === PlayerPresence.INITIALIZING; + // Manage extension lifecycle by calling initPanel() when the panel context changes. // // If we useEffect here instead of useLayoutEffect, the prevRenderState can get polluted with data @@ -556,9 +560,12 @@ function PanelExtensionAdapter( throw new Error("Expected panel container to be mounted"); } - // If the config is too new for this panel to support we bail and don't do any panel initialization - // We will instead show a warning message to the user - if (configTooNew) { + // Also don't show panel when the player is initializing. The initializing state is temporary for + // players to go through to load their sources. Once a player has completed initialization `initPanel` is called again (or even a few times), + // because parts of the player context have changed. This cleans up the old panel that was present + // during initialization. So there can be no state held between extension panels between initialization and + // whatever follows it. To prevent this unnecessary render, we do not render the panel during initialization. + if (configTooNew || playerIsInitializing) { return; } @@ -596,7 +603,7 @@ function PanelExtensionAdapter( getMessagePipelineContext().setSubscriptions(panelId, []); getMessagePipelineContext().setPublishers(panelId, []); }; - }, [initPanel, panelId, partialExtensionContext, getMessagePipelineContext, configTooNew]); + }, [initPanel, panelId, partialExtensionContext, getMessagePipelineContext, configTooNew, playerIsInitializing,]); const style: CSSProperties = {}; if (slowRender) { diff --git a/packages/studio/package.json b/packages/studio/package.json index 1e7353625a..a1e1a50d7c 100644 --- a/packages/studio/package.json +++ b/packages/studio/package.json @@ -1,6 +1,6 @@ { "name": "@foxglove/studio", - "version": "1.0.2", + "version": "1.0.3", "license": "MPL-2.0", "repository": { "type": "git", From 080a4bc8c31513994091433991d2b9a843c45efc Mon Sep 17 00:00:00 2001 From: Lais Portugal Date: Tue, 21 May 2024 15:49:28 +0100 Subject: [PATCH 2/2] apply lint --- .../src/components/MessagePipeline/store.ts | 1 - .../PanelExtensionAdapter.tsx | 14 ++++++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/studio-base/src/components/MessagePipeline/store.ts b/packages/studio-base/src/components/MessagePipeline/store.ts index afd771712c..4d4e36bc5d 100644 --- a/packages/studio-base/src/components/MessagePipeline/store.ts +++ b/packages/studio-base/src/components/MessagePipeline/store.ts @@ -424,7 +424,6 @@ export function reducer( }; } } - } async function builtinFetch(url: string, opts?: { signal?: AbortSignal }) { diff --git a/packages/studio-base/src/components/PanelExtensionAdapter/PanelExtensionAdapter.tsx b/packages/studio-base/src/components/PanelExtensionAdapter/PanelExtensionAdapter.tsx index d768234a81..ec8e13b17b 100644 --- a/packages/studio-base/src/components/PanelExtensionAdapter/PanelExtensionAdapter.tsx +++ b/packages/studio-base/src/components/PanelExtensionAdapter/PanelExtensionAdapter.tsx @@ -119,7 +119,7 @@ function PanelExtensionAdapter( const { playerState, pauseFrame, setSubscriptions, seekPlayback, sortedTopics } = messagePipelineContext; - const { capabilities, profile: dataSourceProfile, presence: playerPresence } = playerState; + const { capabilities, profile: dataSourceProfile, presence: playerPresence } = playerState; const { openSiblingPanel, setMessagePathDropConfig } = usePanelContext(); @@ -548,8 +548,7 @@ function PanelExtensionAdapter( ); }, [initialState, highestSupportedConfigVersion]); - - const playerIsInitializing = playerPresence === PlayerPresence.INITIALIZING; + const playerIsInitializing = playerPresence === PlayerPresence.INITIALIZING; // Manage extension lifecycle by calling initPanel() when the panel context changes. // @@ -603,7 +602,14 @@ function PanelExtensionAdapter( getMessagePipelineContext().setSubscriptions(panelId, []); getMessagePipelineContext().setPublishers(panelId, []); }; - }, [initPanel, panelId, partialExtensionContext, getMessagePipelineContext, configTooNew, playerIsInitializing,]); + }, [ + initPanel, + panelId, + partialExtensionContext, + getMessagePipelineContext, + configTooNew, + playerIsInitializing, + ]); const style: CSSProperties = {}; if (slowRender) {