You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by di...@apache.org on 2024/10/01 11:40:26 UTC

(superset) branch geido/feat/progressive-dashboard-header created (now 3593c6d9b0)

This is an automated email from the ASF dual-hosted git repository.

diegopucci pushed a change to branch geido/feat/progressive-dashboard-header
in repository https://gitbox.apache.org/repos/asf/superset.git


      at 3593c6d9b0 chore(MetadataBar): Add loading state

This branch includes the following new commits:

     new 3593c6d9b0 chore(MetadataBar): Add loading state

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(superset) 01/01: chore(MetadataBar): Add loading state

Posted by di...@apache.org.
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 <di...@gmail.com>
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(