Skip to content

Commit

Permalink
fix: Re-running ui code initially rendering the old document (deephav…
Browse files Browse the repository at this point in the history
…en#1017)

This fixes a bug where re-running code causes the old document to be
rendered first. This can cause initial mounting with old props for
document trees that don't change between runs which can cause problems
with components that rely on an initial prop value only.

This also fixes an issue where the old panels are shown until new
panels/document is fetched. Now, it will show loading spinners on those
panels instead. If the new document (re-assigned to the same variable in
Python) has a new tree structure, then the old panels will be closed and
new panels opened. If the document has the same tree, then the new
panels will replace the old panels.

This Python example shows a slow loading component. Run the code, then
change the props (I commented out the `format_` prop) and re-run. The
existing panel should turn into a loading spinner and then load without
the formatting rules.

```py
from deephaven import ui
import deephaven.plot.express as dx
import time

@ui.component
def my_t():
    time.sleep(2)
    return ui.table(
        dx.data.stocks().update("SymColor=Sym==`FISH` ? `positive` : `salmon`"),
        hidden_columns=["SymColor"],
        format_=[
            ui.TableFormat(value="0.00%"),
            ui.TableFormat(cols="Timestamp", value="E, dd MMM yyyy HH:mm:ss z"),
            ui.TableFormat(cols="Size", color="info", if_="Size < 10"),
            ui.TableFormat(cols="Size", color="notice", if_="Size > 100"),
            ui.TableFormat(cols=["Sym", "Exchange"], alignment="center"),
            ui.TableFormat(
                cols=["Sym", "Exchange"], background_color="negative", if_="Sym=`CAT`"
            ),
            ui.TableFormat(if_="Sym=`DOG`", color="oklab(0.6 -0.3 -0.25)"),
            ui.TableFormat(cols="Sym", color="SymColor"),
        ],
    )

t = my_t()
```
  • Loading branch information
mattrunyon authored Nov 13, 2024
1 parent c25f578 commit b3f5459
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 16 deletions.
8 changes: 1 addition & 7 deletions plugins/ui/src/js/src/DashboardPlugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,7 @@ export function DashboardPlugin(
>(new Map());

const handleWidgetOpen = useCallback(
({
widgetId = nanoid(),
widget,
}: {
widgetId: string;
widget: WidgetDescriptor;
}) => {
({ widgetId, widget }: { widgetId: string; widget: WidgetDescriptor }) => {
log.debug('Opening widget with ID', widgetId, widget);
setWidgetMap(prevWidgetMap => {
const newWidgetMap = new Map(prevWidgetMap);
Expand Down
23 changes: 17 additions & 6 deletions plugins/ui/src/js/src/layout/ReactPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@ import {
useLayoutManager,
useListener,
} from '@deephaven/dashboard';
import { View, ViewProps, Flex, FlexProps } from '@deephaven/components';
import {
View,
ViewProps,
Flex,
FlexProps,
LoadingOverlay,
} from '@deephaven/components';
import Log from '@deephaven/log';
import PortalPanel from './PortalPanel';
import { ReactPanelControl, useReactPanel } from './ReactPanelManager';
Expand Down Expand Up @@ -186,6 +192,15 @@ function ReactPanel({
);
const widgetStatus = useWidgetStatus();

let renderedChildren: React.ReactNode;
if (widgetStatus.status === 'loading') {
renderedChildren = <LoadingOverlay />;
} else if (widgetStatus.status === 'error') {
renderedChildren = <WidgetErrorView error={widgetStatus.error} />;
} else {
renderedChildren = children;
}

return portal
? ReactDOM.createPortal(
<ReactPanelContext.Provider value={panelId}>
Expand Down Expand Up @@ -224,11 +239,7 @@ function ReactPanel({
* Don't render the children if there's an error with the widget. If there's an error with the widget, we can assume the children won't render properly,
* but we still want the panels to appear so things don't disappear/jump around.
*/}
{widgetStatus.status === 'error' ? (
<WidgetErrorView error={widgetStatus.error} />
) : (
children
)}
{renderedChildren}
</ReactPanelErrorBoundary>
</Flex>
</View>
Expand Down
25 changes: 22 additions & 3 deletions plugins/ui/src/js/src/widget/WidgetHandler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import React, {
useRef,
useState,
} from 'react';
// eslint-disable-next-line camelcase
import { unstable_batchedUpdates } from 'react-dom';
import {
JSONRPCClient,
JSONRPCServer,
Expand Down Expand Up @@ -64,6 +66,16 @@ function WidgetHandler({
initialData: initialDataProp,
}: WidgetHandlerProps): JSX.Element | null {
const { widget, error: widgetError } = useWidget(widgetDescriptor);
const [isLoading, setIsLoading] = useState(true);
const [prevWidgetDescriptor, setPrevWidgetDescriptor] =
useState(widgetDescriptor);
// Cannot use usePrevious to change setIsLoading
// Since usePrevious runs in an effect, the value never gets updated if setIsLoading is called during render
// Use the widgetDescriptor because useWidget is async so the widget doesn't immediately change
if (widgetDescriptor !== prevWidgetDescriptor) {
setPrevWidgetDescriptor(widgetDescriptor);
setIsLoading(true);
}

const [document, setDocument] = useState<ReactNode>();

Expand Down Expand Up @@ -224,8 +236,12 @@ function WidgetHandler({
log.debug2(METHOD_DOCUMENT_UPDATED, params);
const [documentParam, stateParam] = params;
const newDocument = parseDocument(documentParam);
setInternalError(undefined);
setDocument(newDocument);
// TODO: Remove unstable_batchedUpdates wrapper when upgrading to React 18
unstable_batchedUpdates(() => {
setInternalError(undefined);
setDocument(newDocument);
setIsLoading(false);
});
if (stateParam != null) {
try {
const newState = JSON.parse(stateParam);
Expand Down Expand Up @@ -339,11 +355,14 @@ function WidgetHandler({
if (error != null) {
return { status: 'error', descriptor: widgetDescriptor, error };
}
if (isLoading) {
return { status: 'loading', descriptor: widgetDescriptor };
}
if (renderedDocument != null) {
return { status: 'ready', descriptor: widgetDescriptor };
}
return { status: 'loading', descriptor: widgetDescriptor };
}, [error, renderedDocument, widgetDescriptor]);
}, [error, renderedDocument, widgetDescriptor, isLoading]);

return useMemo(
() =>
Expand Down

0 comments on commit b3f5459

Please sign in to comment.