From 313f13f9aa632e7794555ba7da3c31ab8dbdef1e Mon Sep 17 00:00:00 2001 From: Oisin Coveney <oisin@metabase.com> Date: Wed, 23 Oct 2024 08:21:20 +0300 Subject: [PATCH] Use `useSelector` and `useDispatch` in `ChartSettingsSidebar` (#48986) --- frontend/src/metabase-types/store/mocks/qb.ts | 1 + frontend/src/metabase-types/store/qb.ts | 1 + .../actions/visualization-settings.ts | 26 +++++---- .../query_builder/components/view/View.jsx | 21 +------ .../view/sidebars/ChartSettingsSidebar.tsx | 57 +++++++++---------- .../ChartSettingsSidebar.unit.spec.js | 25 ++++++-- 6 files changed, 65 insertions(+), 66 deletions(-) diff --git a/frontend/src/metabase-types/store/mocks/qb.ts b/frontend/src/metabase-types/store/mocks/qb.ts index 7e61a7e1cb7..237d73116ab 100644 --- a/frontend/src/metabase-types/store/mocks/qb.ts +++ b/frontend/src/metabase-types/store/mocks/qb.ts @@ -27,6 +27,7 @@ export const createMockQueryBuilderUIControlsState = ( datasetEditorTab: "query", isShowingNotebookNativePreview: false, notebookNativePreviewSidebarWidth: null, + showSidebarTitle: false, ...opts, }); diff --git a/frontend/src/metabase-types/store/qb.ts b/frontend/src/metabase-types/store/qb.ts index 905f8b846ab..7d1dcd13b8b 100644 --- a/frontend/src/metabase-types/store/qb.ts +++ b/frontend/src/metabase-types/store/qb.ts @@ -36,6 +36,7 @@ export interface QueryBuilderUIControls { datasetEditorTab: DatasetEditorTab; isShowingNotebookNativePreview: boolean; notebookNativePreviewSidebarWidth: number | null; + showSidebarTitle: boolean; } export interface QueryBuilderLoadingControls { diff --git a/frontend/src/metabase/query_builder/actions/visualization-settings.ts b/frontend/src/metabase/query_builder/actions/visualization-settings.ts index af5723003de..676f2276944 100644 --- a/frontend/src/metabase/query_builder/actions/visualization-settings.ts +++ b/frontend/src/metabase/query_builder/actions/visualization-settings.ts @@ -48,18 +48,20 @@ export const onUpdateVisualizationSettings = }; export const onReplaceAllVisualizationSettings = - (settings: VisualizationSettings, newQuestion: Question) => + (settings: VisualizationSettings, newQuestion?: Question) => async (dispatch: Dispatch, getState: GetState) => { - const oldQuestion = getQuestion(getState()); - const updatedQuestion = (newQuestion ?? oldQuestion).setSettings(settings); - const { isEditable } = Lib.queryDisplayInfo(updatedQuestion.query()); - const hasWritePermissions = isEditable; + const question = newQuestion ?? getQuestion(getState()); + if (question) { + const updatedQuestion = question.setSettings(settings); + const { isEditable } = Lib.queryDisplayInfo(updatedQuestion.query()); + const hasWritePermissions = isEditable; - await dispatch( - updateQuestion(updatedQuestion, { - // rerun the query when it is changed alongside settings - run: newQuestion != null && hasWritePermissions, - shouldUpdateUrl: hasWritePermissions, - }), - ); + await dispatch( + updateQuestion(updatedQuestion, { + // rerun the query when it is changed alongside settings + run: newQuestion != null && hasWritePermissions, + shouldUpdateUrl: hasWritePermissions, + }), + ); + } }; diff --git a/frontend/src/metabase/query_builder/components/view/View.jsx b/frontend/src/metabase/query_builder/components/view/View.jsx index 0eb99413bdf..b8e5f2a1413 100644 --- a/frontend/src/metabase/query_builder/components/view/View.jsx +++ b/frontend/src/metabase/query_builder/components/view/View.jsx @@ -158,13 +158,6 @@ const ViewLeftSidebarContainer = ({ result, isShowingChartSettingsSidebar, isShowingChartTypeSidebar, - onCloseChartSettings, - addField, - initialChartSetting, - onReplaceAllVisualizationSettings, - onOpenChartType, - visualizationSettings, - showSidebarTitle, }) => match({ isShowingChartSettingsSidebar, @@ -174,19 +167,7 @@ const ViewLeftSidebarContainer = ({ { isShowingChartSettingsSidebar: true, }, - () => ( - <ChartSettingsSidebar - question={question} - result={result} - addField={addField} - initialChartSetting={initialChartSetting} - onReplaceAllVisualizationSettings={onReplaceAllVisualizationSettings} - onOpenChartType={onOpenChartType} - visualizationSettings={visualizationSettings} - showSidebarTitle={showSidebarTitle} - onClose={onCloseChartSettings} - /> - ), + () => <ChartSettingsSidebar question={question} result={result} />, ) .with( { diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/ChartSettingsSidebar.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/ChartSettingsSidebar.tsx index 827b999166f..1ab9622d575 100644 --- a/frontend/src/metabase/query_builder/components/view/sidebars/ChartSettingsSidebar.tsx +++ b/frontend/src/metabase/query_builder/components/view/sidebars/ChartSettingsSidebar.tsx @@ -3,49 +3,47 @@ import { t } from "ttag"; import ErrorBoundary from "metabase/ErrorBoundary"; import CS from "metabase/css/core/index.css"; +import { useDispatch, useSelector } from "metabase/lib/redux"; +import { + onCloseChartSettings, + onOpenChartType, + onReplaceAllVisualizationSettings, +} from "metabase/query_builder/actions"; import SidebarContent from "metabase/query_builder/components/SidebarContent"; -import visualizations from "metabase/visualizations"; import { - ChartSettings, - type Widget, -} from "metabase/visualizations/components/ChartSettings"; + getUiControls, + getVisualizationSettings, +} from "metabase/query_builder/selectors"; +import visualizations from "metabase/visualizations"; +import { ChartSettings } from "metabase/visualizations/components/ChartSettings"; import type Question from "metabase-lib/v1/Question"; -import type { Dataset, VisualizationSettings } from "metabase-types/api"; +import type { Dataset } from "metabase-types/api"; interface ChartSettingsSidebarProps { question: Question; result: Dataset; - addField: () => void; - initialChartSetting: { section: string; widget?: Widget }; - onReplaceAllVisualizationSettings: (settings: VisualizationSettings) => void; - onClose: () => void; - onOpenChartType: () => void; - visualizationSettings: VisualizationSettings; - showSidebarTitle?: boolean; } -function ChartSettingsSidebarInner(props: ChartSettingsSidebarProps) { - const { - question, - result, - addField, - initialChartSetting, - onReplaceAllVisualizationSettings, - onClose, - onOpenChartType, - visualizationSettings, - showSidebarTitle = false, - } = props; +function ChartSettingsSidebarInner({ + question, + result, +}: ChartSettingsSidebarProps) { + const dispatch = useDispatch(); + + const visualizationSettings = useSelector(getVisualizationSettings); + const { initialChartSetting, showSidebarTitle = false } = + useSelector(getUiControls); + const sidebarContentProps = showSidebarTitle ? { title: t`${visualizations.get(question.display())?.uiName} options`, - onBack: () => onOpenChartType(), + onBack: () => dispatch(onOpenChartType()), } : {}; const handleClose = useCallback(() => { - onClose(); - }, [onClose]); + dispatch(onCloseChartSettings()); + }, [dispatch]); const card = question.card(); const series = useMemo(() => { @@ -67,9 +65,10 @@ function ChartSettingsSidebarInner(props: ChartSettingsSidebarProps) { <ErrorBoundary> <ChartSettings question={question} - addField={addField} series={series} - onChange={onReplaceAllVisualizationSettings} + onChange={(settings, question) => + dispatch(onReplaceAllVisualizationSettings(settings, question)) + } onClose={handleClose} noPreview initial={initialChartSetting} diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/ChartSettingsSidebar.unit.spec.js b/frontend/src/metabase/query_builder/components/view/sidebars/ChartSettingsSidebar.unit.spec.js index e37c184138b..f95392efcaa 100644 --- a/frontend/src/metabase/query_builder/components/view/sidebars/ChartSettingsSidebar.unit.spec.js +++ b/frontend/src/metabase/query_builder/components/view/sidebars/ChartSettingsSidebar.unit.spec.js @@ -1,12 +1,19 @@ -import { fireEvent, render, screen } from "@testing-library/react"; +import { fireEvent, screen } from "@testing-library/react"; import { createMockMetadata } from "__support__/metadata"; -import { ChartSettingsSidebar } from "metabase/query_builder/components/view/sidebars/ChartSettingsSidebar"; +import { renderWithProviders } from "__support__/ui"; import registerVisualizations from "metabase/visualizations/register"; import { SAMPLE_DB_ID, createSampleDatabase, } from "metabase-types/api/mocks/presets"; +import { + createMockQueryBuilderState, + createMockQueryBuilderUIControlsState, + createMockState, +} from "metabase-types/store/mocks"; + +import { ChartSettingsSidebar } from "./ChartSettingsSidebar"; registerVisualizations(); @@ -22,7 +29,7 @@ describe("ChartSettingsSidebar", () => { }; it("should hide the title when showSidebarTitle is false", () => { - render( + renderWithProviders( <ChartSettingsSidebar question={db.question().setDisplay("gauge")} result={{ data }} @@ -48,12 +55,20 @@ describe("ChartSettingsSidebar", () => { }); it("should not hide the title when showSidebarTitle is false", () => { - render( + renderWithProviders( <ChartSettingsSidebar question={db.question().setDisplay("scalar")} result={{ data }} - showSidebarTitle={true} />, + { + storeInitialState: createMockState({ + qb: createMockQueryBuilderState({ + uiControls: createMockQueryBuilderUIControlsState({ + showSidebarTitle: true, + }), + }), + }), + }, ); // see header with formatting fields -- GitLab