Skip to content

Commit 49d60d5

Browse files
committed
all interactions leading to update in chart can be skeletonized
1 parent d40abeb commit 49d60d5

File tree

13 files changed

+253
-56
lines changed

13 files changed

+253
-56
lines changed

chartlets.js/packages/lib/src/actions/handleComponentChange.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,18 @@ function getCallbackRequests(
7070
equalObjPaths(input.property, changeEvent.property),
7171
);
7272
if (inputIndex >= 0) {
73+
// Collect output IDs for updating their respective loading states
74+
const outputs = contribution.callbacks?.[callbackIndex]["outputs"];
75+
const outputIds: string[] =
76+
outputs?.map((output) => output.id as string) ?? [];
7377
// Collect triggered callback
7478
callbackRequests.push({
7579
contribPoint,
7680
contribIndex,
7781
callbackIndex,
7882
inputIndex,
7983
inputValues: getInputValues(inputs, contribution, hostStore),
84+
outputIds: outputIds,
8085
});
8186
}
8287
}

chartlets.js/packages/lib/src/actions/handleHostStoreChange.ts

Lines changed: 55 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { invokeCallbacks } from "@/actions/helpers/invokeCallbacks";
1111
import type { ContributionState } from "@/types/state/contribution";
1212
import type { HostStore } from "@/types/state/host";
1313
import { store } from "@/store";
14+
import { shallowEqualArrays } from "@/utils/compare";
1415

