Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improving Filter Box #6523

Merged
merged 2 commits into from
Jan 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions superset/assets/spec/javascripts/components/FormRow_spec.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import React from 'react';
import { shallow } from 'enzyme';

import { Col, Row } from 'react-bootstrap';
import TextControl from '../../../src/explore/components/controls/TextControl';
import InfoTooltipWithTrigger from '../../../src/components/InfoTooltipWithTrigger';
import FormRow from '../../../src/components/FormRow';

const defaultProps = {
label: 'Hello',
tooltip: 'A tooltip',
control: <TextControl label="test_cbox" />,
};

describe('FormRow', () => {
let wrapper;

const getWrapper = (overrideProps = {}) => {
const props = {
...defaultProps,
...overrideProps,
};
return shallow(<FormRow {...props} />);
};

beforeEach(() => {
wrapper = getWrapper();
});

it('renders an InfoTooltipWithTrigger only if needed', () => {
expect(wrapper.find(InfoTooltipWithTrigger)).toHaveLength(1);
wrapper = getWrapper({ tooltip: null });
expect(wrapper.find(InfoTooltipWithTrigger)).toHaveLength(0);
});

it('renders a Row and 2 Cols', () => {
expect(wrapper.find(Row)).toHaveLength(1);
expect(wrapper.find(Col)).toHaveLength(2);
});

});
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/* eslint-disable no-unused-expressions */
import React from 'react';
import sinon from 'sinon';
import { shallow } from 'enzyme';
import { OverlayTrigger } from 'react-bootstrap';

import FilterBoxItemControl from '../../../../src/explore/components/controls/FilterBoxItemControl';
import FormRow from '../../../../src/components/FormRow';
import datasources from '../../../fixtures/mockDatasource';

const defaultProps = {
datasource: datasources['7__table'],
onChange: sinon.spy(),
};

describe('FilterBoxItemControl', () => {
let wrapper;
let inst;

const getWrapper = (propOverrides) => {
const props = { ...defaultProps, ...propOverrides };
return shallow(<FilterBoxItemControl {...props} />);
};
beforeEach(() => {
wrapper = getWrapper();
inst = wrapper.instance();
});

it('renders an OverlayTrigger', () => {
expect(wrapper.find(OverlayTrigger)).toHaveLength(1);
});

it('renderForms does the job', () => {
const popover = shallow(inst.renderForm());
expect(popover.find(FormRow)).toHaveLength(7);
});
});
2 changes: 1 addition & 1 deletion superset/assets/src/chart/ChartRenderer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const defaultProps = {
triggerRender: false,
};

