diff --git a/frontend/src/metabase/query_builder/components/QueryVisualization.jsx b/frontend/src/metabase/query_builder/components/QueryVisualization.jsx index 0020002e53aa148da969aa6c1d55bbcf4d6a97b5..2a20f7169e98003ce0ce62b6d7f4bcec22a61526 100644 --- a/frontend/src/metabase/query_builder/components/QueryVisualization.jsx +++ b/frontend/src/metabase/query_builder/components/QueryVisualization.jsx @@ -250,7 +250,7 @@ export default class QueryVisualization extends Component { <VisualizationResult lastRunDatasetQuery={this.state.lastRunDatasetQuery} onUpdateWarnings={warnings => this.setState({ warnings })} - onOpenChartSettings={() => this.refs.settings.open()} + onOpenChartSettings={initial => this.refs.settings.open(initial)} {...this.props} className="spread" /> diff --git a/frontend/src/metabase/query_builder/components/VisualizationSettings.jsx b/frontend/src/metabase/query_builder/components/VisualizationSettings.jsx index 0b87abba291e1435eb6e55960ba974c5e48fde8f..412f2270643d357baf6dfd19d8604bef94dfa994 100644 --- a/frontend/src/metabase/query_builder/components/VisualizationSettings.jsx +++ b/frontend/src/metabase/query_builder/components/VisualizationSettings.jsx @@ -90,8 +90,8 @@ export default class VisualizationSettings extends React.Component { ); } - open = () => { - this.props.showChartSettings({}); + open = initial => { + this.props.showChartSettings(initial || {}); }; close = () => { @@ -123,7 +123,7 @@ export default class VisualizationSettings extends React.Component { ]} onChange={this.props.onReplaceAllVisualizationSettings} onClose={this.close} - initialWidget={chartSettings && chartSettings.widget} + initial={chartSettings} /> </Modal> </div> diff --git a/frontend/src/metabase/visualizations/components/ChartSettings.jsx b/frontend/src/metabase/visualizations/components/ChartSettings.jsx index 6c64ddaa2877641830342cf1849a1c0b28dce05a..821b4d7f321bd67182862a7315c4398cc1a3e6fd 100644 --- a/frontend/src/metabase/visualizations/components/ChartSettings.jsx +++ b/frontend/src/metabase/visualizations/components/ChartSettings.jsx @@ -19,14 +19,15 @@ import { } from "metabase/visualizations"; import { updateSettings } from "metabase/visualizations/lib/settings"; -const DEFAULT_TAB_PRIORITY = ["Display"]; +// section names are localized +const DEFAULT_TAB_PRIORITY = [t`Display`]; class ChartSettings extends Component { constructor(props) { super(props); this.state = { - currentTab: null, - showWidget: props.initialWidget, + currentSection: (props.initial && props.initial.section) || null, + currentWidget: (props.initial && props.initial.widget) || null, ...this._getState( props.series, props.series[0].card.visualization_settings, @@ -56,8 +57,18 @@ class ChartSettings extends Component { }; } - handleSelectTab = tab => { - this.setState({ currentTab: tab, showWidget: null }); + handleShowSection = section => { + this.setState({ currentSection: section, currentWidget: null }); + }; + + // allows a widget to temporarily replace itself with a different widget + handleShowWidget = widget => { + this.setState({ currentWidget: widget }); + }; + + // go back to previously selected section + handleEndShowWidget = () => { + this.setState({ currentWidget: null }); }; handleResetSettings = () => { @@ -79,21 +90,13 @@ class ChartSettings extends Component { this.props.onClose(); }; - // allows a widget to temporarily replace itself with a different widget - handleShowWidget = widget => { - this.setState({ showWidget: widget }); - }; - handleEndShowWidget = () => { - this.setState({ showWidget: null }); - }; - render() { const { isDashboard, question, addField } = this.props; - const { rawSeries, transformedSeries, showWidget } = this.state; + const { rawSeries, transformedSeries, currentWidget } = this.state; const widgetsById = {}; - const tabs = {}; + const sections = {}; for (const widget of getSettingsWidgetsForSeries( transformedSeries, this.handleChangeSettings, @@ -101,38 +104,38 @@ class ChartSettings extends Component { )) { widgetsById[widget.id] = widget; if (widget.widget && !widget.hidden) { - tabs[widget.section] = tabs[widget.section] || []; - tabs[widget.section].push(widget); + sections[widget.section] = sections[widget.section] || []; + sections[widget.section].push(widget); } } // Move settings from the "undefined" section in the first tab - if (tabs["undefined"] && Object.values(tabs).length > 1) { - let extra = tabs["undefined"]; - delete tabs["undefined"]; - Object.values(tabs)[0].unshift(...extra); + if (sections["undefined"] && Object.values(sections).length > 1) { + let extra = sections["undefined"]; + delete sections["undefined"]; + Object.values(sections)[0].unshift(...extra); } - const tabNames = Object.keys(tabs); - const currentTab = - this.state.currentTab || - _.find(DEFAULT_TAB_PRIORITY, name => name in tabs) || - tabNames[0]; + const sectionNames = Object.keys(sections); + const currentSection = + this.state.currentSection || + _.find(DEFAULT_TAB_PRIORITY, name => name in sections) || + sectionNames[0]; let widgets; - let widget = showWidget && widgetsById[showWidget.id]; + let widget = currentWidget && widgetsById[currentWidget.id]; if (widget) { widget = { ...widget, hidden: false, props: { ...(widget.props || {}), - ...(showWidget.props || {}), + ...(currentWidget.props || {}), }, }; widgets = [widget]; } else { - widgets = tabs[currentTab]; + widgets = sections[currentSection]; } const extraWidgetProps = { @@ -145,12 +148,12 @@ class ChartSettings extends Component { return ( <div className="flex flex-column spread"> - {tabNames.length > 1 && ( + {sectionNames.length > 1 && ( <div className="border-bottom flex flex-no-shrink pl4"> <Radio - value={currentTab} - onChange={this.handleSelectTab} - options={tabNames} + value={currentSection} + onChange={this.handleShowSection} + options={sectionNames} optionNameFn={v => v} optionValueFn={v => v} underlined diff --git a/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx b/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx index 4ae705928611c8917465b12e49f1901f935e3deb..dfa529e0707e87cf85e549a28709681beab67b81 100644 --- a/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx +++ b/frontend/src/metabase/visualizations/components/LineAreaBarChart.jsx @@ -99,7 +99,7 @@ export default class LineAreaBarChart extends Component { if (dimensions.length < 1 || metrics.length < 1) { throw new ChartSettingsError( t`Which fields do you want to use for the X and Y axes?`, - t`Data`, + { section: t`Data` }, t`Choose fields`, ); } diff --git a/frontend/src/metabase/visualizations/components/Visualization.jsx b/frontend/src/metabase/visualizations/components/Visualization.jsx index 9002d937b0dcfacbe0dae2022ae1fb7097dc8260..0a156a60f301ba240496ced51bf36d25cd5fe039 100644 --- a/frontend/src/metabase/visualizations/components/Visualization.jsx +++ b/frontend/src/metabase/visualizations/components/Visualization.jsx @@ -85,7 +85,7 @@ type Props = { // misc onUpdateWarnings: (string[]) => void, - onOpenChartSettings: () => void, + onOpenChartSettings: ({ section?: ?string, widget?: ?any }) => void, // number of grid cells wide and tall gridSize?: { width: number, height: number }, @@ -367,7 +367,7 @@ export default class Visualization extends Component { <div className="mt2"> <button className="Button Button--primary Button--medium" - onClick={this.props.onOpenChartSettings} + onClick={() => this.props.onOpenChartSettings(e.initial)} > {e.buttonText} </button> diff --git a/frontend/src/metabase/visualizations/lib/errors.js b/frontend/src/metabase/visualizations/lib/errors.js index d826d117c3382d21bb7bb778e1ff6a89a334d32c..2d69ae85f2c454928c8fb8634ad2ab2b8b8cee4e 100644 --- a/frontend/src/metabase/visualizations/lib/errors.js +++ b/frontend/src/metabase/visualizations/lib/errors.js @@ -3,6 +3,8 @@ import { t, ngettext, msgid } from "c-3po"; // NOTE: extending Error with Babel requires babel-plugin-transform-builtin-extend +type ChartSettingsInitial = { section?: ?string, widget?: ?any }; + export class MinColumnsError extends Error { constructor(minColumns: number, actualColumns: number) { super( @@ -42,11 +44,16 @@ export class NoBreakoutError extends Error { } export class ChartSettingsError extends Error { - section: ?string; + initial: ?ChartSettingsInitial; buttonText: ?string; - constructor(message: string, section?: string, buttonText?: string) { + + constructor( + message: string, + initial?: ChartSettingsInitial, + buttonText?: string, + ) { super(message || t`Please configure this chart in the chart settings`); - this.section = section; + this.initial = initial; this.buttonText = buttonText || t`Edit Settings`; } } diff --git a/frontend/src/metabase/visualizations/visualizations/Funnel.jsx b/frontend/src/metabase/visualizations/visualizations/Funnel.jsx index e7ae26b78d45584feb4ae657e9c3508931847eb0..fd3cf68357a999cd2057da2dcf46a9e55d129902 100644 --- a/frontend/src/metabase/visualizations/visualizations/Funnel.jsx +++ b/frontend/src/metabase/visualizations/visualizations/Funnel.jsx @@ -56,7 +56,7 @@ export default class Funnel extends Component { if (!settings["funnel.dimension"] || !settings["funnel.metric"]) { throw new ChartSettingsError( t`Which fields do you want to use?`, - t`Data`, + { section: t`Data` }, t`Choose fields`, ); } diff --git a/frontend/src/metabase/visualizations/visualizations/Map.jsx b/frontend/src/metabase/visualizations/visualizations/Map.jsx index 11860a58b8abca680a6dc62711b89545235a27c1..039468968fd073c2878a87796f298d0c9ec41b4f 100644 --- a/frontend/src/metabase/visualizations/visualizations/Map.jsx +++ b/frontend/src/metabase/visualizations/visualizations/Map.jsx @@ -223,17 +223,19 @@ export default class Map extends Component { ) { throw new ChartSettingsError( t`Please select longitude and latitude columns in the chart settings.`, - "Data", + { section: t`Data` }, ); } } else if (settings["map.type"] === "region") { if (!settings["map.region"]) { - throw new ChartSettingsError(t`Please select a region map.`, "Data"); + throw new ChartSettingsError(t`Please select a region map.`, { + section: t`Data`, + }); } if (!settings["map.dimension"] || !settings["map.metric"]) { throw new ChartSettingsError( t`Please select region and metric columns in the chart settings.`, - "Data", + { section: t`Data` }, ); } } diff --git a/frontend/src/metabase/visualizations/visualizations/PieChart.jsx b/frontend/src/metabase/visualizations/visualizations/PieChart.jsx index d9ad29d7978c0676c606fbc17ad897795c9ff608..b6d64b727eda02c807d3ca01d9e31081a272a30d 100644 --- a/frontend/src/metabase/visualizations/visualizations/PieChart.jsx +++ b/frontend/src/metabase/visualizations/visualizations/PieChart.jsx @@ -50,10 +50,9 @@ export default class PieChart extends Component { static checkRenderable([{ data: { cols, rows } }], settings) { if (!settings["pie.dimension"] || !settings["pie.metric"]) { - throw new ChartSettingsError( - t`Which columns do you want to use?`, - t`Data`, - ); + throw new ChartSettingsError(t`Which columns do you want to use?`, { + section: `Data`, + }); } } @@ -91,10 +90,13 @@ export default class PieChart extends Component { title: t`Colors`, widget: "colors", getDefault: (series, settings) => - getColorsForValues(settings["pie._dimensionValues"]), + settings["pie._dimensionValues"] + ? getColorsForValues(settings["pie._dimensionValues"]) + : [], getProps: (series, settings) => ({ - seriesTitles: settings["pie._dimensionValues"], + seriesTitles: settings["pie._dimensionValues"] || [], }), + getDisabled: (series, settings) => !settings["pie._dimensionValues"], readDependencies: ["pie._dimensionValues"], }, // this setting recomputes color assignment using pie.colors as the existing @@ -122,7 +124,10 @@ export default class PieChart extends Component { "pie._dimensionValues": { getValue: ([{ data: { rows } }], settings) => { const dimensionIndex = settings["pie._dimensionIndex"]; - return rows.map(row => row[dimensionIndex]); + return dimensionIndex >= 0 + ? // cast to string because getColorsForValues expects strings + rows.map(row => String(row[dimensionIndex])) + : null; }, readDependencies: ["pie._dimensionIndex"], },