Skip to content

Commit

Permalink
Migrating shared NVD3 controls to new module (#9525)
Browse files Browse the repository at this point in the history
* proto module

* caught a missed 'freq' unique control

* line_interpolation

* linting

* showLegend

* show_controls

* xAxisLabel

* bottomMargin

* x_ticks_layout

* missed one

* x_axis_format

* yLogScale

* y_axis_bounds

* linting

* nixing yarn lock

* x_axis_showminmax

* xAxisShowminmax control

* richTooltip

* linting, syntax fix

* show_bar_value, bar_stacked

* reduceXticks, yaxislabel

* left_margin, max_bubble_size, y_axis_showminmax

* show_labels

* send_time_range, y_axis_2_format, show_markers, order_bars

* nixing commented imports

* fake controls

* looking up actual controls for comparison.

* adding key to test setup

* controls inventory

* apache junk

* lint ✨

* ignore null controls

* fixing goofed up spread operation for xAxisFormat config

* lint ✨

* fixes for errors caused by <hr> element in filterbox controls

* fixing filter controls for 'druid_time_origin', 'granularity', 'granularity_sqla', 'time_grain_sqla'

* getControlsInventory -> getControlsForVizType

* further renaming of chartControlsInventory - > getControlsForVizType

Co-authored-by: David Aaron Suddjian <aasuddjian@gmail.com>
  • Loading branch information
rusackas and suddjian authored Apr 17, 2020
1 parent 1b02b5b commit ea27e68
Show file tree
Hide file tree
Showing 27 changed files with 991 additions and 420 deletions.
30 changes: 0 additions & 30 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -1007,7 +1007,6 @@ Note not all fields are correctly catagorized. The fields vary based on visualiz
| Field | Type | Notes |
| ----------------- | -------- | --------------------------------------------------- |
| `metric_2` | - | The **Right Axis Metric** widget. See Query section |
| `y_axis_2_format` | _string_ | The **Right Axis Format** widget |

### Query

Expand All @@ -1019,7 +1018,6 @@ Note not all fields are correctly catagorized. The fields vary based on visualiz
| `contribution` | _boolean_ | The **Contribution** widget |
| `groupby` | _array(string)_ | The **Group by** or **Series** widget |
| `limit` | _number_ | The **Series Limit** widget |
| `max_bubble_size` | _number_ | The **Max Bubble Size** widget |
| `metric`<br>`metric_2`<br>`metrics`<br>`percent_mertics`<br>`secondary_metric`<br>`size`<br>`x`<br>`y` | _string_,_object_,_array(string)_,_array(object)_ | The metric(s) depending on the visualization type |
| `order_asc` | _boolean_ | The **Sort Descending** widget |
| `row_limit` | _number_ | The **Row limit** widget |
Expand All @@ -1042,38 +1040,17 @@ The filter-box configuration references column names (via the `column` key) and
| `color_picker` | _object_ | The **Fixed Color** widget |
| `global_opacity` | _number_ | The **Opacity** widget |
| `label_colors` | _object_ | The **Color Scheme** widget |
| `line_interpolation` | _string_ | The **Line Style** widget |
| `link_length` | _number_ | The **No of Bins** widget |
| `normalized` | _boolean_ | The **Normalized** widget |
| `number_format` | _string_ | The **Number format** widget |
| `rich_tooltip` | _boolean_ | The **Rich Tooltip** widget |
| `send_time_range` | _boolean_ | The **Show Markers** widget |
| `show_brush` | _string_ | The **Show Range Filter** widget |
| `show_legend` | _boolean_ | The **Legend** widget |
| `show_markers` | _string_ | The **Show Markers** widget |

### X Axis

| Field | Type | Notes |
| -------------------- | --------- | ---------------------------- |
| `bottom_margin` | _string_ | The **Bottom Margin** widget |
| `x_axis_format` | _string_ | The **X Axis Format** widget |
| `x_axis_label` | _string_ | The **X Axis Label** widget |
| `x_axis_showminmax` | _boolean_ | The **X bounds** widget |
| `x_ticks_layout` | _string_ | The **X Tick Layout** widget |

### Y Axis

| Field | Type | Notes |
| ------------------- | --------------- | ---------------------------- |
| `left_margin` | _number_ | The **Left Margin** widget |
| `y_axis_2_label` | _N/A_ | _Deprecated?_ |
| `y_axis_bounds` | _array(string)_ | The **Y Axis Bounds** widget |
| `y_axis_format` | _string_ | The **Y Axis Format** widget |
| `y_axis_label` | _string_ | The **Y Axis Label** widget |
| `y_axis_showminmax` | _boolean_ | The **Y bounds** widget |
| `y_axis_zero` | _N/A_ | _Deprecated?_ |
| `y_log_scale` | _boolean_ | The **Y Log Scale** widget |

Note the `y_axis_format` is defined under various section for some charts.

Expand All @@ -1092,7 +1069,6 @@ Note the `y_axis_format` is defined under various section for some charts.
| `add_to_dash` | _N/A_ | |
| `all_columns_y` | _N/A_ | |
| `annotation_layers` | _N/A_ | |
| `bar_stacked` | _N/A_ | |
| `cache_timeout` | _N/A_ | |
| `code` | _N/A_ | |
| `collapsed_fieldsets` | _N/A_ | |
Expand Down Expand Up @@ -1125,13 +1101,11 @@ Note the `y_axis_format` is defined under various section for some charts.
| `new_slice_name` | _N/A_ | |
| `normalize_across` | _N/A_ | |
| `num_period_compare` | _N/A_ | |
| `order_bars` | _N/A_ | |
| `order_desc` | _N/A_ | |
| `pandas_aggfunc` | _N/A_ | |
| `period_ratio_type` | _N/A_ | |
| `perm` | _N/A_ | |
| `rdo_save` | _N/A_ | |
| `reduce_x_ticks` | _N/A_ | |
| `refresh_frequency` | _N/A_ | |
| `remote_id` | _N/A_ | |
| `resample_fillmethod` | _N/A_ | |
Expand All @@ -1143,11 +1117,7 @@ Note the `y_axis_format` is defined under various section for some charts.
| `schema` | _N/A_ | |
| `select_country` | _N/A_ | |
| `series` | _N/A_ | |
| `show_bar_value` | _N/A_ | |
| `show_brush` | _N/A_ | |
| `show_bubbles` | _N/A_ | |
| `show_controls` | _N/A_ | |
| `show_labels` | _N/A_ | |
| `show_values` | _N/A_ | |
| `slice_name` | _N/A_ | |
| `table_filter` | _N/A_ | |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,16 @@
*/
import React from 'react';
import { shallow } from 'enzyme';

import { Table, Thead, Td, Th, Tr } from 'reactable-arc';
import { getChartControlPanelRegistry } from '@superset-ui/chart';

import AlteredSliceTag from '../../../src/components/AlteredSliceTag';
import ModalTrigger from '../../../src/components/ModalTrigger';
import TooltipWrapper from '../../../src/components/TooltipWrapper';

const defaultProps = {
origFormData: {
viz_type: 'altered_slice_tag_spec',
adhoc_filters: [
{
clause: 'WHERE',
Expand Down Expand Up @@ -111,11 +112,52 @@ const expectedDiffs = {
},
};

const fakePluginControls = {
controlPanelSections: [
{
label: 'Fake Control Panel Sections',
expanded: true,
controlSetRows: [
[
{
name: 'y_axis_bounds',
config: {
type: 'BoundsControl',
label: 'Value bounds',
default: [null, null],
description: 'Value bounds for the y axis',
},
},
{
name: 'column_collection',
config: {
type: 'CollectionControl',
label: 'Fake Collection Control',
},
},
{
name: 'adhoc_filters',
config: {
type: 'AdhocFilterControl',
label: 'Fake Filters',
default: null,
},
},
],
],
},
],
};

describe('AlteredSliceTag', () => {
let wrapper;
let props;

beforeEach(() => {
getChartControlPanelRegistry().registerValue(
'altered_slice_tag_spec',
fakePluginControls,
);
props = { ...defaultProps };
wrapper = shallow(<AlteredSliceTag {...props} />);
});
Expand Down Expand Up @@ -237,6 +279,7 @@ describe('AlteredSliceTag', () => {
});

it('returns "Max" and "Min" for BoundsControl', () => {
// need to pass the viz type to the wrapper
expect(wrapper.instance().formatValue([5, 6], 'y_axis_bounds')).toBe(
'Min: 5, Max: 6',
);
Expand Down
104 changes: 104 additions & 0 deletions superset-frontend/spec/javascripts/utils/getControlsForVizType_spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { getChartControlPanelRegistry } from '@superset-ui/chart';
import getControlsForVizType from 'src/utils/getControlsForVizType';

const fakePluginControls = {
controlPanelSections: [
{
label: 'Fake Control Panel Sections',
expanded: true,
controlSetRows: [
['url_params'],
[
{
name: 'y_axis_bounds',
config: {
type: 'BoundsControl',
label: 'Value bounds',
default: [null, null],
description: 'Value bounds for the y axis',
},
},
],
[
{
name: 'adhoc_filters',
config: {
type: 'AdhocFilterControl',
label: 'Fake Filters',
default: null,
},
},
],
],
},
{
label: 'Fake Control Panel Sections 2',
expanded: true,
controlSetRows: [
[
{
name: 'column_collection',
config: {
type: 'CollectionControl',
label: 'Fake Collection Control',
},
},
],
],
},
],
};

describe('getControlsForVizType', () => {
beforeEach(() => {
getChartControlPanelRegistry().registerValue(
'chart_controls_inventory_fake',
fakePluginControls,
);
});

it('returns a map of the controls', () => {
expect(getControlsForVizType('chart_controls_inventory_fake')).toEqual({
url_params: {
type: 'HiddenControl',
label: 'URL Parameters',
hidden: true,
description: 'Extra parameters for use in jinja templated queries',
},
y_axis_bounds: {
type: 'BoundsControl',
label: 'Value bounds',
default: [null, null],
description: 'Value bounds for the y axis',
},
adhoc_filters: {
type: 'AdhocFilterControl',
label: 'Fake Filters',
default: null,
},
column_collection: {
type: 'CollectionControl',
label: 'Fake Collection Control',
},
});
});
});
30 changes: 24 additions & 6 deletions superset-frontend/src/components/AlteredSliceTag.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ import React from 'react';
import PropTypes from 'prop-types';
import { Table, Tr, Td, Thead, Th } from 'reactable-arc';
import { isEqual, isEmpty } from 'lodash';
import { getChartControlPanelRegistry } from '@superset-ui/chart';
import getControlsForVizType from 'src/utils/getControlsForVizType';
import { t } from '@superset-ui/translation';
import TooltipWrapper from './TooltipWrapper';
import { controls } from '../explore/controls';
import ModalTrigger from './ModalTrigger';
import { safeStringify } from '../utils/safeStringify';

Expand Down Expand Up @@ -52,7 +53,10 @@ export default class AlteredSliceTag extends React.Component {
constructor(props) {
super(props);
const diffs = this.getDiffs(props);
this.state = { diffs, hasDiffs: !isEmpty(diffs) };

const controlsMap = getControlsForVizType(this.props.origFormData.viz_type);

this.state = { diffs, hasDiffs: !isEmpty(diffs), controlsMap };
}

UNSAFE_componentWillReceiveProps(newProps) {
Expand All @@ -69,6 +73,7 @@ export default class AlteredSliceTag extends React.Component {
// current form data and the saved form data
const ofd = props.origFormData;
const cfd = props.currentFormData;

const fdKeys = Object.keys(cfd);
const diffs = {};
for (const fdKey of fdKeys) {
Expand Down Expand Up @@ -98,7 +103,10 @@ export default class AlteredSliceTag extends React.Component {
return 'N/A';
} else if (value === null) {
return 'null';
} else if (controls[key] && controls[key].type === 'AdhocFilterControl') {
} else if (
this.state.controlsMap[key] &&
this.state.controlsMap[key].type === 'AdhocFilterControl'
) {
if (!value.length) {
return '[]';
}
Expand All @@ -111,9 +119,15 @@ export default class AlteredSliceTag extends React.Component {
return `${v.subject} ${v.operator} ${filterVal}`;
})
.join(', ');
} else if (controls[key] && controls[key].type === 'BoundsControl') {
} else if (
this.state.controlsMap[key] &&
this.state.controlsMap[key].type === 'BoundsControl'
) {
return `Min: ${value[0]}, Max: ${value[1]}`;
} else if (controls[key] && controls[key].type === 'CollectionControl') {
} else if (
this.state.controlsMap[key] &&
this.state.controlsMap[key].type === 'CollectionControl'
) {
return value.map(v => safeStringify(v)).join(', ');
} else if (typeof value === 'boolean') {
return value ? 'true' : 'false';
Expand All @@ -133,7 +147,11 @@ export default class AlteredSliceTag extends React.Component {
<Tr key={key}>
<Td
column="control"
data={(controls[key] && controls[key].label) || key}
data={
(this.state.controlsMap[key] &&
this.state.controlsMap[key].label) ||
key
}
/>
<Td column="before">{this.formatValue(diffs[key].before, key)}</Td>
<Td column="after">{this.formatValue(diffs[key].after, key)}</Td>
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/explore/components/Control.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export default class Control extends React.PureComponent {
this.setState({ hovered });
}
render() {
if (!this.props.type) return null; // this catches things like <hr/> elements (not a control!) shoved into the control panel configs.
const ControlType = controlMap[this.props.type];
const divStyle = this.props.hidden ? { display: 'none' } : null;
return (
Expand Down
Loading

0 comments on commit ea27e68

Please sign in to comment.