This is an automated email from the ASF dual-hosted git repository. diegopucci pushed a commit to branch geido/feat/progressive-dashboard-header in repository https://gitbox.apache.org/repos/asf/superset.git
commit 3593c6d9b0e556a2831df9d64eae37ddf50adb09 Author: Diego Pucci <[email protected]> AuthorDate: Tue Oct 1 14:40:10 2024 +0300 chore(MetadataBar): Add loading state --- .../src/components/MetadataBar/MetadataBar.tsx | 44 ++++++++---- .../src/dashboard/components/Header/index.jsx | 84 ++++++++++++---------- .../src/dashboard/containers/DashboardHeader.jsx | 2 +- 3 files changed, 78 insertions(+), 52 deletions(-) diff --git a/superset-frontend/src/components/MetadataBar/MetadataBar.tsx b/superset-frontend/src/components/MetadataBar/MetadataBar.tsx index 2a58264805..5e652fe8b5 100644 --- a/superset-frontend/src/components/MetadataBar/MetadataBar.tsx +++ b/superset-frontend/src/components/MetadataBar/MetadataBar.tsx @@ -23,6 +23,7 @@ import { styled } from '@superset-ui/core'; import { Tooltip, TooltipPlacement } from 'src/components/Tooltip'; import { ContentType } from './ContentType'; import { config } from './ContentConfig'; +import Loading from '../Loading'; export const MIN_NUMBER_ITEMS = 2; export const MAX_NUMBER_ITEMS = 6; @@ -60,6 +61,10 @@ const Bar = styled.div<{ count: number }>` }px; border-radius: ${theme.borderRadius}px; line-height: 1; + + & .loading { + margin: 0 auto; + } `} `; @@ -181,6 +186,10 @@ export interface MetadataBarProps { * Defaults to "top". */ tooltipPlacement?: TooltipPlacement; + /** + * Loading state. If true, the bar will display loading spinners. + */ + loading?: boolean; } /** @@ -191,16 +200,21 @@ export interface MetadataBarProps { * To extend the list of content types, a developer needs to request the inclusion of the new type in the design system. * This process is important to make sure the new type is reviewed by the design team, improving Superset consistency. */ -const MetadataBar = ({ items, tooltipPlacement = 'top' }: MetadataBarProps) => { +const MetadataBar = ({ + items, + tooltipPlacement = 'top', + loading = false, +}: MetadataBarProps) => { const [width, setWidth] = useState<number>(); const [collapsed, setCollapsed] = useState(false); const uniqueItems = uniqWith(items, (a, b) => a.type === b.type); const sortedItems = uniqueItems.sort((a, b) => ORDER[a.type] - ORDER[b.type]); const count = sortedItems.length; - if (count < MIN_NUMBER_ITEMS) { + + if (!loading && count < MIN_NUMBER_ITEMS) { throw Error('The minimum number of items for the metadata bar is 2.'); } - if (count > MAX_NUMBER_ITEMS) { + if (!loading && count > MAX_NUMBER_ITEMS) { throw Error('The maximum number of items for the metadata bar is 6.'); } @@ -222,16 +236,20 @@ const MetadataBar = ({ items, tooltipPlacement = 'top' }: MetadataBarProps) => { return ( <Bar ref={ref} count={count} data-test="metadata-bar"> - {sortedItems.map((item, index) => ( - <Item - barWidth={width} - key={index} - contentType={item} - collapsed={collapsed} - last={index === count - 1} - tooltipPlacement={tooltipPlacement} - /> - ))} + {!loading ? ( + sortedItems.map((item, index) => ( + <Item + barWidth={width} + key={index} + contentType={item} + collapsed={collapsed} + last={index === count - 1} + tooltipPlacement={tooltipPlacement} + /> + )) + ) : ( + <Loading position="inline" /> + )} </Bar> ); }; diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index 9afaf5534c..4aa92567a9 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -65,10 +65,11 @@ const propTypes = { addDangerToast: PropTypes.func.isRequired, addWarningToast: PropTypes.func.isRequired, user: PropTypes.object, // UserWithPermissionsAndRoles, - dashboardInfo: PropTypes.object.isRequired, + dashboardInfo: PropTypes.object, dashboardTitle: PropTypes.string, dataMask: PropTypes.object.isRequired, charts: PropTypes.objectOf(chartPropShape).isRequired, + chartsLoading: PropTypes.bool.isRequired, layout: PropTypes.object.isRequired, expandedSlices: PropTypes.object, customCss: PropTypes.string, @@ -78,7 +79,6 @@ const propTypes = { setUnsavedChanges: PropTypes.func.isRequired, isStarred: PropTypes.bool.isRequired, isPublished: PropTypes.bool.isRequired, - isLoading: PropTypes.bool.isRequired, onSave: PropTypes.func.isRequired, onChange: PropTypes.func.isRequired, fetchFaveStar: PropTypes.func.isRequired, @@ -270,7 +270,7 @@ class Header extends PureComponent { } forceRefresh() { - if (!this.props.isLoading) { + if (!this.props.chartsLoading) { const chartList = Object.keys(this.props.charts); this.props.logEvent(LOG_ACTIONS_FORCE_REFRESH_DASHBOARD, { force: true, @@ -288,10 +288,10 @@ class Header extends PureComponent { } startPeriodicRender(interval) { - let intervalMessage; + const { dashboardInfo } = this.props; - if (interval) { - const { dashboardInfo } = this.props; + let intervalMessage; + if (interval && dashboardInfo) { const periodicRefreshOptions = dashboardInfo.common?.conf?.DASHBOARD_AUTO_REFRESH_INTERVALS; const predefinedValue = periodicRefreshOptions.find( @@ -340,11 +340,13 @@ class Header extends PureComponent { ); }; - this.refreshTimer = setPeriodicRunner({ - interval, - periodicRender, - refreshTimer: this.refreshTimer, - }); + if (dashboardInfo) { + this.refreshTimer = setPeriodicRunner({ + interval, + periodicRender, + refreshTimer: this.refreshTimer, + }); + } } toggleEditMode() { @@ -434,28 +436,28 @@ class Header extends PureComponent { getMetadataItems = () => { const { dashboardInfo } = this.props; + return [ { type: MetadataType.LastModified, - value: dashboardInfo.changed_on_delta_humanized, + value: dashboardInfo?.changed_on_delta_humanized, modifiedBy: - getOwnerName(dashboardInfo.changed_by) || t('Not available'), + getOwnerName(dashboardInfo?.changed_by) || t('Not available'), }, { type: MetadataType.Owner, - createdBy: getOwnerName(dashboardInfo.created_by) || t('Not available'), + createdBy: getOwnerName(dashboardInfo?.created_by) || t('Not available'), owners: - dashboardInfo.owners.length > 0 - ? dashboardInfo.owners.map(getOwnerName) + dashboardInfo?.owners.length > 0 + ? dashboardInfo?.owners.map(getOwnerName) : t('None'), - createdOn: dashboardInfo.created_on_delta_humanized, + createdOn: dashboardInfo?.created_on_delta_humanized, }, ]; }; render() { const { - dashboardTitle, layout, expandedSlices, customCss, @@ -474,27 +476,28 @@ class Header extends PureComponent { editMode, isPublished, user, - dashboardInfo, hasUnsavedChanges, - isLoading, + chartsLoading, refreshFrequency, shouldPersistRefreshFrequency, setRefreshFrequency, lastModifiedTime, logEvent, } = this.props; + const dashboardInfo = null; + const dashboardTitle = null; const userCanEdit = - dashboardInfo.dash_edit_perm && !dashboardInfo.is_managed_externally; - const userCanShare = dashboardInfo.dash_share_perm; - const userCanSaveAs = dashboardInfo.dash_save_perm; + dashboardInfo?.dash_edit_perm && !dashboardInfo?.is_managed_externally; + const userCanShare = dashboardInfo?.dash_share_perm; + const userCanSaveAs = dashboardInfo?.dash_save_perm; const userCanCurate = isFeatureEnabled(FeatureFlag.EmbeddedSuperset) && findPermission('can_set_embedded', 'Dashboard', user.roles); const refreshLimit = - dashboardInfo.common?.conf?.SUPERSET_DASHBOARD_PERIODICAL_REFRESH_LIMIT; + dashboardInfo?.common?.conf?.SUPERSET_DASHBOARD_PERIODICAL_REFRESH_LIMIT; const refreshWarning = - dashboardInfo.common?.conf + dashboardInfo?.common?.conf ?.SUPERSET_DASHBOARD_PERIODICAL_REFRESH_WARNING_MESSAGE; const handleOnPropertiesChange = updates => { @@ -519,7 +522,7 @@ class Header extends PureComponent { <div css={headerContainerStyle} data-test="dashboard-header-container" - data-test-id={dashboardInfo.id} + data-test-id={dashboardInfo?.id} className="dashboard-header-container" > <PageHeaderWithActions @@ -530,13 +533,14 @@ class Header extends PureComponent { placeholder: t('Add the name of the dashboard'), label: t('Dashboard title'), showTooltip: false, + loading: !dashboardTitle, }} certificatiedBadgeProps={{ - certifiedBy: dashboardInfo.certified_by, - details: dashboardInfo.certification_details, + certifiedBy: dashboardInfo?.certified_by, + details: dashboardInfo?.certification_details, }} faveStarProps={{ - itemId: dashboardInfo.id, + itemId: dashboardInfo?.id, fetchFaveStar: this.props.fetchFaveStar, saveFaveStar: this.props.saveFaveStar, isStarred: this.props.isStarred, @@ -545,7 +549,7 @@ class Header extends PureComponent { titlePanelAdditionalItems={[ !editMode && ( <PublishedStatus - dashboardId={dashboardInfo.id} + dashboardId={dashboardInfo?.id} isPublished={isPublished} savePublished={this.props.savePublished} canEdit={userCanEdit} @@ -557,6 +561,7 @@ class Header extends PureComponent { <MetadataBar items={this.getMetadataItems()} tooltipPlacement="bottom" + loading={!dashboardInfo} /> ), ]} @@ -576,7 +581,7 @@ class Header extends PureComponent { > <StyledUndoRedoButton buttonStyle="link" - disabled={undoLength < 1} + disabled={undoLength < 1 || !dashboardInfo} onClick={undoLength && onUndo} > <Icons.Undo @@ -596,7 +601,7 @@ class Header extends PureComponent { > <StyledUndoRedoButton buttonStyle="link" - disabled={redoLength < 1} + disabled={redoLength < 1 || !dashboardInfo} onClick={redoLength && onRedo} > <Icons.Redo @@ -618,17 +623,20 @@ class Header extends PureComponent { buttonStyle="default" data-test="discard-changes-button" aria-label={t('Discard')} + disabled={!hasUnsavedChanges} + loading={!dashboardInfo} > {t('Discard')} </Button> <Button + aria-label={t('Save')} css={saveBtnStyle} buttonSize="small" - disabled={!hasUnsavedChanges} buttonStyle="primary" onClick={this.overwriteDashboard} data-test="header-save-button" - aria-label={t('Save')} + disabled={!hasUnsavedChanges} + loading={!dashboardInfo} > {t('Save')} </Button> @@ -670,7 +678,7 @@ class Header extends PureComponent { <ConnectedHeaderActionsDropdown addSuccessToast={this.props.addSuccessToast} addDangerToast={this.props.addDangerToast} - dashboardId={dashboardInfo.id} + dashboardId={dashboardInfo?.id} dashboardTitle={dashboardTitle} dashboardInfo={dashboardInfo} dataMask={dataMask} @@ -693,7 +701,7 @@ class Header extends PureComponent { userCanShare={userCanShare} userCanSave={userCanSaveAs} userCanCurate={userCanCurate} - isLoading={isLoading} + isLoading={chartsLoading} showPropertiesModal={this.showPropertiesModal} manageEmbedded={this.showEmbedModal} refreshLimit={refreshLimit} @@ -709,7 +717,7 @@ class Header extends PureComponent { /> {this.state.showingPropertiesModal && ( <PropertiesModal - dashboardId={dashboardInfo.id} + dashboardId={dashboardInfo?.id} dashboardInfo={dashboardInfo} dashboardTitle={dashboardTitle} show={this.state.showingPropertiesModal} @@ -726,7 +734,7 @@ class Header extends PureComponent { <DashboardEmbedModal show={this.state.showingEmbedModal} onHide={this.hideEmbedModal} - dashboardId={dashboardInfo.id} + dashboardId={dashboardInfo?.id} /> )} <Global diff --git a/superset-frontend/src/dashboard/containers/DashboardHeader.jsx b/superset-frontend/src/dashboard/containers/DashboardHeader.jsx index cc05916dcc..177281d1a1 100644 --- a/superset-frontend/src/dashboard/containers/DashboardHeader.jsx +++ b/superset-frontend/src/dashboard/containers/DashboardHeader.jsx @@ -87,7 +87,7 @@ function mapStateToProps({ user, isStarred: !!dashboardState.isStarred, isPublished: !!dashboardState.isPublished, - isLoading: isDashboardLoading(charts), + chartsLoading: isDashboardLoading(charts), hasUnsavedChanges: !!dashboardState.hasUnsavedChanges, maxUndoHistoryExceeded: !!dashboardState.maxUndoHistoryExceeded, lastModifiedTime: Math.max(
