superset-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ccwilli...@apache.org
Subject [incubator-superset] branch master updated: Adding column type label to dropdowns (#4566)
Date Fri, 16 Mar 2018 21:19:12 GMT
This is an automated email from the ASF dual-hosted git repository.

ccwilliams pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 3f1dfb3  Adding column type label to dropdowns (#4566)
3f1dfb3 is described below

commit 3f1dfb31731a1611c5552018bd7328abbf1598c9
Author: michellethomas <michelle.q.thomas@gmail.com>
AuthorDate: Fri Mar 16 14:19:09 2018 -0700

    Adding column type label to dropdowns (#4566)
    
    * 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
---
 .../assets/javascripts/components/ColumnOption.jsx | 16 +++++--
 .../javascripts/components/ColumnTypeLabel.jsx     | 33 ++++++++++++++
 .../assets/javascripts/components/MetricOption.jsx |  6 ++-
 .../components/controls/DatasourceControl.jsx      |  4 +-
 superset/assets/javascripts/explore/main.css       | 11 +++++
 .../assets/javascripts/explore/stores/controls.jsx | 23 +++++-----
 .../javascripts/components/ColumnOption_spec.jsx   | 20 +++++++++
 .../components/ColumnTypeLabel_spec.jsx            | 52 ++++++++++++++++++++++
 .../javascripts/components/MetricOption_spec.jsx   |  7 +++
 9 files changed, 155 insertions(+), 17 deletions(-)

diff --git a/superset/assets/javascripts/components/ColumnOption.jsx b/superset/assets/javascripts/components/ColumnOption.jsx
index f579126..0a16db6 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 0000000..719891e
--- /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 8d27ea4..81838f0 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 89b378d..0256aac 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 d21c4fc..2add0ab 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 800e1df..cc35d0e 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 : [],
@@ -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',
@@ -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 : [],
@@ -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 : [],
     }),
   },
 
@@ -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) => {
@@ -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 => ({
@@ -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 => ({
@@ -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 => ({
diff --git a/superset/assets/spec/javascripts/components/ColumnOption_spec.jsx b/superset/assets/spec/javascripts/components/ColumnOption_spec.jsx
index 29b4399..4768f16 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 0000000..7ce058b
--- /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 4eeb13e..0da6c62 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);
+  });
 });

-- 
To stop receiving notification emails like this one, please contact
ccwilliams@apache.org.

Mime
View raw message