Viz Card Settings in popups (metabase#25125)

frontend/src/metabase/modes/components/drill/FormatAction.jsx

@@ -1,12 +1,12 @@
+import React from "react";
+import styled from "@emotion/styled";
+
 /* eslint-disable react/prop-types */
 import { t } from "ttag";
 
-// NOTE: cyclical dependency
-// import { showChartSettings } from "metabase/query_builder/actions";
-function showChartSettings(...args) {
-  return require("metabase/query_builder/actions").showChartSettings(...args);
-}
-
+import { getSettingsWidgetsForSeries } from "metabase/visualizations/lib/settings/visualization";
+import ChartSettingsWidget from "metabase/visualizations/components/ChartSettingsWidget";
+import { updateSettings } from "metabase/visualizations/lib/settings";
 import { keyForColumn } from "metabase/lib/dataset";
 
 export default ({ question, clicked }) => {
@@ -28,13 +28,42 @@ export default ({ question, clicked }) => {
       buttonType: "formatting",
       icon: "gear",
       tooltip: t`Column formatting`,
-      action: () =>
-        showChartSettings({
-          widget: {
-            id: "column_settings",
-            props: { initialKey: keyForColumn(column) },
-          },
-        }),
+      popover: function FormatPopover({ series, onChange }) {
+        const handleChangeSettings = changedSettings => {
+          onChange(
+            updateSettings(
+              series[0].card.visualization_settings,
+              changedSettings,
+            ),
+          );
+        };
+
+        const columnSettingsWidget = getSettingsWidgetsForSeries(
+          series,
+          handleChangeSettings,
+          false,
+        ).find(widget => widget.id === "column_settings");
+
+        return (
+          <PopoverRoot>
+            <ChartSettingsWidget
+              key={columnSettingsWidget.id}
+              {...{
+                ...columnSettingsWidget,
+                props: {
+                  ...columnSettingsWidget.props,
+                  initialKey: keyForColumn(column),
+                },
+              }}
+              hidden={false}
+            />
+          </PopoverRoot>
+        );
+      },
     },
   ];
 };
+
+const PopoverRoot = styled.div`
+  margin-top: 1.5rem;
+`;

frontend/src/metabase/visualizations/components/ChartClickActions.jsx

@@ -112,7 +112,13 @@ class ChartClickActions extends Component {
   };
 
   render() {
-    const { clicked, clickActions, onChangeCardAndRun } = this.props;
+    const {
+      clicked,
+      clickActions,
+      onChangeCardAndRun,
+      series,
+      onUpdateVisualizationSettings,
+    } = this.props;
 
     if (!clicked || !clickActions || clickActions.length === 0) {
       return null;
@@ -142,6 +148,8 @@ class ChartClickActions extends Component {
             );
             this.close();
           }}
+          series={series}
+          onChange={onUpdateVisualizationSettings}
         />
       );
     }

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

@@ -10,6 +10,7 @@ import Radio from "metabase/core/components/Radio";
 import { SectionContainer, SectionWarnings } from "./ChartSettings.styled";
 
 import Visualization from "metabase/visualizations/components/Visualization";
+import ChartSettingsWidgetPopover from "./ChartSettingsWidgetPopover";
 import ChartSettingsWidget from "./ChartSettingsWidget";
 
 import { getSettingsWidgetsForSeries } from "metabase/visualizations/lib/settings/visualization";
