Fix embed loader not translated with #locale (#50841) (#51030)

* Fix embed loader not translated with #locale

* Fix unit tests

* Add E2E tests

* Fix a case where the lcoale is failed to load

* Simplify conditions in `LocaleProvider`



* Make E2E tests faster



---------

Co-authored-by: Mahatthana (Kelvin) Nomsawadi <me@bboykelvin.dev>
Co-authored-by: Nicolò Pretto <info@npretto.com>

e2e/support/helpers/e2e-embedding-helpers.js

@@ -100,7 +100,7 @@ export function getEmbeddedPageUrl(
     const urlHash = getHash(
       {
         ...pageStyle,
-        locale,
+        ...(locale ? { locale } : {}),
       },
       hiddenFilters,
     );

e2e/test/scenarios/embedding/embedding-reproductions.cy.spec.js

 import { H } from "e2e/support";
 import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
 import { ORDERS_DASHBOARD_ID } from "e2e/support/cypress_sample_instance_data";
+import { defer } from "metabase/lib/promise";
 
 const { PRODUCTS, PRODUCTS_ID, ORDERS, ORDERS_ID } = SAMPLE_DATABASE;
 
@@ -1111,8 +1112,85 @@ H.describeEE("issue 8490", () => {
     });
   });
 
+  it("static embeddings with `#locale` should show translate the loading message (metabase#50182)", () => {
+    cy.intercept(
+      {
+        method: "GET",
+        url: "/api/embed/dashboard/*",
+        middleware: true,
+      },
+      req => {
+        req.on("response", res => {
+          const MINUTE = 60 * 1000;
+          res.setDelay(MINUTE);
+        });
+      },
+    ).as("dashboardRequest");
+    cy.intercept(
+      {
+        method: "GET",
+        url: "/api/embed/card/*",
+        middleware: true,
+      },
+      req => {
+        req.on("response", res => {
+          const MINUTE = 60 * 1000;
+          res.setDelay(MINUTE);
+        });
+      },
+    ).as("questionRequest");
+
+    cy.log("test a static embedded dashboard");
+    cy.get("@dashboardId").then(dashboardId => {
+      H.visitEmbeddedPage(
+        {
+          resource: { dashboard: dashboardId },
+          params: {},
+        },
+        {
+          additionalHashOptions: {
+            locale: "ko",
+          },
+        },
+      );
+    });
+
+    // Loading...
+    cy.findByTestId("embed-frame").findByText("로딩...").should("be.visible");
+
+    cy.log("test a static embedded question");
+    cy.get("@lineChartQuestionId").then(lineChartQuestionId => {
+      H.visitEmbeddedPage(
+        {
+          resource: { question: lineChartQuestionId },
+          params: {},
+        },
+        {
+          additionalHashOptions: {
+            locale: "ko",
+          },
+        },
+      );
+    });
+
+    // Loading...
+    cy.findByTestId("embed-frame").findByText("로딩...").should("be.visible");
+  });
+
   it("static embeddings should respect `#locale` hash parameter (metabase#8490, metabase#50182)", () => {
     cy.log("test a static embedded dashboard");
+    const {
+      promise: dashboardLoaderPromise,
+      resolve: resolveDashboardLoaderPromise,
+    } = defer();
+    cy.intercept(
+      {
+        method: "GET",
+        url: "/api/embed/dashboard/*",
+      },
+      () => dashboardLoaderPromise,
+    ).as("dashboardRequest");
+
     cy.get("@dashboardId").then(dashboardId => {
       H.visitEmbeddedPage(
         {
@@ -1128,6 +1206,14 @@ H.describeEE("issue 8490", () => {
     });
 
     cy.findByTestId("embed-frame").within(() => {
+      cy.log(
+        "static embeddings with `#locale` should show a translated the loading message",
+      );
+      // Loading...
+      cy.findByText("로딩...")
+        .should("be.visible")
+        .then(resolveDashboardLoaderPromise);
+
       // PDF export
       cy.findByText("PDF로 내보내기").should("be.visible");
 
@@ -1159,6 +1245,17 @@ H.describeEE("issue 8490", () => {
     });
 
     cy.log("test a static embedded question");
+    const {
+      promise: questionLoaderPromise,
+      resolve: resolveQuestionLoaderPromise,
+    } = defer();
+    cy.intercept(
+      {
+        method: "GET",
+        url: "/api/embed/card/*",
+      },
+      () => questionLoaderPromise,
+    ).as("questionRequest");
 
     cy.log("assert the line chart");
     cy.get("@lineChartQuestionId").then(lineChartQuestionId => {
@@ -1176,6 +1273,14 @@ H.describeEE("issue 8490", () => {
     });
 
     cy.findByTestId("embed-frame").within(() => {
+      cy.log(
+        "static embeddings with `#locale` should show a translated the loading message",
+      );
+      // Loading...
+      cy.findByText("로딩...")
+        .should("be.visible")
+        .then(resolveQuestionLoaderPromise);
+
       // X-axis labels: Jan 2023 (or some other year)
       cy.findByText(/11월 20\d\d\b/).should("be.visible");
       // Aggregation "count"

frontend/src/metabase/components/LoadingAndErrorWrapper/LoadingAndErrorWrapper.jsx

@@ -22,7 +22,7 @@ export default class LoadingAndErrorWrapper extends Component {
     children: PropTypes.any,
     style: PropTypes.object,
     showSpinner: PropTypes.bool,
-    loadingMessages: PropTypes.array,
+    getLoadingMessages: PropTypes.func,
     messageInterval: PropTypes.number,
     loadingScenes: PropTypes.array,
     renderError: PropTypes.func,
@@ -35,7 +35,7 @@ export default class LoadingAndErrorWrapper extends Component {
     noBackground: true,
     noWrapper: false,
     showSpinner: true,
-    loadingMessages: [t`Loading...`],
+    getLoadingMessages: () => [t`Loading...`],
     messageInterval: 6000,
   };
 
@@ -73,9 +73,9 @@ export default class LoadingAndErrorWrapper extends Component {
   }
 
   componentDidMount() {
-    const { loadingMessages, messageInterval } = this.props;
+    const { getLoadingMessages, messageInterval } = this.props;
     // only start cycling if multiple messages are provided
-    if (loadingMessages.length > 1) {
+    if (getLoadingMessages().length > 1) {
       this.cycle = setInterval(this.loadingInterval, messageInterval);
     }
   }
@@ -103,7 +103,7 @@ export default class LoadingAndErrorWrapper extends Component {
   cycleLoadingMessage = () => {
     this.setState({
       messageIndex:
-        this.state.messageIndex + 1 < this.props.loadingMessages.length
+        this.state.messageIndex + 1 < this.props.getLoadingMessages().length
           ? this.state.messageIndex + 1
           : 0,
     });
@@ -116,7 +116,7 @@ export default class LoadingAndErrorWrapper extends Component {
       noBackground,
       noWrapper,
       showSpinner,
-      loadingMessages,
+      getLoadingMessages,
       loadingScenes,
       style,
       className,
@@ -154,7 +154,7 @@ export default class LoadingAndErrorWrapper extends Component {
             {loadingScenes && loadingScenes[sceneIndex]}
             {!loadingScenes && showSpinner && <LoadingSpinner />}
             <h2 className={cx(CS.textNormal, CS.textLight, CS.mt1)}>
-              {loadingMessages[messageIndex]}
+              {getLoadingMessages()[messageIndex]}
             </h2>
           </div>
         ) : (

frontend/src/metabase/components/LoadingAndErrorWrapper/LoadingAndErrorWrapper.unit.spec.js

@@ -49,7 +49,7 @@ describe("LoadingAndErrorWrapper", () => {
           <LoadingAndErrorWrapper
             loading={true}
             error={null}
-            loadingMessages={["One", "Two", "Three"]}
+            getLoadingMessages={() => ["One", "Two", "Three"]}
             messageInterval={interval}
           />,
         );

frontend/src/metabase/public/LocaleProvider.tsx

@@ -3,27 +3,39 @@ import { type PropsWithChildren, useEffect, useState } from "react";
 import { setLocaleHeader } from "metabase/lib/api";
 import { loadLocalization, setUserLocale } from "metabase/lib/i18n";
 
+interface LocaleProviderProps {
+  locale?: string | null;
+  shouldWaitForLocale?: boolean;
+}
+
 export const LocaleProvider = ({
   children,
   locale,
-}: PropsWithChildren<{ locale?: string | null }>) => {
-  // The state is not used explicitly, but we need to trigger a re-render when the locale changes
-  // as changing the locale in ttag doesn't trigger react components to update
-  const [_isLoadingLocale, setIsLoadingLocale] = useState(false);
+  shouldWaitForLocale,
+}: PropsWithChildren<LocaleProviderProps>) => {
+  const shouldLoadLocale = Boolean(locale);
+  const [isLocaleLoading, setIsLocaleLoading] = useState(shouldLoadLocale);
 
   useEffect(() => {
-    if (locale) {
-      setIsLoadingLocale(true);
+    if (shouldLoadLocale) {
       setLocaleHeader(locale);
-      loadLocalization(locale).then(translatedObject => {
-        setIsLoadingLocale(false);
-        setUserLocale(translatedObject);
-      });
+      loadLocalization(locale)
+        .then(translatedObject => {
+          setIsLocaleLoading(false);
+          setUserLocale(translatedObject);
+        })
+        .catch(() => {
+          setIsLocaleLoading(false);
+        });
     }
-  }, [locale]);
+  }, [locale, shouldLoadLocale]);
+
+  if (shouldWaitForLocale && isLocaleLoading) {
+    return null;
+  }
 
   // note: we may show a loader here while loading, this would prevent race
   // conditions and things being rendered for some time with the wrong locale
   // downside is that it would make the initial load slower
-  return <>{children}</>;
+  return children;
 };

frontend/src/metabase/public/containers/PublicOrEmbeddedDashboard/PublicOrEmbeddedDashboard.tsx

@@ -212,7 +212,7 @@ const PublicOrEmbeddedDashboardInner = ({
   });
 
   return (
-    <LocaleProvider locale={locale}>
+    <LocaleProvider locale={locale} shouldWaitForLocale>
       <PublicOrEmbeddedDashboardView
         dashboard={dashboard}
         hasNightModeToggle={hasNightModeToggle}

frontend/src/metabase/public/containers/PublicOrEmbeddedQuestion/PublicOrEmbeddedQuestion/PublicOrEmbeddedQuestion.tsx

@@ -173,7 +173,10 @@ export const PublicOrEmbeddedQuestion = ({
   };
 
   return (
-    <LocaleProvider locale={canWhitelabel ? locale : undefined}>
+    <LocaleProvider
+      locale={canWhitelabel ? locale : undefined}
+      shouldWaitForLocale
+    >
       <PublicOrEmbeddedQuestionView
         initialized={initialized}
         card={card}