class ChartRenderer extends React.PureComponent {
class ChartRenderer extends React.Component {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated change, caught some React related warning

constructor(props) {
super(props);
this.state = {};
Expand Down
47 changes: 47 additions & 0 deletions superset/assets/src/components/FormRow.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import React from 'react';
import PropTypes from 'prop-types';
import { Row, Col } from 'react-bootstrap';

import InfoTooltipWithTrigger from './InfoTooltipWithTrigger';

const STYLE_ROW = { marginTop: '5px', minHeight: '30px' };
const STYLE_RALIGN = { textAlign: 'right' };

const propTypes = {
label: PropTypes.string.isRequired,
tooltip: PropTypes.string,
control: PropTypes.node.isRequired,
isCheckbox: PropTypes.bool,
};

const defaultProps = {
tooltip: null,
isCheckbox: false,
};

export default function FormRow({ label, tooltip, control, isCheckbox }) {
const labelAndTooltip = (
<span>
{label}{' '}
{tooltip &&
<InfoTooltipWithTrigger
placement="top"
label={label}
tooltip={tooltip}
/>}
</span>);
if (isCheckbox) {
return (
<Row style={STYLE_ROW}>
<Col md={4} style={STYLE_RALIGN}>{control}</Col>
<Col md={8}>{labelAndTooltip}</Col>
</Row>);
}
return (
<Row style={STYLE_ROW}>
<Col md={4} style={STYLE_RALIGN}>{labelAndTooltip}</Col>
<Col md={8}>{control}</Col>
</Row>);
}
FormRow.propTypes = propTypes;
FormRow.defaultProps = defaultProps;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.CollectionControl .list-group-item i.fa {
padding-top: 5px;
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
import InfoTooltipWithTrigger from '../../../components/InfoTooltipWithTrigger';
import ControlHeader from '../ControlHeader';
import controlMap from './';
import './CollectionControl.css';

const propTypes = {
name: PropTypes.string.isRequired,
Expand Down Expand Up @@ -82,6 +83,7 @@ export default class CollectionControl extends React.Component {
</div>
<div className="pull-left">
<Control
{...this.props}
{...o}
onChange={this.onChange.bind(this, i)}
/>
Expand All @@ -101,7 +103,7 @@ export default class CollectionControl extends React.Component {
}
render() {
return (
<div>
<div className="CollectionControl">
<ControlHeader {...this.props} />
{this.renderList()}
<InfoTooltipWithTrigger
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
import React from 'react';
import PropTypes from 'prop-types';
import { OverlayTrigger, Popover } from 'react-bootstrap';
import { t } from '@superset-ui/translation';

import InfoTooltipWithTrigger from '../../../components/InfoTooltipWithTrigger';
import FormRow from '../../../components/FormRow';
import SelectControl from './SelectControl';
import CheckboxControl from './CheckboxControl';
import TextControl from './TextControl';

const propTypes = {
datasource: PropTypes.object.isRequired,
onChange: PropTypes.func,
asc: PropTypes.bool,
clearable: PropTypes.bool,
multiple: PropTypes.bool,
column: PropTypes.string,
metric: PropTypes.string,
defaultValue: PropTypes.string,
};

const defaultProps = {
onChange: () => {},
asc: true,
clearable: true,
multiple: true,
};

const STYLE_WIDTH = { width: 350 };

export default class FilterBoxItemControl extends React.Component {
constructor(props) {
super(props);
const { column, metric, asc, clearable, multiple, defaultValue } = props;
const state = { column, metric, asc, clearable, multiple, defaultValue };
this.state = state;
this.onChange = this.onChange.bind(this);
this.onControlChange = this.onControlChange.bind(this);
}
onChange() {
this.props.onChange(this.state);
}
onControlChange(attr, value) {
this.setState({ [attr]: value }, this.onChange);
}
setType() {
}
textSummary() {
return this.state.column || 'N/A';
}
renderForm() {
return (
<div>
<FormRow
label={t('Column')}
control={
<SelectControl
value={this.state.column}
name="column"
clearable={false}
options={this.props.datasource.columns.map(col => ({
value: col.column_name,
label: col.column_name,
}))}
onChange={v => this.onControlChange('column', v)}
/>
}
/>
<FormRow
label={t('Label')}
control={
<TextControl
value={this.state.label}
name="label"
onChange={v => this.onControlChange('label', v)}
/>
}
/>
<FormRow
label={t('Default')}
tooltip={t(
'(optional) default value for the filter, when using ' +
'the multiple option, you can use a semicolon-delimited list ' +
'of options.')}
control={
<TextControl
value={this.state.defaultValue}
name="defaultValue"
onChange={v => this.onControlChange('defaultValue', v)}
/>
}
/>
<FormRow
label={t('Sort Metric')}
tooltip={t('Metric to sort the results by')}
control={
<SelectControl
value={this.state.metric}
name="column"
options={this.props.datasource.metrics.map(m => ({
value: m.metric_name,
label: m.metric_name,
}))}
onChange={v => this.onControlChange('metric', v)}
/>
}
/>
<FormRow
label={t('Sort Ascending')}
tooltip={t('Check for sorting ascending')}
isCheckbox
control={
<CheckboxControl
value={this.state.asc}
onChange={v => this.onControlChange('asc', v)}
/>
}
/>
<FormRow
label={t('Allow Multiple Selections')}
isCheckbox
tooltip={t(
'Multiple selections allowed, otherwise filter ' +
'is limited to a single value')}
control={
<CheckboxControl
value={this.state.multiple}
onChange={v => this.onControlChange('multiple', v)}
/>
}
/>
<FormRow
label={t('Required')}
tooltip={t('User must select a value for this filter')}
isCheckbox
control={
<CheckboxControl
value={!this.state.clearable}
onChange={v => this.onControlChange('clearable', !v)}
/>
}
/>
</div>);
}
renderPopover() {
return (
<Popover id="ts-col-popo" title={t('Filter Configuration')}>
<div style={STYLE_WIDTH}>
{this.renderForm()}
</div>
</Popover>
);
}
render() {
return (
<span>
{this.textSummary()}{' '}
<OverlayTrigger
container={document.body}
trigger="click"
rootClose
ref="trigger"
placement="right"
overlay={this.renderPopover()}
>
<InfoTooltipWithTrigger
icon="edit"
className="text-primary"
label="edit-ts-column"
/>
</OverlayTrigger>
</span>
);
}
}

FilterBoxItemControl.propTypes = propTypes;
FilterBoxItemControl.defaultProps = defaultProps;
2 changes: 2 additions & 0 deletions superset/assets/src/explore/components/controls/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import VizTypeControl from './VizTypeControl';
import MetricsControl from './MetricsControl';
import AdhocFilterControl from './AdhocFilterControl';
import FilterPanel from './FilterPanel';
import FilterBoxItemControl from './FilterBoxItemControl';

const controlMap = {
AnnotationLayerControl,
Expand All @@ -44,5 +45,6 @@ const controlMap = {
MetricsControl,
AdhocFilterControl,
FilterPanel,
FilterBoxItemControl,
};
export default controlMap;
Original file line number Diff line number Diff line change
@@ -1,29 +1,27 @@
import React from 'react';
import { t } from '@superset-ui/translation';

export default {
controlPanelSections: [
{
label: t('Query'),
label: t('Filters Configuration'),
expanded: true,
controlSetRows: [
['groupby'],
['metric'],
['adhoc_filters'],
['filter_configs'],
[<hr />],
['date_filter', 'instant_filtering'],
['show_sqla_time_granularity', 'show_sqla_time_column'],
['show_druid_time_granularity', 'show_druid_time_origin'],
['adhoc_filters'],
],
},
],
controlOverrides: {
groupby: {
label: t('Filter controls'),
adhoc_filters: {
label: t('Global Filters'),
description: t(
'The controls you want to filter on. Note that only columns ' +
'checked as "filterable" will show up on this list.'),
mapStateToProps: state => ({
options: (state.datasource) ? state.datasource.columns.filter(c => c.filterable) : [],
}),
'These filters, like the time filters, will be applied ' +
'to each individual filters as the values are populated.'),
},
},
};
Loading