Skip to content

Commit

Permalink
fix(Skeleton): Fix missing slice
Browse files Browse the repository at this point in the history
  • Loading branch information
@geido
geido committed Sep 26, 2024
1 parent ec69380 commit 198a7d7
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 49 deletions.
1 change: 1 addition & 0 deletions superset-frontend/src/dashboard/actions/hydrate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ export const hydrateDashboardInfo = dashboard => (dispatch, getState) => {
const crossFiltersEnabled = isCrossFiltersEnabled(
metadata.cross_filters_enabled,
);
// metadata will be processed by hydrateDashboard action
const { metadata: _, ...dashboardWithoutMetadata } = dashboard;

const dashboardInfo = {
Expand Down
20 changes: 8 additions & 12 deletions superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,7 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
nativeFiltersEnabled,
} = useNativeFilters();

const canOpenFilters = dashboardFiltersOpen && isDashboardHydrated;
// Unblocking the Dashboard rendering unless filters are required
const canShowDashboard = !requiredFirstFilter.length
? isDashboardLayoutHydrated
Expand All @@ -494,24 +495,19 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
(isSticky || standaloneMode ? 0 : MAIN_HEADER_HEIGHT);

const filterBarHeight = `calc(100vh - ${offset}px)`;
const filterBarOffset = dashboardFiltersOpen ? 0 : barTopOffset + 20;
const filterBarOffset = canOpenFilters ? 0 : barTopOffset + 20;

const draggableStyle = useMemo(
() => ({
marginLeft:
dashboardFiltersOpen ||
canOpenFilters ||
editMode ||
!nativeFiltersEnabled ||
filterBarOrientation === FilterBarOrientation.Horizontal
? 0
: -32,
}),
[
dashboardFiltersOpen,
editMode,
filterBarOrientation,
nativeFiltersEnabled,
],
[canOpenFilters, editMode, filterBarOrientation, nativeFiltersEnabled],
);

// If a new tab was added, update the directPathToChild to reflect it
Expand Down Expand Up @@ -587,7 +583,7 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
);

