Skip to content

Commit

Permalink
Adding column type label to dropdowns (#4566)
Browse files Browse the repository at this point in the history
* Adding column type label to dropdowns

* Changing the style of column type label

* Adding tests for ColumnTypeLabel

* Adding tests for time and fixing if statement order

* Changing location of ColumnTypeLabel tests

* Updating ColumnTypeLabel structure
  • Loading branch information
michellethomas authored and williaster committed Mar 16, 2018
1 parent 6875868 commit 3f1dfb3
Show file tree
Hide file tree
Showing 9 changed files with 155 additions and 17 deletions.
16 changes: 12 additions & 4 deletions superset/assets/javascripts/components/ColumnOption.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import PropTypes from 'prop-types';

import ColumnTypeLabel from './ColumnTypeLabel';
import InfoTooltipWithTrigger from './InfoTooltipWithTrigger';

const propTypes = {
Expand All @@ -12,8 +13,18 @@ const defaultProps = {
};

export default function ColumnOption({ column, showType }) {
const hasExpression = column.expression && column.expression !== column.column_name;

let columnType = column.type;
if (column.is_dttm) {
columnType = 'time';
} else if (hasExpression) {
columnType = 'expression';
}

return (
<span>
{showType && <ColumnTypeLabel type={columnType} />}
<span className="m-r-5 option-label">
{column.verbose_name || column.column_name}
</span>
Expand All @@ -25,17 +36,14 @@ export default function ColumnOption({ column, showType }) {
label={`descr-${column.column_name}`}
/>
}
{column.expression && column.expression !== column.column_name &&
{hasExpression &&
<InfoTooltipWithTrigger
className="m-r-5 text-muted"
icon="question-circle-o"
tooltip={column.expression}
label={`expr-${column.column_name}`}
/>
}
{showType &&
<span className="text-muted">{column.type}</span>
}
</span>);
}
ColumnOption.propTypes = propTypes;
Expand Down
33 changes: 33 additions & 0 deletions superset/assets/javascripts/components/ColumnTypeLabel.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import React from 'react';
import PropTypes from 'prop-types';

const propTypes = {
type: PropTypes.string.isRequired,
};

export default function ColumnTypeLabel({ type }) {
let stringIcon = '';
if (type === '' || type === 'expression') {
stringIcon = 'ƒ';
} else if (type.match(/.*char.*/i) || type.match(/string.*/i) || type.match(/.*text.*/i)) {
stringIcon = 'ABC';
} else if (type.match(/.*int.*/i) || type === 'LONG' || type === 'DOUBLE') {
stringIcon = '#';
} else if (type.match(/.*bool.*/i)) {
stringIcon = 'T/F';
} else if (type.match(/.*time.*/i)) {
stringIcon = 'time';
} else if (type.match(/unknown/i)) {
stringIcon = '?';
}

const typeIcon = stringIcon === 'time' ?
<i className="fa fa-clock-o type-label" /> :
<div className="type-label">{stringIcon}</div>;

return (
<span>
{typeIcon}
</span>);
}
ColumnTypeLabel.propTypes = propTypes;
6 changes: 5 additions & 1 deletion superset/assets/javascripts/components/MetricOption.jsx
Original file line number Diff line number Diff line change
@@ -1,23 +1,27 @@
import React from 'react';
import PropTypes from 'prop-types';

import ColumnTypeLabel from './ColumnTypeLabel';
import InfoTooltipWithTrigger from './InfoTooltipWithTrigger';

const propTypes = {
metric: PropTypes.object.isRequired,
openInNewWindow: PropTypes.bool,
showFormula: PropTypes.bool,
showType: PropTypes.bool,
url: PropTypes.string,
};
const defaultProps = {
showFormula: true,
showType: false,
};

export default function MetricOption({ metric, openInNewWindow, showFormula, url }) {
export default function MetricOption({ metric, openInNewWindow, showFormula, showType, url }) {
const verbose = metric.verbose_name || metric.metric_name;
const link = url ? <a href={url} target={openInNewWindow ? '_blank' : null}>{verbose}</a> : verbose;
return (
<div>
{showType && <ColumnTypeLabel type="expression" />}
<span className="m-r-5 option-label">{link}</span>
{metric.description &&
<InfoTooltipWithTrigger
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ export default class DatasourceControl extends React.PureComponent {
</Label>
{` ${datasource.database.name} `}
</div>
<Row>
<Row className="datasource-container">
<Col md={6}>
<strong>Columns</strong>
{datasource.columns.map(col => (
Expand All @@ -163,7 +163,7 @@ export default class DatasourceControl extends React.PureComponent {
<Col md={6}>
<strong>Metrics</strong>
{datasource.metrics.map(m => (
<div key={m.metric_name}><MetricOption metric={m} /></div>
<div key={m.metric_name}><MetricOption metric={m} showType /></div>
))}
</Col>
</Row>
Expand Down
11 changes: 11 additions & 0 deletions superset/assets/javascripts/explore/main.css
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,14 @@
background-color: transparent;
}

.type-label {
margin-right: 8px;
width: 30px;
display: inline-block;
text-align: center;
font-weight: bold;
}

.datasource-container {
overflow: auto;
}
23 changes: 13 additions & 10 deletions superset/assets/javascripts/explore/stores/controls.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ const groupByControl = {
default: [],
includeTime: false,
description: t('One or many controls to group by'),
optionRenderer: c => <ColumnOption column={c} />,
optionRenderer: c => <ColumnOption column={c} showType />,
valueRenderer: c => <ColumnOption column={c} />,
valueKey: 'column_name',
mapStateToProps: (state, control) => {
Expand Down Expand Up @@ -129,7 +129,7 @@ export const controls = {
label: t('Metrics'),
validators: [v.nonEmpty],
valueKey: 'metric_name',
optionRenderer: m => <MetricOption metric={m} />,
optionRenderer: m => <MetricOption metric={m} showType />,
valueRenderer: m => <MetricOption metric={m} />,
default: c => c.options && c.options.length > 0 ? [c.options[0].metric_name] : null,
mapStateToProps: state => ({
Expand All @@ -143,7 +143,7 @@ export const controls = {
multi: true,
label: t('Percentage Metrics'),
valueKey: 'metric_name',
optionRenderer: m => <MetricOption metric={m} />,
optionRenderer: m => <MetricOption metric={m} showType />,
valueRenderer: m => <MetricOption metric={m} />,
mapStateToProps: state => ({
options: (state.datasource) ? state.datasource.metrics : [],
Expand Down Expand Up @@ -216,7 +216,7 @@ export const controls = {
clearable: false,
description: t('Choose the metric'),
validators: [v.nonEmpty],
optionRenderer: m => <MetricOption metric={m} />,
optionRenderer: m => <MetricOption metric={m} showType />,
valueRenderer: m => <MetricOption metric={m} />,
default: c => c.options && c.options.length > 0 ? c.options[0].metric_name : null,
valueKey: 'metric_name',
Expand All @@ -233,7 +233,7 @@ export const controls = {
clearable: true,
description: t('Choose a metric for right axis'),
valueKey: 'metric_name',
optionRenderer: m => <MetricOption metric={m} />,
optionRenderer: m => <MetricOption metric={m} showType />,
valueRenderer: m => <MetricOption metric={m} />,
mapStateToProps: state => ({
options: (state.datasource) ? state.datasource.metrics : [],
Expand Down Expand Up @@ -536,8 +536,11 @@ export const controls = {
label: t('Columns'),
default: [],
description: t('Columns to display'),
optionRenderer: c => <ColumnOption column={c} showType />,
valueRenderer: c => <ColumnOption column={c} />,
valueKey: 'column_name',
mapStateToProps: state => ({
choices: (state.datasource) ? state.datasource.all_cols : [],
options: (state.datasource) ? state.datasource.columns : [],
}),
},

Expand Down Expand Up @@ -770,7 +773,7 @@ export const controls = {
return null;
},
clearable: false,
optionRenderer: c => <ColumnOption column={c} />,
optionRenderer: c => <ColumnOption column={c} showType />,
valueRenderer: c => <ColumnOption column={c} />,
valueKey: 'column_name',
mapStateToProps: (state) => {
Expand Down Expand Up @@ -992,7 +995,7 @@ export const controls = {
description: t('Metric assigned to the [X] axis'),
default: null,
validators: [v.nonEmpty],
optionRenderer: m => <MetricOption metric={m} />,
optionRenderer: m => <MetricOption metric={m} showType />,
valueRenderer: m => <MetricOption metric={m} />,
valueKey: 'metric_name',
mapStateToProps: state => ({
Expand All @@ -1006,7 +1009,7 @@ export const controls = {
default: null,
validators: [v.nonEmpty],
description: t('Metric assigned to the [Y] axis'),
optionRenderer: m => <MetricOption metric={m} />,
optionRenderer: m => <MetricOption metric={m} showType />,
valueRenderer: m => <MetricOption metric={m} />,
valueKey: 'metric_name',
mapStateToProps: state => ({
Expand All @@ -1019,7 +1022,7 @@ export const controls = {
label: t('Bubble Size'),
default: null,
validators: [v.nonEmpty],
optionRenderer: m => <MetricOption metric={m} />,
optionRenderer: m => <MetricOption metric={m} showType />,
valueRenderer: m => <MetricOption metric={m} />,
valueKey: 'metric_name',
mapStateToProps: state => ({
Expand Down
20 changes: 20 additions & 0 deletions superset/assets/spec/javascripts/components/ColumnOption_spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { describe, it } from 'mocha';
import { shallow } from 'enzyme';

import ColumnOption from '../../../javascripts/components/ColumnOption';
import ColumnTypeLabel from '../../../javascripts/components/ColumnTypeLabel';
import InfoTooltipWithTrigger from '../../../javascripts/components/InfoTooltipWithTrigger';

describe('ColumnOption', () => {
Expand All @@ -14,6 +15,7 @@ describe('ColumnOption', () => {
expression: 'SUM(foo)',
description: 'Foo is the greatest column of all',
},
showType: false,
};

let wrapper;
Expand Down Expand Up @@ -44,4 +46,22 @@ describe('ColumnOption', () => {
wrapper = shallow(factory(props));
expect(wrapper.find('.option-label').first().text()).to.equal('foo');
});
it('shows a column type label when showType is true', () => {
props.showType = true;
wrapper = shallow(factory(props));
expect(wrapper.find(ColumnTypeLabel)).to.have.length(1);
});
it('column with expression has correct column label if showType is true', () => {
props.showType = true;
wrapper = shallow(factory(props));
expect(wrapper.find(ColumnTypeLabel)).to.have.length(1);
expect(wrapper.find(ColumnTypeLabel).props().type).to.equal('expression');
});
it('dttm column has correct column label if showType is true', () => {
props.showType = true;
props.column.is_dttm = true;
wrapper = shallow(factory(props));
expect(wrapper.find(ColumnTypeLabel)).to.have.length(1);
expect(wrapper.find(ColumnTypeLabel).props().type).to.equal('time');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import React from 'react';
import { expect } from 'chai';
import { describe, it } from 'mocha';
import { shallow } from 'enzyme';

import ColumnTypeLabel from '../../../javascripts/components/ColumnTypeLabel';

describe('ColumnOption', () => {
const defaultProps = {
type: 'string',
};

const props = { ...defaultProps };

function getWrapper(overrides) {
const wrapper = shallow(<ColumnTypeLabel {...props} {...overrides} />);
return wrapper;
}

it('is a valid element', () => {
expect(React.isValidElement(<ColumnTypeLabel {...defaultProps} />)).to.equal(true);
});
it('string type shows ABC icon', () => {
const lbl = getWrapper({}).find('.type-label');
expect(lbl).to.have.length(1);
expect(lbl.first().text()).to.equal('ABC');
});
it('int type shows # icon', () => {
const lbl = getWrapper({ type: 'int(164)' }).find('.type-label');
expect(lbl).to.have.length(1);
expect(lbl.first().text()).to.equal('#');
});
it('bool type shows T/F icon', () => {
const lbl = getWrapper({ type: 'BOOL' }).find('.type-label');
expect(lbl).to.have.length(1);
expect(lbl.first().text()).to.equal('T/F');
});
it('expression type shows function icon', () => {
const lbl = getWrapper({ type: 'expression' }).find('.type-label');
expect(lbl).to.have.length(1);
expect(lbl.first().text()).to.equal('ƒ');
});
it('unknown type shows question mark', () => {
const lbl = getWrapper({ type: 'unknown' }).find('.type-label');
expect(lbl).to.have.length(1);
expect(lbl.first().text()).to.equal('?');
});
it('unknown type shows question mark', () => {
const lbl = getWrapper({ type: 'datetime' }).find('.fa-clock-o');
expect(lbl).to.have.length(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { describe, it } from 'mocha';
import { shallow } from 'enzyme';

import MetricOption from '../../../javascripts/components/MetricOption';
import ColumnTypeLabel from '../../../javascripts/components/ColumnTypeLabel';
import InfoTooltipWithTrigger from '../../../javascripts/components/InfoTooltipWithTrigger';

describe('MetricOption', () => {
Expand All @@ -15,6 +16,7 @@ describe('MetricOption', () => {
description: 'Foo is the greatest metric of all',
warning_text: 'Be careful when using foo',
},
showType: false,
};

let wrapper;
Expand Down Expand Up @@ -59,4 +61,9 @@ describe('MetricOption', () => {
wrapper = shallow(factory(props));
expect(wrapper.find('a').prop('target')).to.equal('_blank');
});
it('shows a metric type label when showType is true', () => {
props.showType = true;
wrapper = shallow(factory(props));
expect(wrapper.find(ColumnTypeLabel)).to.have.length(1);
});
});

0 comments on commit 3f1dfb3

Please sign in to comment.