williaster closed pull request #4566: Adding column type label to dropdowns
URL: https://github.com/apache/incubator-superset/pull/4566
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/superset/assets/javascripts/components/ColumnOption.jsx
b/superset/assets/javascripts/components/ColumnOption.jsx
index f579126b8d..0a16db6869 100644
--- a/superset/assets/javascripts/components/ColumnOption.jsx
+++ b/superset/assets/javascripts/components/ColumnOption.jsx
@@ -1,6 +1,7 @@
import React from 'react';
import PropTypes from 'prop-types';
+import ColumnTypeLabel from './ColumnTypeLabel';
import InfoTooltipWithTrigger from './InfoTooltipWithTrigger';
const propTypes = {
@@ -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>
@@ -25,7 +36,7 @@ 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"
@@ -33,9 +44,6 @@ export default function ColumnOption({ column, showType }) {
label={`expr-${column.column_name}`}
/>
}
- {showType &&
- <span className="text-muted">{column.type}</span>
- }
</span>);
}
ColumnOption.propTypes = propTypes;
diff --git a/superset/assets/javascripts/components/ColumnTypeLabel.jsx
b/superset/assets/javascripts/components/ColumnTypeLabel.jsx
new file mode 100644
index 0000000000..719891e4a2
--- /dev/null
+++ b/superset/assets/javascripts/components/ColumnTypeLabel.jsx
@@ -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;
diff --git a/superset/assets/javascripts/components/MetricOption.jsx
b/superset/assets/javascripts/components/MetricOption.jsx
index 8d27ea4de5..81838f0d0a 100644
--- a/superset/assets/javascripts/components/MetricOption.jsx
+++ b/superset/assets/javascripts/components/MetricOption.jsx
@@ -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
diff --git
a/superset/assets/javascripts/explore/components/controls/DatasourceControl.jsx
b/superset/assets/javascripts/explore/components/controls/DatasourceControl.jsx
index 89b378de11..0256aacc38 100644
---
a/superset/assets/javascripts/explore/components/controls/DatasourceControl.jsx
+++
b/superset/assets/javascripts/explore/components/controls/DatasourceControl.jsx
@@ -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 => (
@@ -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>
diff --git a/superset/assets/javascripts/explore/main.css
b/superset/assets/javascripts/explore/main.css
index d21c4fc4ab..2add0abf68 100644
--- a/superset/assets/javascripts/explore/main.css
+++ b/superset/assets/javascripts/explore/main.css
@@ -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;
+}
diff --git a/superset/assets/javascripts/explore/stores/controls.jsx
b/superset/assets/javascripts/explore/stores/controls.jsx
index 5e2c44e839..3f1bbf7c6c 100644
--- a/superset/assets/javascripts/explore/stores/controls.jsx
+++ b/superset/assets/javascripts/explore/stores/controls.jsx
@@ -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) => {
@@ -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 => ({
@@ -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 : [],
@@ -202,7 +202,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',
@@ -219,7 +219,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 : [],
@@ -522,8 +522,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 : [],
}),
},
@@ -756,7 +759,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) => {
@@ -978,7 +981,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 => ({
@@ -992,7 +995,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 => ({
@@ -1005,7 +1008,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 => ({
diff --git a/superset/assets/spec/javascripts/components/ColumnOption_spec.jsx
b/superset/assets/spec/javascripts/components/ColumnOption_spec.jsx
index 29b4399ffb..4768f16e94 100644
--- a/superset/assets/spec/javascripts/components/ColumnOption_spec.jsx
+++ b/superset/assets/spec/javascripts/components/ColumnOption_spec.jsx
@@ -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', () => {
@@ -14,6 +15,7 @@ describe('ColumnOption', () => {
expression: 'SUM(foo)',
description: 'Foo is the greatest column of all',
},
+ showType: false,
};
let wrapper;
@@ -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');
+ });
});
diff --git
a/superset/assets/spec/javascripts/components/ColumnTypeLabel_spec.jsx
b/superset/assets/spec/javascripts/components/ColumnTypeLabel_spec.jsx
new file mode 100644
index 0000000000..7ce058be73
--- /dev/null
+++ b/superset/assets/spec/javascripts/components/ColumnTypeLabel_spec.jsx
@@ -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);
+ });
+});
diff --git a/superset/assets/spec/javascripts/components/MetricOption_spec.jsx
b/superset/assets/spec/javascripts/components/MetricOption_spec.jsx
index 4eeb13ed1f..0da6c62573 100644
--- a/superset/assets/spec/javascripts/components/MetricOption_spec.jsx
+++ b/superset/assets/spec/javascripts/components/MetricOption_spec.jsx
@@ -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', () => {
@@ -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;
@@ -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);
+ });
});
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