const dashboardContentMarginLeft =
!dashboardFiltersOpen &&
!canOpenFilters &&
!editMode &&
nativeFiltersEnabled &&
filterBarOrientation !== FilterBarOrientation.Horizontal
Expand All @@ -601,13 +597,13 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
<>
<ResizableSidebar
id={`dashboard:${dashboardId}`}
enable={dashboardFiltersOpen}
enable={canOpenFilters}
minWidth={OPEN_FILTER_BAR_WIDTH}
maxWidth={OPEN_FILTER_BAR_MAX_WIDTH}
initialWidth={OPEN_FILTER_BAR_WIDTH}
>
{adjustedWidth => {
const filterBarWidth = dashboardFiltersOpen
const filterBarWidth = canOpenFilters
? adjustedWidth
: CLOSED_FILTER_BAR_WIDTH;
return (
Expand All @@ -621,7 +617,7 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
<FilterBar
orientation={FilterBarOrientation.Vertical}
verticalConfig={{
filtersOpen: dashboardFiltersOpen,
filtersOpen: canOpenFilters,
toggleFiltersBar: toggleDashboardFiltersOpen,
width: filterBarWidth,
height: filterBarHeight,
Expand Down
8 changes: 5 additions & 3 deletions superset-frontend/src/dashboard/components/Header/index.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,7 @@ class Header extends PureComponent {
user,
dashboardInfo,
dashboardInfoHydrated,
dashboardLayoutHydrated,
dashboardHydrated,
hasUnsavedChanges,
isLoading,
Expand All @@ -516,6 +517,7 @@ class Header extends PureComponent {
dashboardInfo.common?.conf
?.SUPERSET_DASHBOARD_PERIODICAL_REFRESH_WARNING_MESSAGE;
const dashboardId = dashboardInfoHydrated && dashboardInfo.id;
const dashboardInfoTitle = dashboardInfoHydrated && dashboardInfo.dashboard_title;
const certifiedBy = dashboardInfoHydrated && dashboardInfo.certified_by;
const certificationDetails = dashboardInfoHydrated && dashboardInfo.certification_details;
const isStarred = dashboardInfoHydrated && this.props.isStarred;
Expand Down Expand Up @@ -548,13 +550,13 @@ class Header extends PureComponent {
>
<PageHeaderWithActions
editableTitleProps={{
title: dashboardTitle,
title: dashboardTitle || dashboardInfoTitle,
canEdit: userCanEdit && editMode,
onSave: this.handleChangeText,
placeholder: t('Add the name of the dashboard'),
label: t('Dashboard title'),
showTooltip: false,
loading: !dashboardHydrated,
loading: !dashboardLayoutHydrated,
}}
certificatiedBadgeProps={{
certifiedBy: certifiedBy,
Expand Down Expand Up @@ -655,7 +657,7 @@ class Header extends PureComponent {
data-test="header-save-button"
aria-label={t('Save')}
disabled={!hasUnsavedChanges}
loading={isLoading || !dashboardHydrated}
loading={isLoading}
>
{t('Save')}
</Button>
Expand Down
7 changes: 5 additions & 2 deletions superset-frontend/src/dashboard/components/SliceHeader/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ const SliceHeader: FC<SliceHeaderProps> = ({
width,
height,
}) => {
if (!slice) {
return null;
}

const SliceHeaderExtension = extensionsRegistry.get('dashboard.slice.header');
const uiConfig = useUiConfig();
const dashboardPageId = useContext(DashboardPageIdContext);
Expand All @@ -171,6 +175,7 @@ const SliceHeader: FC<SliceHeaderProps> = ({
);

const canExplore = !editMode && supersetCanExplore;
const exploreUrl = `/explore/?dashboard_page_id=${dashboardPageId}&slice_id=${slice.slice_id}`;

useEffect(() => {
const headerElement = headerRef.current;
Expand All @@ -187,8 +192,6 @@ const SliceHeader: FC<SliceHeaderProps> = ({
}
}, [sliceName, width, height, canExplore]);

const exploreUrl = `/explore/?dashboard_page_id=${dashboardPageId}&slice_id=${slice.slice_id}`;

return (
<ChartHeaderStyles data-test="slice-header" ref={innerRef}>
<div className="header-title" ref={headerRef}>
Expand Down
32 changes: 15 additions & 17 deletions superset-frontend/src/dashboard/components/gridComponents/Chart.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,21 +410,19 @@ class Chart extends Component {
emitCrossFilters,
logEvent,
} = this.props;

const { width } = this.state;
// this prevents throwing in the case that a gridComponent
// references a chart that is not associated with the dashboard
if (!chart || !slice) {
return <MissingChart height={this.getChartHeight()} />;
}

const { queriesResponse, chartUpdateEndTime, chartStatus } = chart;

// Controlling the status of a Chart based on Dashboard hydration
const { isDashboardHydrated } = this.props;
const { queriesResponse, chartUpdateEndTime, chartStatus } = chart;
const controlledChartStatus = isDashboardHydrated ? chartStatus : 'loading';
const { triggerQuery } = chart;
const { width } = this.state;
const isLoading = controlledChartStatus === 'loading';

// this prevents throwing in the case that a gridComponent
// references a chart that is not associated with the dashboard
if ((!chart || !slice) && isDashboardHydrated) {
return <MissingChart height={this.getChartHeight()} />;
}

// eslint-disable-next-line camelcase
const isCached = queriesResponse?.map(({ is_cached }) => is_cached) || [];
Expand All @@ -438,8 +436,8 @@ class Chart extends Component {
className="chart-slice"
data-test="chart-grid-component"
data-test-chart-id={id}
data-test-viz-type={slice.viz_type}
data-test-chart-name={slice.slice_name}
data-test-viz-type={slice?.viz_type}
data-test-chart-name={slice?.slice_name}
>
<SliceHeader
innerRef={this.setHeaderRef}
Expand Down Expand Up @@ -485,7 +483,7 @@ class Chart extends Component {
and
https://github.com/apache/superset/pull/23862
*/}
{isExpanded && slice.description_markeddown && (
{isExpanded && slice?.description_markeddown && (
<div
className="slice_description bs-callout bs-callout-default"
ref={this.setDescriptionRef}
Expand All @@ -497,7 +495,7 @@ class Chart extends Component {

<ChartWrapper
className={cx('dashboard-chart')}
aria-label={slice.description}
aria-label={slice?.description}
>
{isLoading && (
<ChartOverlay
Expand All @@ -514,8 +512,8 @@ class Chart extends Component {
addFilter={this.changeFilter}
onFilterMenuOpen={this.handleFilterMenuOpen}
onFilterMenuClose={this.handleFilterMenuClose}
annotationData={chart.annotationData}
chartAlert={chart.chartAlert}
annotationData={chart?.annotationData}
chartAlert={chart?.chartAlert}
chartId={id}
chartStatus={controlledChartStatus}
datasource={datasource}
Expand All @@ -529,7 +527,7 @@ class Chart extends Component {
queriesResponse={chart.queriesResponse}
timeout={timeout}
triggerQuery={triggerQuery}
vizType={slice.viz_type}
vizType={slice?.viz_type}
setControlValue={setControlValue}
postTransformProps={postTransformProps}
datasetsStatus={datasetsStatus}
Expand Down
7 changes: 5 additions & 2 deletions superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,9 @@ const FilterBar: FC<FiltersBarProps> = ({
dataMaskApplied,
filtersInScope.filter(isNativeFilter),
);
const isDashboardHydrated = useSelector<RootState, boolean>(
state => state.dashboardState.dashboardHydrated,
);
const isInitialized = useInitialization();

const actions = (
Expand All @@ -296,7 +299,7 @@ const FilterBar: FC<FiltersBarProps> = ({
dashboardId={dashboardId}
dataMaskSelected={dataMaskSelected}
filterValues={filterValues}
isInitialized={isInitialized}
isInitialized={isInitialized && isDashboardHydrated}
onSelectionChange={handleFilterSelectionChange}
/>
) : verticalConfig ? (
Expand All @@ -306,7 +309,7 @@ const FilterBar: FC<FiltersBarProps> = ({
dataMaskSelected={dataMaskSelected}
filtersOpen={verticalConfig.filtersOpen}
filterValues={filterValues}
isInitialized={isInitialized}
isInitialized={isInitialized && isDashboardHydrated}
height={verticalConfig.height}
offset={verticalConfig.offset}
onSelectionChange={handleFilterSelectionChange}
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/dashboard/containers/Chart.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ function mapStateToProps(
filters: getActiveFilters() || EMPTY_OBJECT,
formData,
editMode: dashboardState.editMode,
isExpanded: !!dashboardState.expandedSlices[id],
isExpanded: !!dashboardState.expandedSlices?.[id],
supersetCanExplore: !!dashboardInfo.superset_can_explore,
supersetCanShare: !!dashboardInfo.superset_can_share,
supersetCanCSV: !!dashboardInfo.superset_can_csv,
Expand Down
3 changes: 2 additions & 1 deletion superset-frontend/src/dashboard/containers/DashboardHeader.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ function mapStateToProps({
dashboardInfo,
dashboardInfoHydrated: dashboardState.dashboardInfoHydrated,
dashboardHydrated: dashboardState.dashboardHydrated,
dashboardLayoutHydrated: dashboardState.dashboardLayoutHydrated,
undoLength: undoableLayout.past.length,
redoLength: undoableLayout.future.length,
layout: undoableLayout.present,
Expand All @@ -89,7 +90,7 @@ function mapStateToProps({
user,
isStarred: !!dashboardState.isStarred,
isPublished: !!dashboardState.isPublished,
isLoading: isDashboardLoading(charts),
isLoading: isDashboardLoading(charts) || !dashboardState.dashboardHydrated,
hasUnsavedChanges: !!dashboardState.hasUnsavedChanges,
maxUndoHistoryExceeded: !!dashboardState.maxUndoHistoryExceeded,
lastModifiedTime: Math.max(
Expand Down
29 changes: 18 additions & 11 deletions superset-frontend/src/dashboard/containers/DashboardPage.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,12 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
isDashboardHydrated,
isDashboardDatamaskHydrated,
isDashboardInfoHydrated,
] = useSelector<RootState, [boolean, boolean, boolean]>(state => [
isDashboardLayoutHydrated,
] = useSelector<RootState, [boolean, boolean, boolean, boolean]>(state => [
state.dashboardState.dashboardHydrated,
state.dashboardState.dataMaskHydrated,
state.dashboardState.dashboardInfoHydrated,
state.dashboardState.dashboardLayoutHydrated,
]);
const currentDataMask = useSelector<RootState, DataMaskStateWithId>(
state => state.dataMask,
Expand All @@ -120,7 +122,8 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
const readyToHydrate =
Boolean(dashboard && charts) &&
!isDashboardHydrated &&
isDashboardInfoHydrated;
isDashboardInfoHydrated &&
isDashboardLayoutHydrated;
const { dashboard_title, css, id = 0 } = dashboard || {};

useEffect(() => {
Expand Down Expand Up @@ -153,12 +156,14 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
useEffect(() => {
if (dashboard && !isDashboardInfoHydrated) {
dispatch(hydrateDashboardInfo(dashboard));
}
if (dashboard && !isDashboardLayoutHydrated) {
dispatch(hydrateDashboardInitialLayout(dashboard));
}
}, [dashboard, isDashboardInfoHydrated, dispatch]);
}, [dashboard, isDashboardInfoHydrated, dispatch, isDashboardLayoutHydrated]);

/**
* Hydrate dashboard with layout and filters.
* Final dashboard hydration with layout and filters.
*/
useEffect(() => {
if (readyToHydrate) {
Expand All @@ -173,7 +178,7 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
}, [charts, dashboard, dispatch, history, readyToHydrate]);

/**
* Hydrate dashboard data mask and active tabs.
* Hydrate dataMask and active tabs.
* Depends on dashboard layout and filters being hydrated.
*/
useEffect(() => {
Expand All @@ -196,15 +201,11 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
dataMask = isOldRison;
}

if (activeTabs?.length) {
dispatch(hydrateDashboardActiveTabs(activeTabs));
}

dispatch(hydrateDashboardActiveTabs(activeTabs));
dispatch(hydrateDashboardDataMask(dataMask, dashboardInfo));
}
if (
id &&
dashboardInfo &&
isDashboardHydrated &&
!isDashboardDatamaskHydrated &&
!Object.keys(currentDataMask).length &&
Expand All @@ -213,7 +214,6 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
getDataMaskApplied();
}
if (
dashboardInfo &&
isDashboardHydrated &&
!isDashboardDatamaskHydrated &&
Object.keys(currentDataMask).length
Expand Down Expand Up @@ -260,6 +260,13 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {

if (error) throw error; // caught in error boundary

// TODO: @geido some charts are not loading
// TODO: @geido when adding a chart to dashboard, position metadata is not saved (save the chart, generate position, and put dashboard)
// TODO: @geido can we pre-render the charts
// TODO: @geido a bunch of warnings
// TODO: @geido warning showing up quickly when pre-filter even though the filter exists
// TODO: @geido test with dashboard virtualization OFF

return (
<>
<Global
Expand Down

0 comments on commit 198a7d7

Please sign in to comment.