1516
/**
1617
* A reference to a property of an input of a callback of a contribution.
@@ -43,29 +44,69 @@ export function handleHostStoreChange() {
4344
contributionsRecord,
4445
hostStore,
4546
);
46-
if (callbackRequests && callbackRequests.length > 0) {
47-
invokeCallbacks(callbackRequests);
47+
48+
const filteredCallbackRequests = callbackRequests.filter(
49+
(callbackRequest): callbackRequest is CallbackRequest =>
50+
callbackRequest !== undefined,
51+
);
52+
if (filteredCallbackRequests && filteredCallbackRequests.length > 0) {
53+
invokeCallbacks(filteredCallbackRequests);
4854
}
4955
}
5056

51-
function getCallbackRequests(
57+
// Exporting for testing only
58+
export function getCallbackRequests(
5259
propertyRefs: PropertyRef[],
5360
contributionsRecord: Record<string, ContributionState[]>,
5461
hostStore: HostStore,
55-
): CallbackRequest[] {
56-
return propertyRefs.map((propertyRef) => {
57-
const contributions = contributionsRecord[propertyRef.contribPoint];
58-
const contribution = contributions[propertyRef.contribIndex];
59-
const callback = contribution.callbacks![propertyRef.callbackIndex];
60-
const inputValues = getInputValues(
61-
callback.inputs!,
62-
contribution,
62+
): (CallbackRequest | undefined)[] {
63+
const { lastCallbackInputValues } = store.getState();
64+
return propertyRefs.map((propertyRef) =>
65+
getCallbackRequest(
66+
propertyRef,
67+
lastCallbackInputValues,
68+
contributionsRecord,
6369
hostStore,
64-
);
65-
return { ...propertyRef, inputValues };
66-
});
70+
),
71+
);
6772
}
6873

74+
const getCallbackRequest = (
75+
propertyRef: PropertyRef,
76+
lastCallbackInputValues: Record<string, unknown[]>,
77+
contributionsRecord: Record<string, ContributionState[]>,
78+
hostStore: HostStore,
79+
) => {
80+
const contribPoint: string = propertyRef.contribPoint;
81+
const contribIndex: number = propertyRef.contribIndex;
82+
const callbackIndex: number = propertyRef.callbackIndex;
83+
const contributions = contributionsRecord[contribPoint];
84+
const contribution = contributions[contribIndex];
85+
const callback = contribution.callbacks![callbackIndex];
86+
const inputValues = getInputValues(callback.inputs!, contribution, hostStore);
87+
const callbackId = `${contribPoint}-${contribIndex}-${callbackIndex}`;
88+
const lastInputValues = lastCallbackInputValues[callbackId];
89+
if (shallowEqualArrays(lastInputValues, inputValues)) {
90+
// We no longer log, as the situation is quite common
91+
// Enable error logging for debugging only:
92+
// console.groupCollapsed("Skipping callback request");
93+
// console.debug("inputValues", inputValues);
94+
// console.groupEnd();
95+
return undefined;
96+
}
97+
store.setState({
98+
lastCallbackInputValues: {
99+
...lastCallbackInputValues,
100+
[callbackId]: inputValues,
101+
},
102+
});
103+
// Collect output IDs for updating their respective loading states
104+
const outputs = contribution.callbacks?.[callbackIndex]["outputs"];
105+
const outputIds: string[] =
106+
outputs?.map((output) => output.id as string) ?? [];
107+
return { ...propertyRef, inputValues, outputIds };
108+
};
109+
69110
// TODO: use a memoized selector to get hostStorePropertyRefs
70111
// Note that this will only be effective and once we split the
71112
// static contribution infos and dynamic contribution states.

chartlets.js/packages/lib/src/actions/helpers/invokeCallbacks.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { fetchCallback } from "@/api/fetchCallback";
44
import { applyStateChangeRequests } from "@/actions/helpers/applyStateChangeRequests";
55

66
export function invokeCallbacks(callbackRequests: CallbackRequest[]) {
7-
const { configuration } = store.getState();
7+
const { configuration, loadingState } = store.getState();
88
const shouldLog = configuration.logging?.enabled;
99
const invocationId = getInvocationId();
1010
if (shouldLog) {
@@ -13,6 +13,20 @@ export function invokeCallbacks(callbackRequests: CallbackRequest[]) {
1313
callbackRequests,
1414
);
1515
}
16+
17+
// Set the respective callback's outputs loading state to true before
18+
// sending the request
19+
callbackRequests.forEach((callbackRequest) => {
20+
const outputIds = callbackRequest.outputIds;
21+
outputIds.forEach((outputId) => {
22+
store.setState({
23+
loadingState: {
24+
...loadingState,
25+
[outputId]: true,
26+
},
27+
});
28+
});
29+
});
1630
fetchCallback(callbackRequests, configuration.api).then(
1731
(changeRequestsResult) => {
1832
if (changeRequestsResult.data) {
@@ -23,13 +37,39 @@ export function invokeCallbacks(callbackRequests: CallbackRequest[]) {
2337
);
2438
}
2539
applyStateChangeRequests(changeRequestsResult.data);
40+
// Set the loading state of each output ID of the callback's that
41+
// were invoked to false as the fetch was successful.
42+
callbackRequests.forEach((callbackRequest) => {
43+
const outputIds = callbackRequest.outputIds;
44+
outputIds.forEach((outputId) => {
45+
store.setState({
46+
loadingState: {
47+
...loadingState,
48+
[outputId]: false,
49+
},
50+
});
51+
});
52+
});
2653
} else {
2754
console.error(
2855
"callback failed:",
2956
changeRequestsResult.error,
3057
"for call requests:",
3158
callbackRequests,
3259
);
60+
// Set the loading state of each output ID of the callback's that
61+
// were invoked to `failed` as the fetch was unsuccessful.
62+
callbackRequests.forEach((callbackRequest) => {
63+
const outputIds = callbackRequest.outputIds;
64+
outputIds.forEach((outputId) => {
65+
store.setState({
66+
loadingState: {
67+
...loadingState,
68+
[outputId]: "failed",
69+
},
70+
});
71+
});
72+
});
3373
}
3474
},
3575
);

chartlets.js/packages/lib/src/hooks.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,16 @@ const selectContributionsRecord = (state: StoreState) =>
2020

2121
const selectThemeMode = (state: StoreState) => state.themeMode;
2222

23+
const selectLoadingState = (state: StoreState) => state.loadingState;
24+
2325
const useStore = store;
2426

2527
export const useConfiguration = () => useStore(selectConfiguration);
2628
export const useExtensions = () => useStore(selectExtensions);
2729
export const useContributionsResult = () => useStore(selectContributionsResult);
2830
export const useContributionsRecord = () => useStore(selectContributionsRecord);
2931
export const useThemeMode = () => useStore(selectThemeMode);
32+
export const useLoadingState = () => useStore(selectLoadingState);
3033

3134
/**
3235
* A hook that retrieves the contributions for the given contribution

chartlets.js/packages/lib/src/plugins/mui/Skeleton.tsx

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,27 +11,52 @@ interface SkeletonState extends Omit<ComponentState, "type" | "children"> {
1111
width?: MuiSkeletonProps["width"];
1212
height?: MuiSkeletonProps["height"];
1313
animation?: MuiSkeletonProps["animation"];
14-
loading: boolean;
14+
opacity?: number;
15+
isLoading: boolean;
1516
children?: ReactElement;
1617
}
1718

18-
interface SkeletonProps extends SkeletonState {}
19+
export interface SkeletonProps extends SkeletonState {}
1920

2021
export const Skeleton = ({
2122
id,
2223
style,
23-
loading,
2424
children,
25-
...skeletonProps
25+
isLoading,
26+
...props
2627
}: SkeletonProps) => {
27-
return loading && skeletonProps && Object.keys(skeletonProps).length > 0 ? (
28-
<MuiSkeleton
29-
id={id}
30-
style={style}
31-
{...skeletonProps}
32-
data-testid="skeleton-test-id"
33-
/>
34-
) : (
35-
children
28+
const opacity: number = props.opacity ?? 0.7;
29+
props.width = props.width ?? "100%";
30+
props.height = props.height ?? "100%";
31+
console.log("opacity", opacity);
32+
console.log("props.width", props.width);
33+
console.log("props.height", props.height);
34+
return (
35+
<div style={{ position: "relative", ...style }} id={id}>
36+
{children}
37+
{isLoading && (
38+
<div
39+
style={{
40+
position: "absolute",
41+
top: 0,
42+
left: 0,
43+
right: 0,
44+
bottom: 0,
45+
backgroundColor: `rgba(255, 255, 255, ${opacity})`,
46+
display: "flex",
47+
justifyContent: "center",
48+
alignItems: "center",
49+
zIndex: 1,
50+
}}
51+
>
52+
<MuiSkeleton
53+
id={id}
54+
style={{ transform: "none" }}
55+
{...props}
56+
data-testid="skeleton-test-id"
57+
/>
58+
</div>
59+
)}
60+
</div>
3661
);
3762
};
Lines changed: 34 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import { VegaLite } from "react-vega";
22
import type { TopLevelSpec } from "vega-lite";
33

4-
import type { ComponentProps, ComponentState } from "@/index";
5-
import { useSignalListeners } from "./hooks/useSignalListeners";
4+
import { type ComponentProps, type ComponentState } from "@/index";
65
import { useVegaTheme, type VegaTheme } from "./hooks/useVegaTheme";
7-
import { useEffect, useState } from "react";
6+
import { useSignalListeners } from "@/plugins/vega/hooks/useSignalListeners";
87
import { Skeleton } from "@/plugins/mui/Skeleton";
8+
import { useLoadingState } from "@/hooks";
9+
import type { ReactElement } from "react";
910

1011
interface VegaChartState extends ComponentState {
1112
theme?: VegaTheme | "default" | "system";
@@ -27,29 +28,39 @@ export function VegaChart({
2728
}: VegaChartProps) {
2829
const signalListeners = useSignalListeners(initialChart, type, id, onChange);
2930
const vegaTheme = useVegaTheme(theme);
30-
const [chart, setChart] = useState<TopLevelSpec | null | undefined>(null);
31-
const [loading, setLoading] = useState(true);
3231

33-
useEffect(() => {
34-
if (initialChart) {
35-
setLoading(false);
36-
setChart(initialChart);
37-
}
38-
}, [initialChart]);
32+
const loadingState = useLoadingState();
33+
if (!id) {
34+
return;
35+
}
36+
const isLoading = loadingState[id];
3937

38+
if (isLoading == "failed") {
39+
return <div>An error occurred while loading the data.</div>;
40+
}
41+
const chart: ReactElement | null = initialChart ? (
42+
<VegaLite
43+
theme={vegaTheme}
44+
spec={initialChart}
45+
signalListeners={signalListeners}
46+
actions={false}
47+
/>
48+
) : (
49+
<div />
50+
);
51+
const isSkeletonRequired = skeletonProps !== undefined;
52+
if (!isSkeletonRequired) {
53+
return chart;
54+
}
55+
const skeletonId = id + "-skeleton";
4056
return (
41-
<Skeleton loading={loading} {...skeletonProps}>
42-
{chart ? (
43-
<VegaLite
44-
theme={vegaTheme}
45-
spec={chart}
46-
style={style}
47-
signalListeners={signalListeners}
48-
actions={false}
49-
/>
50-
) : (
51-
<div id={id} style={style} />
52-
)}
57+
<Skeleton
58+
isLoading={isLoading}
59+
id={skeletonId}
60+
style={style}
61+
{...skeletonProps}
62+
>
63+
{chart}
5364
</Skeleton>
5465
);
5566
}

chartlets.js/packages/lib/src/store.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,6 @@ export const store = create<StoreState>(() => ({
77
extensions: [],
88
contributionsResult: {},
99
contributionsRecord: {},
10+
lastCallbackInputValues: {},
11+
loadingState: {},
1012
}));

chartlets.js/packages/lib/src/types/model/callback.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@ export interface InputRef {
7373

7474
/**
7575
* A `CallbackRequest` is a request to invoke a server-side callback.
76-
* The result from invoking server-side callbacks is a list of `StateChangeRequest`
77-
* instances.
76+
* The result from invoking server-side callbacks is a list of
77+
* `StateChangeRequest` instances.
7878
*/
7979
export interface CallbackRequest extends ContribRef, CallbackRef, InputRef {
8080
/**
@@ -83,11 +83,17 @@ export interface CallbackRequest extends ContribRef, CallbackRef, InputRef {
8383
* as the callback's inputs.
8484
*/
8585
inputValues: unknown[];
86+
/**
87+
* The output IDs of the callback that will be used to update the
88+
* loading state of its respective output.
89+
*/
90+
outputIds: string[];
8691
}
8792

8893
/**
8994
* A `StateChangeRequest` is a request to change the application state.
90-
* Instances of this interface are returned from invoking a server-side callback.
95+
* Instances of this interface are returned from invoking a server-side
96+
* callback.
9197
*/
9298
export interface StateChangeRequest extends ContribRef {
9399
/**

chartlets.js/packages/lib/src/types/state/component.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { type CSSProperties } from "react";
22
import { isObject } from "@/utils/isObject";
33
import { isString } from "@/utils/isString";
4-
import type { SkeletonProps } from "@mui/material";
4+
import type { SkeletonProps } from "@/plugins/mui/Skeleton";
55

66
export type ComponentNode =
77
| ComponentState
@@ -25,7 +25,7 @@ export interface ComponentState {
2525
label?: string;
2626
color?: string;
2727
tooltip?: string;
28-
skeletonProps?: SkeletonProps;
28+
skeletonProps?: Omit<SkeletonProps, "isLoading" | "children">;
2929
}
3030

3131
export interface ContainerState extends ComponentState {

0 commit comments

Comments
 (0)