Improve xAxis ticks, thinner bottom margin (apache#4756)

* Improve xAxis ticks, thinner bottom margin * Moving utils folder * Add isTruthy
michellethomas · May 23, 2018 · 997041e · 997041e
1 parent 10fef4e
commit 997041e
Show file tree

Hide file tree

Showing 13 changed files with 213 additions and 114 deletions.
diff --git a/superset/assets/javascripts/SqlLab/components/CopyQueryTabUrl.jsx b/superset/assets/javascripts/SqlLab/components/CopyQueryTabUrl.jsx
@@ -1,7 +1,7 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import CopyToClipboard from '../../components/CopyToClipboard';
-import { storeQuery } from '../../../utils/common';
+import { storeQuery } from '../../utils/common';
 import { t } from '../../locales';
 
 const propTypes = {

diff --git a/superset/assets/javascripts/SqlLab/components/QueryTable.jsx b/superset/assets/javascripts/SqlLab/components/QueryTable.jsx
@@ -10,7 +10,7 @@ import ResultSet from './ResultSet';
 import ModalTrigger from '../../components/ModalTrigger';
 import HighlightedSql from './HighlightedSql';
 import { fDuration } from '../../modules/dates';
-import { storeQuery } from '../../../utils/common';
+import { storeQuery } from '../../utils/common';
 import QueryStateLabel from './QueryStateLabel';
 import { t } from '../../locales';
 

diff --git a/superset/assets/javascripts/chart/Chart.jsx b/superset/assets/javascripts/chart/Chart.jsx
@@ -127,9 +127,7 @@ class Chart extends React.PureComponent {
   }
 
   clearError() {
-    this.setState({
-      errorMsg: null,
-    });
+    this.setState({ errorMsg: null });
   }
 
   width() {
@@ -151,6 +149,10 @@ class Chart extends React.PureComponent {
     return d3format(format, number);
   }
 
+  error(e) {
+    this.props.actions.chartRenderingFailed(e, this.props.chartKey);
+  }
+
   render_template(s) {
     const context = {
       width: this.width(),
@@ -199,7 +201,6 @@ class Chart extends React.PureComponent {
       });
       this.props.actions.chartRenderingSucceeded(this.props.chartKey);
     } catch (e) {
-      console.error(e);  // eslint-disable-line
       this.props.actions.chartRenderingFailed(e, this.props.chartKey);
     }
   }

diff --git a/superset/assets/javascripts/explore/components/SaveModal.jsx b/superset/assets/javascripts/explore/components/SaveModal.jsx
@@ -6,7 +6,7 @@ import { connect } from 'react-redux';
 import { Modal, Alert, Button, Radio } from 'react-bootstrap';
 import Select from 'react-select';
 import { t } from '../../locales';
-import { supersetURL } from '../../../utils/common';
+import { supersetURL } from '../../utils/common';
 
 const propTypes = {
   can_overwrite: PropTypes.bool,

diff --git a/superset/assets/javascripts/explore/components/URLShortLinkButton.jsx b/superset/assets/javascripts/explore/components/URLShortLinkButton.jsx
@@ -2,7 +2,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import { Popover, OverlayTrigger } from 'react-bootstrap';
 import CopyToClipboard from './../../components/CopyToClipboard';
-import { getShortUrl } from '../../../utils/common';
+import { getShortUrl } from '../../utils/common';
 import { getExploreLongUrl } from '../exploreUtils';
 import { t } from '../../locales';
 

diff --git a/superset/assets/javascripts/explore/stores/controls.jsx b/superset/assets/javascripts/explore/stores/controls.jsx
@@ -724,6 +724,16 @@ export const controls = {
     description: t('Bottom margin, in pixels, allowing for more room for axis labels'),
   },
 
+  x_ticks_layout: {
+    type: 'SelectControl',
+    label: t('X Tick Layout'),
+    choices: formatSelectOptions(['auto', 'flat', '45°', 'staggered']),
+    default: 'auto',
+    clearable: false,
+    renderTrigger: true,
+    description: t('The way the ticks are laid out on the X axis'),
+  },
+
   left_margin: {
     type: 'SelectControl',
     freeForm: true,

diff --git a/superset/assets/javascripts/explore/stores/visTypes.js b/superset/assets/javascripts/explore/stores/visTypes.js
@@ -118,9 +118,16 @@ export const visTypes = {
           ['color_scheme'],
           ['show_legend', 'show_bar_value'],
           ['bar_stacked', 'order_bars'],
-          ['y_axis_format', 'bottom_margin'],
-          ['x_axis_label', 'y_axis_label'],
-          ['reduce_x_ticks', 'show_controls'],
+          ['y_axis_format', 'y_axis_label'],
+          ['show_controls', null],
+        ],
+      },
+      {
+        label: t('X Axis'),
+        expanded: true,
+        controlSetRows: [
+          ['x_axis_label', 'bottom_margin'],
+          ['x_ticks_layout', 'reduce_x_ticks'],
         ],
       },
     ],
@@ -182,7 +189,8 @@ export const visTypes = {
         expanded: true,
         controlSetRows: [
           ['x_axis_label', 'bottom_margin'],
-          ['x_axis_showminmax', 'x_axis_format'],
+          ['x_ticks_layout', 'x_axis_format'],
+          ['x_axis_showminmax', null],
         ],
       },
       {
@@ -312,13 +320,21 @@ export const visTypes = {
         ],
       },
       {
-        label: t('Axes'),
+        label: t('X Axis'),
         expanded: true,
         controlSetRows: [
-          ['x_axis_format', 'y_axis_format'],
+          ['x_axis_label', 'bottom_margin'],
+          ['x_ticks_layout', 'x_axis_format'],
           ['x_axis_showminmax', 'reduce_x_ticks'],
-          ['x_axis_label', 'y_axis_label'],
-          ['y_axis_bounds', 'y_log_scale'],
+        ],
+      },
+      {
+        label: t('Y Axis'),
+        expanded: true,
+        controlSetRows: [
+          ['y_axis_label', 'left_margin'],
+          ['y_axis_showminmax', 'y_log_scale'],
+          ['y_axis_format', 'y_axis_bounds'],
         ],
       },
       sections.NVD3TimeSeries[1],
@@ -342,7 +358,24 @@ export const visTypes = {
         expanded: true,
         controlSetRows: [
           ['color_scheme'],
-          ['x_axis_format', 'y_axis_format'],
+        ],
+      },
+      {
+        label: t('X Axis'),
+        expanded: true,
+        controlSetRows: [
+          ['x_axis_label', 'bottom_margin'],
+          ['x_ticks_layout', 'x_axis_format'],
+          ['x_axis_showminmax', null],
+        ],
+      },
+      {
+        label: t('Y Axis'),
+        expanded: true,
+        controlSetRows: [
+          ['y_axis_label', 'left_margin'],
+          ['y_axis_showminmax', 'y_log_scale'],
+          ['y_axis_format', 'y_axis_bounds'],
         ],
       },
       sections.NVD3TimeSeries[1],
@@ -724,10 +757,18 @@ export const visTypes = {
         ],
       },
       {
-        label: t('Axes'),
+        label: t('X Axis'),
+        expanded: true,
+        controlSetRows: [
+          ['x_axis_label', 'bottom_margin'],
+          ['x_ticks_layout', 'x_axis_format'],
+          ['x_axis_showminmax', null],
+        ],
+      },
+      {
+        label: t('Y Axis'),
         expanded: true,
         controlSetRows: [
-          ['x_axis_format', 'x_axis_showminmax'],
           ['y_axis_format', 'y_axis_bounds'],
           ['y_log_scale', null],
         ],
@@ -979,7 +1020,9 @@ export const visTypes = {
         expanded: true,
         controlSetRows: [
           ['series', 'entity'],
-          ['size', 'limit'],
+          ['x', 'y'],
+          ['size', 'max_bubble_size'],
+          ['limit', null],
         ],
       },
       {
@@ -990,18 +1033,12 @@ export const visTypes = {
           ['show_legend', null],
         ],
       },
-      {
-        label: t('Bubbles'),
-        controlSetRows: [
-          ['size', 'max_bubble_size'],
-        ],
-      },
       {
         label: t('X Axis'),
         expanded: true,
         controlSetRows: [
           ['x_axis_label', 'left_margin'],
-          ['x', 'x_axis_format'],
+          ['x_axis_format', 'x_ticks_layout'],
           ['x_log_scale', 'x_axis_showminmax'],
         ],
       },
@@ -1010,7 +1047,7 @@ export const visTypes = {
         expanded: true,
         controlSetRows: [
           ['y_axis_label', 'bottom_margin'],
-          ['y', 'y_axis_format'],
+          ['y_axis_format', null],
           ['y_log_scale', 'y_axis_showminmax'],
         ],
       },

diff --git a/superset/assets/utils/common.js → superset/assets/javascripts/utils/common.js b/superset/assets/utils/common.js → superset/assets/javascripts/utils/common.js
@@ -94,3 +94,12 @@ export function supersetURL(rootUrl, getParams = {}) {
   }
   return url.href;
 }
+
+export function isTruthy(obj) {
+  if (typeof obj === 'boolean') {
+    return obj;
+  } else if (typeof obj === 'string') {
+    return ['yes', 'y', 'true', 't', '1'].indexOf(obj.toLowerCase()) >= 0;
+  }
+  return !!obj;
+}
diff --git a/superset/assets/utils/reducerUtils.js → .../assets/javascripts/utils/reducerUtils.js b/superset/assets/utils/reducerUtils.js → .../assets/javascripts/utils/reducerUtils.js
diff --git a/superset/assets/spec/javascripts/explore/utils_spec.jsx b/superset/assets/spec/javascripts/explore/utils_spec.jsx
@@ -3,7 +3,7 @@ import { expect } from 'chai';
 import URI from 'urijs';
 import { getExploreUrlAndPayload, getExploreLongUrl } from '../../../javascripts/explore/exploreUtils';
 
-describe('utils', () => {
+describe('exploreUtils', () => {
   const location = window.location;
   const formData = {
     datasource: '1__table',

diff --git a/superset/assets/spec/javascripts/utils/common_spec.jsx b/superset/assets/spec/javascripts/utils/common_spec.jsx
@@ -0,0 +1,43 @@
+import { it, describe } from 'mocha';
+import { expect } from 'chai';
+import { isTruthy } from '../../../javascripts/utils/common';
+
+describe('utils/common', () => {
+  describe('isTruthy', () => {
+    it('evals false-looking strings properly', () => {
+      expect(isTruthy('f')).to.equal(false);
+      expect(isTruthy('false')).to.equal(false);
+      expect(isTruthy('no')).to.equal(false);
+      expect(isTruthy('n')).to.equal(false);
+      expect(isTruthy('F')).to.equal(false);
+      expect(isTruthy('False')).to.equal(false);
+      expect(isTruthy('NO')).to.equal(false);
+      expect(isTruthy('N')).to.equal(false);
+    });
+    it('evals true-looking strings properly', () => {
+      expect(isTruthy('t')).to.equal(true);
+      expect(isTruthy('true')).to.equal(true);
+      expect(isTruthy('yes')).to.equal(true);
+      expect(isTruthy('y')).to.equal(true);
+      expect(isTruthy('Y')).to.equal(true);
+      expect(isTruthy('True')).to.equal(true);
+      expect(isTruthy('Yes')).to.equal(true);
+      expect(isTruthy('YES')).to.equal(true);
+    });
+    it('evals bools properly', () => {
+      expect(isTruthy(false)).to.equal(false);
+      expect(isTruthy(true)).to.equal(true);
+    });
+    it('evals ints properly', () => {
+      expect(isTruthy(0)).to.equal(false);
+      expect(isTruthy(1)).to.equal(true);
+    });
+    it('evals constants properly', () => {
+      expect(isTruthy(null)).to.equal(false);
+      expect(isTruthy(undefined)).to.equal(false);
+    });
+    it('string auto is false', () => {
+      expect(isTruthy('false')).to.equal(false);
+    });
+  });
+});
diff --git a/superset/assets/visualizations/mapbox.jsx b/superset/assets/visualizations/mapbox.jsx
@@ -17,7 +17,7 @@ import {
   DEFAULT_LONGITUDE,
   DEFAULT_LATITUDE,
   DEFAULT_ZOOM,
-} from '../utils/common';
+} from '../javascripts/utils/common';
 import './mapbox.css';
 
 const NOOP = () => {};