@@ -26,6 +27,28 @@ import {
 // section names are localized
 const DEFAULT_TAB_PRIORITY = [t`Display`];
 
+const getPopoverWidget = (widgets, currentWidget, extraWidgetProps) => {
+  const widget =
+    currentWidget && widgets.find(widget => widget.id === currentWidget.id);
+
+  if (widget) {
+    return (
+      <ChartSettingsWidget
+        key={widget.id}
+        {...widget}
+        props={{
+          ...widget.props,
+          ...currentWidget.props,
+        }}
+        hidden={false}
+        {...extraWidgetProps}
+      />
+    );
+  }
+
+  return undefined;
+};
+
 const withTransientSettingState = ComposedComponent =>
   class extends React.Component {
     static displayName = `withTransientSettingState[${
@@ -83,13 +106,14 @@ class ChartSettings extends Component {
   };
 
   // allows a widget to temporarily replace itself with a different widget
-  handleShowWidget = widget => {
+  handleShowWidget = (widget, ref) => {
+    this.setState({ popoverRef: ref });
     this.setState({ currentWidget: widget });
   };
 
   // go back to previously selected section
   handleEndShowWidget = () => {
-    this.setState({ currentWidget: null });
+    this.setState({ currentWidget: null, popoverRef: null });
   };
 
   handleResetSettings = () => {
@@ -162,7 +186,7 @@ class ChartSettings extends Component {
       setSidebarPropsOverride,
       dashboard,
     } = this.props;
-    const { currentWidget } = this.state;
+    const { currentWidget, popoverRef } = this.state;
 
     const settings = this._getSettings();
     const widgets = this._getWidgets();
@@ -210,21 +234,7 @@ class ChartSettings extends Component {
         : _.find(DEFAULT_TAB_PRIORITY, name => name in sections) ||
           sectionNames[0];
 
-    let visibleWidgets;
-    let widget = currentWidget && widgetsById[currentWidget.id];
-    if (widget) {
-      widget = {
-        ...widget,
-        hidden: false,
-        props: {
-          ...(widget.props || {}),
-          ...(currentWidget.props || {}),
-        },
-      };
-      visibleWidgets = [widget];
-    } else {
-      visibleWidgets = sections[currentSection] || [];
-    }
+    const visibleWidgets = sections[currentSection] || [];
 
     // This checks whether the current section contains a column settings widget
     // at the top level. If it does, we avoid hiding the section tabs and
@@ -258,7 +268,7 @@ class ChartSettings extends Component {
 
     const widgetList = visibleWidgets.map(widget => (
       <ChartSettingsWidget
-        key={`${widget.id}`}
+        key={widget.id}
         {...widget}
         {...extraWidgetProps}
         setSidebarPropsOverride={setSidebarPropsOverride}
@@ -341,6 +351,11 @@ class ChartSettings extends Component {
             </div>
           </div>
         )}
+        <ChartSettingsWidgetPopover
+          anchor={popoverRef}
+          widget={getPopoverWidget(widgets, currentWidget, extraWidgetProps)}
+          handleEndShowWidget={this.handleEndShowWidget}
+        />
       </div>
     );
   }

frontend/src/metabase/visualizations/components/ChartSettingsWidgetPopover.styled.tsx

@@ -0,0 +1,14 @@
+import styled from "@emotion/styled";
+
+export const PopoverRoot = styled.div`
+  padding: 1.5rem 0;
+  overflow-y: auto;
+  max-height: 600px;
+  min-width: 336px;
+`;
+
+export const PopoverTitle = styled.h3`
+  margin-left: 2rem;
+  margin-right: 2rem;
+  margin-bottom: 1.5rem;
+`;

frontend/src/metabase/visualizations/components/ChartSettingsWidgetPopover.tsx

@@ -0,0 +1,51 @@
+import React from "react";
+import { t } from "ttag";
+
+import TippyPopover from "metabase/components/Popover/TippyPopover";
+
+import { PopoverRoot, PopoverTitle } from "./ChartSettingsWidgetPopover.styled";
+
+interface Widget {
+  id: string;
+  props: Record<string, unknown>;
+}
+
+interface ChartSettingsWidgetPopoverProps {
+  anchor: HTMLElement;
+  handleEndShowWidget: () => void;
+  widget: Widget;
+}
+
+const ChartSettingsWidgetPopover = ({
+  anchor,
+  handleEndShowWidget,
+  widget,
+}: ChartSettingsWidgetPopoverProps) => {
+  return (
+    <TippyPopover
+      reference={anchor}
+      content={
+        <PopoverRoot>
+          <PopoverTitle>{t`Settings`}</PopoverTitle>
+          {widget}
+        </PopoverRoot>
+      }
+      visible={!!anchor}
+      onClose={handleEndShowWidget}
+      placement="right"
+      offset={[10, 10]}
+      popperOptions={{
+        modifiers: [
+          {
+            name: "preventOverflow",
+            options: {
+              padding: 16,
+            },
+          },
+        ],
+      }}
+    />
+  );
+};
+
+export default ChartSettingsWidgetPopover;

frontend/src/metabase/visualizations/components/Visualization.jsx

@@ -149,7 +149,7 @@ class Visualization extends React.PureComponent {
       : null;
     this.setState({
       hovered: null,
-      clicked: null,
+      //clicked: null,
       error: null,
       warnings: [],
       yAxisSplit: null,
@@ -325,6 +325,7 @@ class Visualization extends React.PureComponent {
       expectedDuration,
       replacementContent,
       onOpenChartSettings,
+      onUpdateVisualizationSettings,
     } = this.props;
     const { visualization } = this.state;
     const small = width < 330;
@@ -551,6 +552,8 @@ class Visualization extends React.PureComponent {
             clickActions={clickActions}
             onChangeCardAndRun={this.handleOnChangeCardAndRun}
             onClose={this.hideActions}
+            series={series}
+            onUpdateVisualizationSettings={onUpdateVisualizationSettings}
           />
         )}
       </div>

frontend/src/metabase/visualizations/components/settings/ChartNestedSettingColumns.jsx

@@ -11,7 +11,7 @@ export default class ChartNestedSettingColumns extends React.Component {
   render() {
     const { object, objects, onChangeEditingObject } = this.props;
     if (object) {
-      return <ColumnWidgets {...this.props} />;
+      return <div>{this.props.objectSettingsWidgets}</div>;
     } else {
       return (
         <div>
@@ -28,59 +28,3 @@ export default class ChartNestedSettingColumns extends React.Component {
     }
   }
 }
-
-// ColumnWidgets is a component just to hook into mount/unmount
-class ColumnWidgets extends React.Component {
-  componentDidMount() {
-    const {
-      setSidebarPropsOverride,
-      object,
-      onEndShowWidget,
-      currentSectionHasColumnSettings,
-    } = this.props;
-
-    // These two props (title and onBack) are overridden to display a column
-    // name instead of the visualization type when viewing a column's settings.
-    // If the column setting is directly within the section rather than an
-    // additional widget we drilled into, clicking back should still return us
-    // to the visualization list. In that case, we don't override these at all.
-    if (setSidebarPropsOverride && !currentSectionHasColumnSettings) {
-      setSidebarPropsOverride({
-        title: displayNameForColumn(object),
-        onBack: onEndShowWidget,
-      });
-    }
-  }
-
-  componentDidUpdate(prevProps) {
-    const {
-      setSidebarPropsOverride,
-      object,
-      onEndShowWidget,
-      currentSectionHasColumnSettings,
-    } = this.props;
-
-    if (
-      displayNameForColumn(object) !== displayNameForColumn(prevProps.object) ||
-      onEndShowWidget !== prevProps.onEndShowWidget
-    ) {
-      if (setSidebarPropsOverride && !currentSectionHasColumnSettings) {
-        setSidebarPropsOverride({
-          title: displayNameForColumn(object),
-          onBack: onEndShowWidget,
-        });
-      }
-    }
-  }
-
-  componentWillUnmount() {
-    const { setSidebarPropsOverride } = this.props;
-    if (setSidebarPropsOverride) {
-      setSidebarPropsOverride(null);
-    }
-  }
-
-  render() {
-    return <div>{this.props.objectSettingsWidgets}</div>;
-  }
-}

frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.jsx

@@ -41,13 +41,16 @@ const ChartSettingFieldPicker = ({
       {columnKey && (
         <SettingsIcon
           name="ellipsis"
-          onClick={() => {
-            onShowWidget({
-              id: "column_settings",
-              props: {
-                initialKey: columnKey,
+          onClick={e => {
+            onShowWidget(
+              {
+                id: "column_settings",
+                props: {
+                  initialKey: columnKey,
+                },
               },
-            });
+              e.target,
+            );
           }}
         />
       )}

frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.styled.tsx

@@ -32,7 +32,6 @@ export const SettingsIcon = styled(Icon)`
   margin-left: 0.5rem;
   color: ${color("text-medium")};
   cursor: pointer;
-  visibility: ${props => (props.onClick ? "visible" : "hidden")};
 
   &:hover {
     color: ${color("brand")};