Skip to content

Commit 1991cd3

Browse files
committed
fix: [FIXUP] fix placeholder in Superset dashboards
1 parent 0f6a6b2 commit 1991cd3

File tree

7 files changed

+69
-56
lines changed

7 files changed

+69
-56
lines changed

src/charts/DashboardPlaceholderLayout/DashboardPlaceholderLayout.js renamed to src/charts/DashboardPlaceholder/DashboardPlaceholderLayout.js

File renamed without changes.
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
export { default } from './DashboardPlaceholder';
1+
export { default } from './DashboardPlaceholderLayout';

src/charts/DashboardPlaceholder/DashboardPlaceholder.js renamed to src/charts/DashboardPlaceholder/useDashboardPlaceholder.js

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
// Copyright (c) Cosmo Tech.
22
// Licensed under the MIT license.
3-
import React from 'react';
4-
import PropTypes from 'prop-types';
3+
import React, { useCallback } from 'react';
54
import { RUNNER_RUN_STATE } from '../../common/apiConstants';
6-
import DashboardPlaceholderLayout from '../DashboardPlaceholderLayout';
5+
import DashboardPlaceholderLayout from './DashboardPlaceholderLayout';
76

87
const DEFAULT_LABELS = {
98
noScenario: {
@@ -25,16 +24,15 @@ const DEFAULT_LABELS = {
2524
},
2625
};
2726

28-
const DashboardPlaceholder = (props) => {
29-
const {
30-
alwaysShowReports,
31-
disabled,
32-
downloadLogsFile,
33-
noDashboardConfigured,
34-
labels: tmpLabels,
35-
scenario,
36-
scenarioDTO,
37-
} = props;
27+
const forgeDashboardPlaceholder = ({
28+
alwaysShowReports,
29+
disabled,
30+
downloadLogsFile,
31+
noDashboardConfigured,
32+
labels: tmpLabels,
33+
scenario,
34+
scenarioDTO,
35+
}) => {
3836
const labels = { ...DEFAULT_LABELS, ...tmpLabels };
3937

4038
const noScenario = scenario === null;
@@ -64,14 +62,11 @@ const DashboardPlaceholder = (props) => {
6462
return null;
6563
};
6664

67-
DashboardPlaceholder.propTypes = {
68-
alwaysShowReports: PropTypes.bool,
69-
disabled: PropTypes.bool,
70-
downloadLogsFile: PropTypes.func,
71-
noDashboardConfigured: PropTypes.bool,
72-
scenario: PropTypes.object,
73-
scenarioDTO: PropTypes.object,
74-
labels: PropTypes.object,
75-
};
65+
export const useDashboardPlaceholder = () => {
66+
const getDashboardPlaceholder = useCallback((params) => {
67+
const placeholder = forgeDashboardPlaceholder(params);
68+
return { placeholder, showPlaceholder: placeholder != null };
69+
}, []);
7670

77-
export default DashboardPlaceholder;
71+
return { getDashboardPlaceholder };
72+
};

src/charts/DashboardPlaceholderLayout/index.js

Lines changed: 0 additions & 1 deletion
This file was deleted.

src/charts/PowerBIReport/PowerBIReport.js

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { styled } from '@mui/material/styles';
99
import { PowerBIUtils } from '@cosmotech/azure';
1010
import { RUNNER_RUN_STATE } from '../../common/apiConstants';
1111
import { FadingTooltip } from '../../misc';
12-
import DashboardPlaceholder from '../DashboardPlaceholder';
12+
import { useDashboardPlaceholder } from '../DashboardPlaceholder/useDashboardPlaceholder';
1313

1414
const PREFIX = 'PowerBIReport';
1515
const classes = { report: `${PREFIX}-report` };
@@ -64,6 +64,7 @@ export const PowerBIReport = ({
6464
}) => {
6565
const labels = useMemo(() => ({ ...DEFAULT_LABELS, ...tmpLabels }), [tmpLabels]);
6666
const { reportId, settings, staticFilters, dynamicFilters, pageName } = reportConfiguration[index] || {};
67+
const { getDashboardPlaceholder } = useDashboardPlaceholder();
6768

6869
// 1 Embed or 0 Aad
6970
const tokenType = useAAD ? 0 : 1;
@@ -90,19 +91,27 @@ export const PowerBIReport = ({
9091
[dynamicFilters, scenarioDTO]
9192
);
9293

93-
const placeholder = useMemo(() => {
94-
return (
95-
<DashboardPlaceholder
96-
alwaysShowReports={alwaysShowReports}
97-
disabled={reports?.status === 'DISABLED'}
98-
downloadLogsFile={downloadLogsFile}
99-
noDashboardConfigured={reportConfiguration[index] == null}
100-
scenario={scenario}
101-
scenarioDTO={scenarioDTO}
102-
labels={labels}
103-
/>
104-
);
105-
}, [alwaysShowReports, reports, downloadLogsFile, reportConfiguration, index, scenario, scenarioDTO, labels]);
94+
const { placeholder } = useMemo(() => {
95+
return getDashboardPlaceholder({
96+
alwaysShowReports,
97+
disabled: reports?.status === 'DISABLED',
98+
downloadLogsFile,
99+
noDashboardConfigured: reportConfiguration[index] == null,
100+
scenario,
101+
scenarioDTO,
102+
labels,
103+
});
104+
}, [
105+
getDashboardPlaceholder,
106+
alwaysShowReports,
107+
reports,
108+
downloadLogsFile,
109+
reportConfiguration,
110+
index,
111+
scenario,
112+
scenarioDTO,
113+
labels,
114+
]);
106115

107116
useEffect(() => {
108117
const newConfig = {

src/charts/SupersetReport/SupersetReport.js

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import PropTypes from 'prop-types';
55
import { Box, CircularProgress, Backdrop } from '@mui/material';
66
import { embedDashboard } from '@superset-ui/embedded-sdk';
77
import { PowerBIUtils } from '@cosmotech/azure';
8-
import DashboardPlaceholder from '../DashboardPlaceholder';
8+
import { useDashboardPlaceholder } from '../DashboardPlaceholder/useDashboardPlaceholder';
99

1010
export const SupersetReport = ({
1111
alwaysShowReports = false,
@@ -26,6 +26,8 @@ export const SupersetReport = ({
2626
const [isEmbedded, setIsEmbedded] = useState(false);
2727
const [dashboard, setDashboard] = useState(null);
2828

29+
const { getDashboardPlaceholder } = useDashboardPlaceholder();
30+
2931
useEffect(() => {
3032
tokenRef.current = guestToken;
3133
}, [guestToken]);
@@ -64,24 +66,32 @@ export const SupersetReport = ({
6466
// eslint-disable-next-line react-hooks/exhaustive-deps
6567
}, [report.id, options.supersetUrl, guestToken]);
6668

67-
const placeholder = useMemo(() => {
69+
const { placeholder, showPlaceholder } = useMemo(() => {
6870
const scenarioDTO = PowerBIUtils.constructScenarioDTO(scenario, visibleScenarios);
69-
return (
70-
<DashboardPlaceholder
71-
alwaysShowReports={alwaysShowReports}
72-
disabled={disabled}
73-
downloadLogsFile={downloadLogsFile}
74-
noDashboardConfigured={report == null}
75-
scenario={scenario}
76-
scenarioDTO={scenarioDTO}
77-
labels={labels}
78-
/>
79-
);
80-
}, [alwaysShowReports, disabled, downloadLogsFile, report, scenario, visibleScenarios, labels]);
71+
return getDashboardPlaceholder({
72+
alwaysShowReports,
73+
disabled,
74+
downloadLogsFile,
75+
noDashboardConfigured: report == null,
76+
scenario,
77+
scenarioDTO,
78+
labels,
79+
});
80+
}, [
81+
getDashboardPlaceholder,
82+
alwaysShowReports,
83+
disabled,
84+
downloadLogsFile,
85+
report,
86+
scenario,
87+
visibleScenarios,
88+
labels,
89+
]);
8190

82-
const isReportVisible = isEmbedded && placeholder == null;
91+
const isReportVisible = isEmbedded && !showPlaceholder;
8392
const reportContainerDisplay = isReportVisible ? undefined : 'none';
84-
const showLoadingSpinner = placeholder == null && (isLoading || !isEmbedded)
93+
const showLoadingSpinner = placeholder == null && (isLoading || !isEmbedded);
94+
8595
return (
8696
<Box
8797
sx={{

src/misc/ErrorBoundary/ErrorBoundary.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Licensed under the MIT license.
33
import React from 'react';
44
import PropTypes from 'prop-types';
5-
import DashboardPlaceholderLayout from '../../charts/DashboardPlaceholderLayout';
5+
import DashboardPlaceholderLayout from '../../charts/DashboardPlaceholder';
66

77
// As of React 17.0, error boundaries can only be implemented with class components
88
class ErrorBoundary extends React.Component {

0 commit comments

Comments
 (0)