From 8786c23a50b07872a038d3e46188b130f0edd1ee Mon Sep 17 00:00:00 2001
From: Denis Berezin <denis.berezin@metabase.com>
Date: Mon, 1 Jul 2024 18:10:59 +0300
Subject: [PATCH] feat(sdk): Improve dashboard and question loaders to show in
 the middle (#44710)

* Fix SDK components loaders

* Review fixes
---
 .../private/InteractiveQuestionResult.tsx     | 30 +++++----
 .../private/PublicComponentStylesWrapper.tsx  |  4 ++
 .../PublicComponentWrapper.tsx                | 14 ++---
 .../PublicComponentWrapper/SdkError.tsx       | 19 +++---
 .../PublicComponentWrapper/SdkLoader.tsx      | 10 ++-
 .../InteractiveDashboard.tsx                  | 62 +++++++++----------
 6 files changed, 78 insertions(+), 61 deletions(-)

diff --git a/enterprise/frontend/src/embedding-sdk/components/private/InteractiveQuestionResult.tsx b/enterprise/frontend/src/embedding-sdk/components/private/InteractiveQuestionResult.tsx
index 91fc9faf397..8b8377073fc 100644
--- a/enterprise/frontend/src/embedding-sdk/components/private/InteractiveQuestionResult.tsx
+++ b/enterprise/frontend/src/embedding-sdk/components/private/InteractiveQuestionResult.tsx
@@ -80,20 +80,14 @@ export const InteractiveQuestionResult = ({
   const { defaultHeight, isQueryRunning, queryResults, question } =
     useInteractiveQuestionData();
 
-  if (isQuestionLoading || isQueryRunning) {
-    return <SdkLoader />;
-  }
+  let content;
 
-  if (!question || !queryResults) {
-    return <SdkError message={t`Question not found`} />;
-  }
-
-  return (
-    <Box
-      className={cx(CS.flexFull, CS.fullWidth)}
-      h={height ?? defaultHeight}
-      bg="var(--mb-color-bg-question)"
-    >
+  if (isQuestionLoading || isQueryRunning) {
+    content = <SdkLoader />;
+  } else if (!question || !queryResults) {
+    content = <SdkError message={t`Question not found`} />;
+  } else {
+    content = (
       <Stack h="100%">
         <Flex direction="row" gap="md" px="md" align="center">
           <BackButton />
@@ -124,6 +118,16 @@ export const InteractiveQuestionResult = ({
           />
         </Group>
       </Stack>
+    );
+  }
+
+  return (
+    <Box
+      className={cx(CS.flexFull, CS.fullWidth)}
+      h={height ?? defaultHeight}
+      bg="var(--mb-color-bg-question)"
+    >
+      {content}
     </Box>
   );
 };
diff --git a/enterprise/frontend/src/embedding-sdk/components/private/PublicComponentStylesWrapper.tsx b/enterprise/frontend/src/embedding-sdk/components/private/PublicComponentStylesWrapper.tsx
index d85d79d3c45..e50fe1bdf83 100644
--- a/enterprise/frontend/src/embedding-sdk/components/private/PublicComponentStylesWrapper.tsx
+++ b/enterprise/frontend/src/embedding-sdk/components/private/PublicComponentStylesWrapper.tsx
@@ -11,6 +11,10 @@ import { saveDomImageStyles } from "metabase/visualizations/lib/save-chart-image
  */
 export const PublicComponentStylesWrapper = styled.div`
   width: 100%;
+  height: 100%;
+
+  position: relative;
+
   font-weight: 400;
   color: var(--mb-color-text-dark);
   font-family: var(--mb-default-font-family), sans-serif;
diff --git a/enterprise/frontend/src/embedding-sdk/components/private/PublicComponentWrapper/PublicComponentWrapper.tsx b/enterprise/frontend/src/embedding-sdk/components/private/PublicComponentWrapper/PublicComponentWrapper.tsx
index 935ae760a48..decc59bc640 100644
--- a/enterprise/frontend/src/embedding-sdk/components/private/PublicComponentWrapper/PublicComponentWrapper.tsx
+++ b/enterprise/frontend/src/embedding-sdk/components/private/PublicComponentWrapper/PublicComponentWrapper.tsx
@@ -14,23 +14,23 @@ export const PublicComponentWrapper = ({
 }) => {
   const loginStatus = useSdkSelector(getLoginStatus);
 
+  let content = children;
+
   if (loginStatus.status === "uninitialized") {
-    return <div>{t`Initializing…`}</div>;
+    content = <div>{t`Initializing…`}</div>;
   }
 
   if (loginStatus.status === "validated") {
-    return <div>{t`JWT is valid.`}</div>;
+    content = <div>{t`JWT is valid.`}</div>;
   }
 
   if (loginStatus.status === "loading") {
-    return <SdkLoader />;
+    content = <SdkLoader />;
   }
 
   if (loginStatus.status === "error") {
-    return <SdkError message={loginStatus.error.message} />;
+    content = <SdkError message={loginStatus.error.message} />;
   }
 
-  return (
-    <PublicComponentStylesWrapper>{children}</PublicComponentStylesWrapper>
-  );
+  return <PublicComponentStylesWrapper>{content}</PublicComponentStylesWrapper>;
 };
diff --git a/enterprise/frontend/src/embedding-sdk/components/private/PublicComponentWrapper/SdkError.tsx b/enterprise/frontend/src/embedding-sdk/components/private/PublicComponentWrapper/SdkError.tsx
index 9f984be73df..c8667bf4dc9 100644
--- a/enterprise/frontend/src/embedding-sdk/components/private/PublicComponentWrapper/SdkError.tsx
+++ b/enterprise/frontend/src/embedding-sdk/components/private/PublicComponentWrapper/SdkError.tsx
@@ -2,20 +2,25 @@ import { t } from "ttag";
 
 import { useSdkSelector } from "embedding-sdk/store";
 import { getErrorComponent } from "embedding-sdk/store/selectors";
+import { Center } from "metabase/ui";
 
 export type SdkErrorProps = { message: string };
 
 export const SdkError = ({ message }: SdkErrorProps) => {
   const CustomError = useSdkSelector(getErrorComponent);
 
-  if (CustomError) {
-    return <CustomError message={message} />;
-  }
+  const ErrorMessageComponent = CustomError || DefaultErrorMessage;
 
   return (
-    <div>
-      <div>{t`Error`}</div>
-      <div>{message}</div>
-    </div>
+    <Center h="100%" w="100%" mx="auto">
+      <ErrorMessageComponent message={message} />
+    </Center>
   );
 };
+
+const DefaultErrorMessage = ({ message }: { message: string }) => (
+  <div>
+    <div>{t`Error`}</div>
+    <div>{message}</div>
+  </div>
+);
diff --git a/enterprise/frontend/src/embedding-sdk/components/private/PublicComponentWrapper/SdkLoader.tsx b/enterprise/frontend/src/embedding-sdk/components/private/PublicComponentWrapper/SdkLoader.tsx
index 3cbe19486d3..c5c5902f8b1 100644
--- a/enterprise/frontend/src/embedding-sdk/components/private/PublicComponentWrapper/SdkLoader.tsx
+++ b/enterprise/frontend/src/embedding-sdk/components/private/PublicComponentWrapper/SdkLoader.tsx
@@ -1,11 +1,15 @@
 import { useSdkSelector } from "embedding-sdk/store";
 import { getLoaderComponent } from "embedding-sdk/store/selectors";
-import { Loader } from "metabase/ui";
+import { Loader, Center } from "metabase/ui";
 
-export const SdkLoader = () => {
+export const SdkLoader = ({ className }: { className?: string }) => {
   const CustomLoader = useSdkSelector(getLoaderComponent);
 
   const LoaderComponent = CustomLoader || Loader;
 
-  return <LoaderComponent data-testid="loading-indicator" />;
+  return (
+    <Center className={className} h="100%" w="100%" mx="auto">
+      <LoaderComponent data-testid="loading-indicator" />
+    </Center>
+  );
 };
diff --git a/enterprise/frontend/src/embedding-sdk/components/public/InteractiveDashboard/InteractiveDashboard.tsx b/enterprise/frontend/src/embedding-sdk/components/public/InteractiveDashboard/InteractiveDashboard.tsx
index 585d5ec5320..950d8005673 100644
--- a/enterprise/frontend/src/embedding-sdk/components/public/InteractiveDashboard/InteractiveDashboard.tsx
+++ b/enterprise/frontend/src/embedding-sdk/components/public/InteractiveDashboard/InteractiveDashboard.tsx
@@ -8,7 +8,6 @@ import {
   type SdkDashboardDisplayProps,
   useSdkDashboardParams,
 } from "embedding-sdk/hooks/private/use-sdk-dashboard-params";
-import CS from "metabase/css/core/index.css";
 import { NAVIGATE_TO_NEW_CARD, reset } from "metabase/dashboard/actions";
 import { getNewCardUrl } from "metabase/dashboard/actions/getNewCardUrl";
 import type { NavigateToNewCardFromDashboardOpts } from "metabase/dashboard/components/DashCard/types";
@@ -24,6 +23,8 @@ import type { QuestionDashboardCard } from "metabase-types/api";
 export type InteractiveDashboardProps = SdkDashboardDisplayProps & {
   questionHeight?: number;
   questionPlugins?: SdkClickActionPluginsConfig;
+
+  className?: string;
 };
 
 const InteractiveDashboardInner = ({
@@ -35,6 +36,7 @@ const InteractiveDashboardInner = ({
   hiddenParameters = [],
   questionHeight,
   questionPlugins,
+  className,
 }: InteractiveDashboardProps) => {
   const {
     displayOptions,
@@ -108,37 +110,35 @@ const InteractiveDashboardInner = ({
     setAdhocQuestionUrl(null);
   };
 
-  if (adhocQuestionUrl) {
-    return (
-      <InteractiveAdHocQuestion
-        questionPath={adhocQuestionUrl}
-        withTitle={withTitle}
-        height={questionHeight}
-        plugins={questionPlugins}
-        onNavigateBack={handleNavigateBackToDashboard}
-      />
-    );
-  }
-
   return (
-    <Box w="100%" ref={ref} className={CS.overflowAuto}>
-      <PublicOrEmbeddedDashboard
-        dashboardId={dashboardId}
-        parameterQueryParams={initialParameterValues}
-        hideDownloadButton={displayOptions.hideDownloadButton}
-        hideParameters={displayOptions.hideParameters}
-        titled={displayOptions.titled}
-        cardTitled={withCardTitle}
-        theme={theme}
-        isFullscreen={isFullscreen}
-        onFullscreenChange={onFullscreenChange}
-        refreshPeriod={refreshPeriod}
-        onRefreshPeriodChange={onRefreshPeriodChange}
-        setRefreshElapsedHook={setRefreshElapsedHook}
-        font={font}
-        bordered={displayOptions.bordered}
-        navigateToNewCardFromDashboard={handleNavigateToNewCardFromDashboard}
-      />
+    <Box w="100%" h="100%" ref={ref} className={className}>
+      {adhocQuestionUrl ? (
+        <InteractiveAdHocQuestion
+          questionPath={adhocQuestionUrl}
+          withTitle={withTitle}
+          height={questionHeight}
+          plugins={questionPlugins}
+          onNavigateBack={handleNavigateBackToDashboard}
+        />
+      ) : (
+        <PublicOrEmbeddedDashboard
+          dashboardId={dashboardId}
+          parameterQueryParams={initialParameterValues}
+          hideDownloadButton={displayOptions.hideDownloadButton}
+          hideParameters={displayOptions.hideParameters}
+          titled={displayOptions.titled}
+          cardTitled={withCardTitle}
+          theme={theme}
+          isFullscreen={isFullscreen}
+          onFullscreenChange={onFullscreenChange}
+          refreshPeriod={refreshPeriod}
+          onRefreshPeriodChange={onRefreshPeriodChange}
+          setRefreshElapsedHook={setRefreshElapsedHook}
+          font={font}
+          bordered={displayOptions.bordered}
+          navigateToNewCardFromDashboard={handleNavigateToNewCardFromDashboard}
+        />
+      )}
     </Box>
   );
 };
-- 
GitLab