rework ChartSettings to typescript (#48877)

* rework ChartSettings to typescript

* lint

frontend/src/metabase/query_builder/components/view/View.jsx

@@ -47,7 +47,7 @@ import {
 } from "./View.styled";
 import { ViewFooter } from "./ViewFooter";
 import ViewSidebar from "./ViewSidebar";
-import ChartSettingsSidebar from "./sidebars/ChartSettingsSidebar";
+import { ChartSettingsSidebar } from "./sidebars/ChartSettingsSidebar";
 import { ChartTypeSidebar } from "./sidebars/ChartTypeSidebar";
 import { QuestionInfoSidebar } from "./sidebars/QuestionInfoSidebar";
 import { QuestionSettingsSidebar } from "./sidebars/QuestionSettingsSidebar";

frontend/src/metabase/query_builder/components/view/sidebars/ChartSettingsSidebar.jsx → frontend/src/metabase/query_builder/components/view/sidebars/ChartSettingsSidebar.tsx

-/* eslint-disable react/prop-types */
-import { memo, useCallback, useMemo, useState } from "react";
+import { memo, useCallback, useMemo } from "react";
 import { t } from "ttag";
 
 import ErrorBoundary from "metabase/ErrorBoundary";
 import CS from "metabase/css/core/index.css";
 import SidebarContent from "metabase/query_builder/components/SidebarContent";
 import visualizations from "metabase/visualizations";
-import ChartSettings from "metabase/visualizations/components/ChartSettings";
+import {
+  ChartSettings,
+  type Widget,
+} from "metabase/visualizations/components/ChartSettings";
+import type Question from "metabase-lib/v1/Question";
+import type { Dataset, VisualizationSettings } from "metabase-types/api";
 
-function ChartSettingsSidebar(props) {
-  const [sidebarPropsOverride, setSidebarPropsOverride] = useState(null);
-  const handleSidebarPropsOverride = useCallback(
-    sidebarPropsOverride => {
-      setSidebarPropsOverride(sidebarPropsOverride);
-    },
-    [setSidebarPropsOverride],
-  );
+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,
@@ -30,7 +38,7 @@ function ChartSettingsSidebar(props) {
   } = props;
   const sidebarContentProps = showSidebarTitle
     ? {
-        title: t`${visualizations.get(question.display()).uiName} options`,
+        title: t`${visualizations.get(question.display())?.uiName} options`,
         onBack: () => onOpenChartType(),
       }
     : {};
@@ -55,7 +63,6 @@ function ChartSettingsSidebar(props) {
         className={CS.fullHeight}
         onDone={handleClose}
         {...sidebarContentProps}
-        {...sidebarPropsOverride}
       >
         <ErrorBoundary>
           <ChartSettings
@@ -66,7 +73,6 @@ function ChartSettingsSidebar(props) {
             onClose={handleClose}
             noPreview
             initial={initialChartSetting}
-            setSidebarPropsOverride={handleSidebarPropsOverride}
             computedSettings={visualizationSettings}
           />
         </ErrorBoundary>
@@ -75,4 +81,4 @@ function ChartSettingsSidebar(props) {
   );
 }
 
-export default memo(ChartSettingsSidebar);
+export const ChartSettingsSidebar = memo(ChartSettingsSidebarInner);

frontend/src/metabase/query_builder/components/view/sidebars/ChartSettingsSidebar.unit.spec.js

 import { fireEvent, render, screen } from "@testing-library/react";
 
 import { createMockMetadata } from "__support__/metadata";
-import ChartSettingsSidebar from "metabase/query_builder/components/view/sidebars/ChartSettingsSidebar";
+import { ChartSettingsSidebar } from "metabase/query_builder/components/view/sidebars/ChartSettingsSidebar";
 import registerVisualizations from "metabase/visualizations/register";
 import {
   SAMPLE_DB_ID,

frontend/src/metabase/visualizations/components/ChartSettings.jsx → frontend/src/metabase/visualizations/components/ChartSettings.tsx

-/* eslint-disable react/prop-types */
 import { assocIn } from "icepick";
 import { Component } from "react";
 import * as React from "react";
@@ -23,7 +22,18 @@ import {
 import { getSettingDefinitionsForColumn } from "metabase/visualizations/lib/settings/column";
 import { keyForSingleSeries } from "metabase/visualizations/lib/settings/series";
 import { getSettingsWidgetsForSeries } from "metabase/visualizations/lib/settings/visualization";
+import type Question from "metabase-lib/v1/Question";
 import { getColumnKey } from "metabase-lib/v1/queries/utils/column-key";
+import type {
+  Dashboard,
+  DashboardCard,
+  DatasetColumn,
+  RawSeries,
+  Series,
+  VisualizationSettings,
+} from "metabase-types/api";
+
+import type { ComputedVisualizationSettings } from "../types";
 
 import {
   ChartSettingsFooterRoot,
@@ -44,20 +54,25 @@ const DEFAULT_TAB_PRIORITY = [t`Data`];
 /**
  * @deprecated HOCs are deprecated
  */
-const withTransientSettingState = ComposedComponent =>
-  class extends React.Component {
+function withTransientSettingState(
+  ComposedComponent: React.ComponentType<ChartSettingsProps>,
+) {
+  return class extends React.Component<
+    ChartSettingsProps,
+    { settings?: VisualizationSettings }
+  > {
     static displayName = `withTransientSettingState[${
       ComposedComponent.displayName || ComposedComponent.name
     }]`;
 
-    constructor(props) {
+    constructor(props: ChartSettingsProps) {
       super(props);
       this.state = {
         settings: props.settings,
       };
     }
 
-    UNSAFE_componentWillReceiveProps(nextProps) {
+    UNSAFE_componentWillReceiveProps(nextProps: ChartSettingsProps) {
       if (this.props.settings !== nextProps.settings) {
         this.setState({ settings: nextProps.settings });
       }
@@ -68,17 +83,61 @@ const withTransientSettingState = ComposedComponent =>
         <ComposedComponent
           {...this.props}
           settings={this.state.settings}
-          onChange={settings => this.setState({ settings })}
-          onDone={settings =>
-            this.props.onChange(settings || this.state.settings)
+          onChange={(settings: VisualizationSettings) =>
+            this.setState({ settings })
+          }
+          onDone={(settings: VisualizationSettings) =>
+            this.props.onChange?.(settings || this.state.settings)
           }
         />
       );
     }
   };
+}
+
+// this type is not full, we need to extend it later
+export interface Widget {
+  id: string;
+  section: string;
+  hidden?: boolean;
+  props: Record<string, unknown>;
+  title?: string;
+  widget: (() => JSX.Element | null) | undefined;
+}
 
-class ChartSettings extends Component {
-  constructor(props) {
+export interface ChartSettingsProps {
+  className?: string;
+  dashboard?: Dashboard;
+  dashcard?: DashboardCard;
+  initial?: { section: string; widget?: Widget };
+  onCancel?: () => void;
+  onDone?: (settings: VisualizationSettings) => void;
+  onReset?: () => void;
+  onChange?: (
+    settings: ComputedVisualizationSettings,
+    question?: Question,
+  ) => void;
+  onClose?: () => void;
+  rawSeries?: RawSeries[];
+  settings?: VisualizationSettings;
+  widgets?: Widget[];
+  series: Series;
+  computedSettings?: ComputedVisualizationSettings;
+  isDashboard?: boolean;
+  question?: Question;
+  addField?: () => void;
+  noPreview?: boolean;
+}
+
+interface ChartSettingsState {
+  currentSection: string | null;
+  currentWidget: Widget | null;
+  popoverRef?: HTMLElement | null;
+  warnings?: string[];
+}
+
+class ChartSettings extends Component<ChartSettingsProps, ChartSettingsState> {
+  constructor(props: ChartSettingsProps) {
     super(props);
     this.state = {
       currentSection: (props.initial && props.initial.section) || null,
@@ -86,7 +145,7 @@ class ChartSettings extends Component {
     };
   }
 
-  componentDidUpdate(prevProps) {
+  componentDidUpdate(prevProps: ChartSettingsProps) {
     const { initial } = this.props;
     if (!_.isEqual(initial, prevProps.initial)) {
       this.setState({
@@ -96,7 +155,7 @@ class ChartSettings extends Component {
     }
   }
 
-  handleShowSection = section => {
+  handleShowSection = (section: string) => {
     this.setState({
       currentSection: section,
       currentWidget: null,
@@ -104,7 +163,7 @@ class ChartSettings extends Component {
   };
 
   // allows a widget to temporarily replace itself with a different widget
-  handleShowWidget = (widget, ref) => {
+  handleShowWidget = (widget: Widget, ref: HTMLElement | null) => {
     this.setState({ popoverRef: ref, currentWidget: widget });
   };
 
@@ -115,32 +174,38 @@ class ChartSettings extends Component {
 
   handleResetSettings = () => {
     const originalCardSettings =
-      this.props.dashcard.card.visualization_settings;
+      this.props.dashcard?.card.visualization_settings;
     const clickBehaviorSettings = getClickBehaviorSettings(this._getSettings());
 
-    this.props.onChange({ ...originalCardSettings, ...clickBehaviorSettings });
+    this.props.onChange?.({
+      ...originalCardSettings,
+      ...clickBehaviorSettings,
+    });
   };
 
-  handleChangeSettings = (changedSettings, question) => {
-    this.props.onChange(
+  handleChangeSettings = (
+    changedSettings: VisualizationSettings,
+    question: Question,
+  ) => {
+    this.props.onChange?.(
       updateSettings(this._getSettings(), changedSettings),
       question,
     );
   };
 
-  handleChangeSeriesColor = (seriesKey, color) => {
-    this.props.onChange(
+  handleChangeSeriesColor = (seriesKey: string, color: string) => {
+    this.props.onChange?.(
       updateSeriesColor(this._getSettings(), seriesKey, color),
     );
   };
 
   handleDone = () => {
-    this.props.onDone(this._getSettings());
-    this.props.onClose();
+    this.props.onDone?.(this._getSettings());
+    this.props.onClose?.();
   };
 
   handleCancel = () => {
-    this.props.onClose();
+    this.props.onClose?.();
   };
 
   _getSettings() {
@@ -153,7 +218,7 @@ class ChartSettings extends Component {
     return this.props.computedSettings || {};
   }
 
-  _getWidgets() {
+  _getWidgets(): Widget[] {
     if (this.props.widgets) {
       return this.props.widgets;
     } else {
@@ -188,7 +253,7 @@ class ChartSettings extends Component {
     return transformedSeries;
   }
 
-  columnHasSettings(col) {
+  columnHasSettings(col: DatasetColumn) {
     const { series } = this.props;
     const settings = this._getSettings() || {};
     const settingsDefs = getSettingDefinitionsForColumn(series, col);
@@ -206,7 +271,7 @@ class ChartSettings extends Component {
     ).some(widget => !widget.hidden);
   }
 
-  getStyleWidget = widgets => {
+  getStyleWidget = (widgets: Widget[]): Widget | null => {
     const series = this._getTransformedSeries();
     const settings = this._getComputedSettings();
     const { currentWidget } = this.state;
@@ -235,7 +300,8 @@ class ChartSettings extends Component {
         },
       };
     } else if (currentWidget.props?.initialKey) {
-      const hasBreakouts = settings["graph.dimensions"]?.length > 1;
+      const hasBreakouts =
+        settings["graph.dimensions"] && settings["graph.dimensions"].length > 1;
 
       if (hasBreakouts) {
         return null;
@@ -244,7 +310,9 @@ class ChartSettings extends Component {
       const singleSeriesForColumn = series.find(single => {
         const metricColumn = single.data.cols[1];
         if (metricColumn) {
-          return getColumnKey(metricColumn) === currentWidget.props.initialKey;
+          return (
+            getColumnKey(metricColumn) === currentWidget?.props?.initialKey
+          );
         }
       });
 
@@ -262,7 +330,7 @@ class ChartSettings extends Component {
     return null;
   };
 
-  getFormattingWidget = widgets => {
+  getFormattingWidget = (widgets: Widget[]): Widget | null => {
     const { currentWidget } = this.state;
     const widget =
       currentWidget && widgets.find(widget => widget.id === currentWidget.id);
@@ -279,10 +347,10 @@ class ChartSettings extends Component {
       className,
       question,
       addField,
-      noPreview,
+      noPreview = false,
       dashboard,
       dashcard,
-      isDashboard,
+      isDashboard = false,
     } = this.props;
     const { popoverRef } = this.state;
 
@@ -290,8 +358,8 @@ class ChartSettings extends Component {
     const widgets = this._getWidgets();
     const rawSeries = this._getRawSeries();
 
-    const widgetsById = {};
-    const sections = {};
+    const widgetsById: Record<string, Widget> = {};
+    const sections: Record<string, Widget[]> = {};
 
     for (const widget of widgets) {
       widgetsById[widget.id] = widget;
@@ -338,7 +406,7 @@ class ChartSettings extends Component {
     // overriding the sidebar title.
     const currentSectionHasColumnSettings = (
       sections[currentSection] || []
-    ).some(widget => widget.id === "column_settings");
+    ).some((widget: Widget) => widget.id === "column_settings");
 
     const extraWidgetProps = {
       // NOTE: special props to support adding additional fields
@@ -347,8 +415,8 @@ class ChartSettings extends Component {
       onShowWidget: this.handleShowWidget,
       onEndShowWidget: this.handleEndShowWidget,
       currentSectionHasColumnSettings,
-      columnHasSettings: col => this.columnHasSettings(col),
-      onChangeSeriesColor: (seriesKey, color) =>
+      columnHasSettings: (col: DatasetColumn) => this.columnHasSettings(col),
+      onChangeSeriesColor: (seriesKey: string, color: string) =>
         this.handleChangeSeriesColor(seriesKey, color),
     };
 
@@ -409,7 +477,9 @@ class ChartSettings extends Component {
                 isSettings
                 showWarnings
                 onUpdateVisualizationSettings={this.handleChangeSettings}
-                onUpdateWarnings={warnings => this.setState({ warnings })}
+                onUpdateWarnings={(warnings: string[]) =>
+                  this.setState({ warnings })
+                }
               />
             </ChartSettingsVisualizationContainer>
             <ChartSettingsFooter
@@ -420,11 +490,11 @@ class ChartSettings extends Component {
           </ChartSettingsPreview>
         )}
         <ChartSettingsWidgetPopover
-          anchor={popoverRef}
+          anchor={popoverRef as HTMLElement}
           widgets={[
             this.getStyleWidget(widgets),
             this.getFormattingWidget(widgets),
-          ].filter(widget => !!widget)}
+          ].filter((widget): widget is Widget => !!widget)}
           handleEndShowWidget={this.handleEndShowWidget}
         />
       </ChartSettingsRoot>
@@ -432,7 +502,15 @@ class ChartSettings extends Component {
   }
 }
 
-const ChartSettingsFooter = ({ onDone, onCancel, onReset }) => (
+const ChartSettingsFooter = ({
+  onDone,
+  onCancel,
+  onReset,
+}: {
+  onDone: () => void;
+  onCancel: () => void;
+  onReset: (() => void) | null;
+}) => (
   <ChartSettingsFooterRoot>
     {onReset && (
       <Button
@@ -446,6 +524,6 @@ const ChartSettingsFooter = ({ onDone, onCancel, onReset }) => (
   </ChartSettingsFooterRoot>
 );
 
-export default ChartSettings;
+export { ChartSettings };
 
 export const ChartSettingsWithState = withTransientSettingState(ChartSettings);

frontend/src/metabase/visualizations/components/ChartSettings.unit.spec.tsx

 import userEvent from "@testing-library/user-event";
 
 import { fireEvent, renderWithProviders, screen } from "__support__/ui";
-import ChartSettings from "metabase/visualizations/components/ChartSettings";
+import {
+  ChartSettings,
+  type ChartSettingsProps,
+  type Widget,
+} from "metabase/visualizations/components/ChartSettings";
 import registerVisualizations from "metabase/visualizations/register";
-import type { DashboardCard, Series } from "metabase-types/api";
 import {
   createMockCard,
   createMockDashboardCard,
+  createMockDataset,
   createMockVisualizationSettings,
 } from "metabase-types/api/mocks";
 
-import type { ComputedVisualizationSettings } from "../types";
-
 registerVisualizations();
 
 const DEFAULT_PROPS = {
   series: [
-    { card: { visualization_settings: {} }, data: { rows: [], cols: [] } },
+    {
+      card: createMockCard({ visualization_settings: {} }),
+      ...createMockDataset({ data: { rows: [], cols: [] } }),
+    },
   ],
   settings: {},
 };
 
-function widget(widget = {}) {
+function widget(widget: Partial<Widget> = {}): Widget {
   return {
     id: "id-" + Math.random(),
     title: "title-" + Math.random(),
     widget: () => null,
+    section: "section-" + Math.random(),
+    props: {},
     ...widget,
   };
 }
 
-// TODO Use ChartSettingsProps after ChartSettings is converted to TypeScript
-type SetupOpts = {
-  series?: Series;
-  settings?: ComputedVisualizationSettings;
-  dashcard?: DashboardCard;
-  widgets?: any[];
-  initial?: { section: string };
-  onChange?: (settings: ComputedVisualizationSettings) => void;
-};
+type SetupOpts = Partial<ChartSettingsProps>;
 
 const setup = (props: SetupOpts) => {
   return renderWithProviders(<ChartSettings {...DEFAULT_PROPS} {...props} />);

frontend/src/metabase/visualizations/components/settings/ChartNestedSettingsSeriesMultiple.unit.spec.js

@@ -2,7 +2,7 @@
 import userEvent from "@testing-library/user-event";
 
 import { renderWithProviders, screen, within } from "__support__/ui";
-import ChartSettings from "metabase/visualizations/components/ChartSettings";
+import { ChartSettings } from "metabase/visualizations/components/ChartSettings";
 import registerVisualizations from "metabase/visualizations/register";
 
 registerVisualizations();

frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.unit.spec.js

@@ -2,7 +2,7 @@
 import { within } from "@testing-library/react";
 
 import { renderWithProviders, screen } from "__support__/ui";
-import ChartSettings from "metabase/visualizations/components/ChartSettings";
+import { ChartSettings } from "metabase/visualizations/components/ChartSettings";
 import registerVisualizations from "metabase/visualizations/register";
 
 registerVisualizations();

frontend/src/metabase/visualizations/components/settings/ChartSettingStacked.unit.spec.js

 // these tests use ChartSettings directly, but logic we're testing lives in ChartNestedSettingSeries
 import { renderWithProviders, screen } from "__support__/ui";
-import ChartSettings from "metabase/visualizations/components/ChartSettings";
+import { ChartSettings } from "metabase/visualizations/components/ChartSettings";
 import registerVisualizations from "metabase/visualizations/register";
 
 registerVisualizations();

frontend/src/metabase/visualizations/visualizations/BarChart.unit.spec.tsx

 import { createMockMetadata } from "__support__/metadata";
 import { renderWithProviders, screen } from "__support__/ui";
-import ChartSettings from "metabase/visualizations/components/ChartSettings";
+import { ChartSettings } from "metabase/visualizations/components/ChartSettings";
 import registerVisualizations from "metabase/visualizations/register";
 import Question from "metabase-lib/v1/Question";
 import type { Series } from "metabase-types/api";

frontend/src/metabase/visualizations/visualizations/PivotTable/PivotTable.unit.spec.js

@@ -4,7 +4,7 @@ import { useState } from "react";
 
 import { createMockMetadata } from "__support__/metadata";
 import { render, screen } from "__support__/ui";
-import ChartSettings from "metabase/visualizations/components/ChartSettings";
+import { ChartSettings } from "metabase/visualizations/components/ChartSettings";
 import registerVisualizations from "metabase/visualizations/register";
 import Question from "metabase-lib/v1/Question";
 import { createMockColumn } from "metabase-types/api/mocks";

frontend/src/metabase/visualizations/visualizations/Table.unit.spec.js

@@ -4,7 +4,7 @@ import { useState } from "react";
 
 import { createMockMetadata } from "__support__/metadata";
 import { renderWithProviders, screen, within } from "__support__/ui";
-import ChartSettings from "metabase/visualizations/components/ChartSettings";
+import { ChartSettings } from "metabase/visualizations/components/ChartSettings";
 import registerVisualizations from "metabase/visualizations/register";
 import Question from "metabase-lib/v1/Question";
 import { createMockVisualizationSettings } from "metabase-types/api/mocks";