From 504840b85ac0c0fae79facc892fd7d427e6dbe4b Mon Sep 17 00:00:00 2001 From: Denis Berezin Date: Fri, 22 Mar 2024 19:44:30 +0300 Subject: [PATCH] Review fixes --- .../private/AppInitializeController.tsx | 48 +++++++++++++++ .../{ => private}/SdkContentWrapper.tsx | 0 .../components/public/MetabaseProvider.tsx | 60 ++++++------------- .../components/public/QueryVisualization.tsx | 14 ++--- .../frontend/src/embedding-sdk/context.ts | 4 -- .../src/embedding-sdk/hooks/private/index.ts | 1 - .../hooks/private/use-init-data.ts | 18 +++--- .../hooks/private/use-sdk-context.ts | 7 --- .../hooks/public/use-available-fonts.ts | 8 +-- .../hooks/public/use-question-search.ts | 2 +- .../frontend/src/embedding-sdk/styles.css | 1 - .../src/embedding-sdk/{config.ts => types.ts} | 0 12 files changed, 85 insertions(+), 78 deletions(-) create mode 100644 enterprise/frontend/src/embedding-sdk/components/private/AppInitializeController.tsx rename enterprise/frontend/src/embedding-sdk/components/{ => private}/SdkContentWrapper.tsx (100%) delete mode 100644 enterprise/frontend/src/embedding-sdk/hooks/private/use-sdk-context.ts delete mode 100644 enterprise/frontend/src/embedding-sdk/styles.css rename enterprise/frontend/src/embedding-sdk/{config.ts => types.ts} (100%) diff --git a/enterprise/frontend/src/embedding-sdk/components/private/AppInitializeController.tsx b/enterprise/frontend/src/embedding-sdk/components/private/AppInitializeController.tsx new file mode 100644 index 0000000000000..7ac7a0505326a --- /dev/null +++ b/enterprise/frontend/src/embedding-sdk/components/private/AppInitializeController.tsx @@ -0,0 +1,48 @@ +import type * as React from "react"; +import { useEffect } from "react"; +import { t } from "ttag"; + +import { useDispatch } from "metabase/lib/redux"; +import { setOptions } from "metabase/redux/embed"; + +import { EmbeddingContext } from "../../context"; +import { useInitData } from "../../hooks"; +import type { SDKConfigType } from "../../types"; + +import { SdkContentWrapper } from "./SdkContentWrapper"; + +interface AppInitializeControllerProps { + children: React.ReactNode; + config: SDKConfigType; +} + +export const AppInitializeController = ({ + config, + children, +}: AppInitializeControllerProps) => { + const { isLoggedIn, isInitialized } = useInitData({ + config, + }); + const dispatch = useDispatch(); + + const font = config.font ?? "Lato"; + + useEffect(() => { + if (font) { + dispatch(setOptions({ font })); + } + }, [dispatch, font]); + + return ( + + + {!isInitialized ?
{t`Loading…`}
: children} + + + ); +}; diff --git a/enterprise/frontend/src/embedding-sdk/components/SdkContentWrapper.tsx b/enterprise/frontend/src/embedding-sdk/components/private/SdkContentWrapper.tsx similarity index 100% rename from enterprise/frontend/src/embedding-sdk/components/SdkContentWrapper.tsx rename to enterprise/frontend/src/embedding-sdk/components/private/SdkContentWrapper.tsx diff --git a/enterprise/frontend/src/embedding-sdk/components/public/MetabaseProvider.tsx b/enterprise/frontend/src/embedding-sdk/components/public/MetabaseProvider.tsx index f437f2d95e059..243024f557cdd 100644 --- a/enterprise/frontend/src/embedding-sdk/components/public/MetabaseProvider.tsx +++ b/enterprise/frontend/src/embedding-sdk/components/public/MetabaseProvider.tsx @@ -1,20 +1,21 @@ +import type { AnyAction, Store } from "@reduxjs/toolkit"; import type * as React from "react"; -import { memo, useEffect, useState } from "react"; +import { memo } from "react"; import { Provider } from "react-redux"; import reducers from "metabase/reducers-main"; -import { setOptions } from "metabase/redux/embed"; import { getStore } from "metabase/store"; import { EmotionCacheProvider } from "metabase/styled-components/components/EmotionCacheProvider"; import { ThemeProvider } from "metabase/ui/components/theme/ThemeProvider"; +import type { State } from "metabase-types/store"; -import type { SDKConfigType } from "../../config"; -import { EmbeddingContext } from "../../context"; -import { useInitData } from "../../hooks"; -import { SdkContentWrapper } from "../SdkContentWrapper"; +import type { SDKConfigType } from "../../types"; +import { AppInitializeController } from "../private/AppInitializeController"; import "metabase/css/vendor.css"; -import "../../styles.css"; +import "metabase/css/index.module.css"; + +const store = getStore(reducers) as unknown as Store; const MetabaseProviderInternal = ({ children, @@ -22,42 +23,17 @@ const MetabaseProviderInternal = ({ }: { children: React.ReactNode; config: SDKConfigType; -}): JSX.Element => { - const store = getStore(reducers); - - const [font, setFont] = useState(config.font ?? "Lato"); - - useEffect(() => { - if (font) { - store.dispatch(setOptions({ font })); - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [font]); - - const { isLoggedIn, isInitialized } = useInitData({ - store, - config, - }); - +}): React.JSX.Element => { return ( - - - - - - {!isInitialized ?
Initializing...
: children} - - - - - + + + + + {children} + + + + ); }; diff --git a/enterprise/frontend/src/embedding-sdk/components/public/QueryVisualization.tsx b/enterprise/frontend/src/embedding-sdk/components/public/QueryVisualization.tsx index d9d06a09e4908..5289b6dea824a 100644 --- a/enterprise/frontend/src/embedding-sdk/components/public/QueryVisualization.tsx +++ b/enterprise/frontend/src/embedding-sdk/components/public/QueryVisualization.tsx @@ -16,7 +16,7 @@ import { PublicMode } from "metabase/visualizations/click-actions/modes/PublicMo import Question from "metabase-lib/v1/Question"; import type { Card, CardId, Dataset } from "metabase-types/api"; -import { useEmbeddingContext } from "../../hooks"; +import { useEmbeddingContext } from "../../context"; interface QueryVisualizationProps { questionId: CardId; @@ -37,7 +37,7 @@ export const QueryVisualizationSdk = ( const { questionId } = props; - const [state, setState] = useState({ + const [{ loading, card, result }, setState] = useState({ loading: false, card: null, result: null, @@ -85,12 +85,10 @@ export const QueryVisualizationSdk = ( loadCardData({ questionId }); }, [isInitialized, isLoggedIn, questionId]); - const { card, result } = state; - const changeVisualization = (newQuestion: Question) => { setState({ card: newQuestion.card(), - result: state.result, + result: result, loading: false, }); }; @@ -108,10 +106,12 @@ export const QueryVisualizationSdk = ( ); } + const isLoading = loading || !result; + return ( @@ -140,7 +140,7 @@ export const QueryVisualizationSdk = ( className="flex full-width" question={question} rawSeries={[{ card, data: result && result.data }]} - isRunning={state.loading} + isRunning={isLoading} isObjectDetail={false} isResultDirty={false} isNativeEditorOpen={false} diff --git a/enterprise/frontend/src/embedding-sdk/context.ts b/enterprise/frontend/src/embedding-sdk/context.ts index c994702acc11c..bac2f6c52704c 100644 --- a/enterprise/frontend/src/embedding-sdk/context.ts +++ b/enterprise/frontend/src/embedding-sdk/context.ts @@ -3,15 +3,11 @@ import { createContext, useContext } from "react"; interface EmbeddingSdkContextData { isInitialized: boolean; isLoggedIn: boolean; - font: string | null; - setFont: (font: string) => void; } export const EmbeddingContext = createContext({ isInitialized: false, isLoggedIn: false, - font: null, - setFont: () => {}, }); export const useEmbeddingContext = () => { diff --git a/enterprise/frontend/src/embedding-sdk/hooks/private/index.ts b/enterprise/frontend/src/embedding-sdk/hooks/private/index.ts index 98b2c2700e921..a159167a9e0ba 100644 --- a/enterprise/frontend/src/embedding-sdk/hooks/private/index.ts +++ b/enterprise/frontend/src/embedding-sdk/hooks/private/index.ts @@ -1,2 +1 @@ -export * from "./use-sdk-context"; export * from "./use-init-data"; diff --git a/enterprise/frontend/src/embedding-sdk/hooks/private/use-init-data.ts b/enterprise/frontend/src/embedding-sdk/hooks/private/use-init-data.ts index b479f80bab7cc..8bd756e440061 100644 --- a/enterprise/frontend/src/embedding-sdk/hooks/private/use-init-data.ts +++ b/enterprise/frontend/src/embedding-sdk/hooks/private/use-init-data.ts @@ -3,25 +3,25 @@ import _ from "underscore"; import { reloadSettings } from "metabase/admin/settings/settings"; import api from "metabase/lib/api"; +import { useDispatch } from "metabase/lib/redux"; import { refreshCurrentUser } from "metabase/redux/user"; import registerVisualizations from "metabase/visualizations/register"; -import type { SDKConfigType } from "../../config"; +import type { SDKConfigType } from "../../types"; const registerVisualizationsOnce = _.once(registerVisualizations); -type InitDataLoaderProps = { - store: any; +interface InitDataLoaderParameters { config: SDKConfigType; -}; +} export const useInitData = ({ - store, config, -}: InitDataLoaderProps): { +}: InitDataLoaderParameters): { isLoggedIn: boolean; isInitialized: boolean; } => { + const dispatch = useDispatch(); const [isInitialized, setIsInitialized] = useState(false); const [isLoggedIn, setIsLoggedIn] = useState(false); @@ -40,13 +40,13 @@ export const useInitData = ({ } Promise.all([ - store.dispatch(refreshCurrentUser()), - store.dispatch(reloadSettings()), + dispatch(refreshCurrentUser()), + dispatch(reloadSettings()), ]).then(() => { setIsInitialized(true); setIsLoggedIn(true); }); - }, [config, store]); + }, [config, dispatch]); return { isLoggedIn, diff --git a/enterprise/frontend/src/embedding-sdk/hooks/private/use-sdk-context.ts b/enterprise/frontend/src/embedding-sdk/hooks/private/use-sdk-context.ts deleted file mode 100644 index 5896863a232aa..0000000000000 --- a/enterprise/frontend/src/embedding-sdk/hooks/private/use-sdk-context.ts +++ /dev/null @@ -1,7 +0,0 @@ -import { useContext } from "react"; - -import { EmbeddingContext } from "../../context"; - -export const useEmbeddingContext = () => { - return useContext(EmbeddingContext); -}; diff --git a/enterprise/frontend/src/embedding-sdk/hooks/public/use-available-fonts.ts b/enterprise/frontend/src/embedding-sdk/hooks/public/use-available-fonts.ts index a0e157c838eca..9253cc8c95fce 100644 --- a/enterprise/frontend/src/embedding-sdk/hooks/public/use-available-fonts.ts +++ b/enterprise/frontend/src/embedding-sdk/hooks/public/use-available-fonts.ts @@ -1,14 +1,10 @@ import { useSelector } from "metabase/lib/redux"; +import { getEmbedOptions } from "metabase/selectors/embed"; import { getSetting } from "metabase/selectors/settings"; -import { useEmbeddingContext } from "../../hooks/private/use-sdk-context"; - export const useAvailableFonts = () => { - const { font, setFont } = useEmbeddingContext(); - return { availableFonts: useSelector(state => getSetting(state, "available-fonts")), - currentFont: font, - setFont, + currentFont: useSelector(getEmbedOptions)?.font, }; }; diff --git a/enterprise/frontend/src/embedding-sdk/hooks/public/use-question-search.ts b/enterprise/frontend/src/embedding-sdk/hooks/public/use-question-search.ts index 7f72f345373b0..699f12c8c60b6 100644 --- a/enterprise/frontend/src/embedding-sdk/hooks/public/use-question-search.ts +++ b/enterprise/frontend/src/embedding-sdk/hooks/public/use-question-search.ts @@ -2,7 +2,7 @@ import { useMemo } from "react"; import { useSearchListQuery } from "metabase/common/hooks"; -import { useEmbeddingContext } from "../private/use-sdk-context"; +import { useEmbeddingContext } from "../../context"; export const useQuestionSearch = (searchQuery?: string) => { const { isInitialized, isLoggedIn } = useEmbeddingContext(); diff --git a/enterprise/frontend/src/embedding-sdk/styles.css b/enterprise/frontend/src/embedding-sdk/styles.css deleted file mode 100644 index d7e71512b9a92..0000000000000 --- a/enterprise/frontend/src/embedding-sdk/styles.css +++ /dev/null @@ -1 +0,0 @@ -@import "../../../../frontend/src/metabase/css/index.module.css"; diff --git a/enterprise/frontend/src/embedding-sdk/config.ts b/enterprise/frontend/src/embedding-sdk/types.ts similarity index 100% rename from enterprise/frontend/src/embedding-sdk/config.ts rename to enterprise/frontend/src/embedding-sdk/types.ts