From f35f2bd5d19dc5d26cec46a7308a63f76cf7a962 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Sat, 7 Apr 2018 15:19:58 -0700 Subject: [PATCH] [line] fix verbose names in time shift (#4765) * [line] fix verbose names in time shift * Addressing comments --- .../visualizations/nvd3_viz_spec.jsx | 28 +++++++++++++++++++ superset/assets/visualizations/main.js | 23 +++++++-------- superset/assets/visualizations/nvd3_vis.js | 23 ++++++++------- superset/viz.py | 9 +++--- 4 files changed, 56 insertions(+), 27 deletions(-) create mode 100644 superset/assets/spec/javascripts/visualizations/nvd3_viz_spec.jsx diff --git a/superset/assets/spec/javascripts/visualizations/nvd3_viz_spec.jsx b/superset/assets/spec/javascripts/visualizations/nvd3_viz_spec.jsx new file mode 100644 index 0000000000000..c8117234ce9e2 --- /dev/null +++ b/superset/assets/spec/javascripts/visualizations/nvd3_viz_spec.jsx @@ -0,0 +1,28 @@ +import { describe, it } from 'mocha'; +import { expect } from 'chai'; + +import { formatLabel } from '../../../visualizations/nvd3_vis'; + +describe('nvd3 viz', () => { + const verboseMap = { + foo: 'Foo', + bar: 'Bar', + }; + describe('formatLabel', () => { + it('formats simple labels', () => { + expect(formatLabel('foo')).to.equal('foo'); + expect(formatLabel(['foo'])).to.equal('foo'); + expect(formatLabel(['foo', 'bar'])).to.equal('foo, bar'); + }); + it('formats simple labels with lookups', () => { + expect(formatLabel('foo', verboseMap)).to.equal('Foo'); + expect(formatLabel('baz', verboseMap)).to.equal('baz'); + expect(formatLabel(['foo'], verboseMap)).to.equal('Foo'); + expect(formatLabel(['foo', 'bar', 'baz'], verboseMap)).to.equal('Foo, Bar, baz'); + }); + it('deals with --- properly', () => { + expect(formatLabel(['foo', '---'], verboseMap)).to.equal('Foo ---'); + expect(formatLabel(['foo', 'bar', 'baz', '---'], verboseMap)).to.equal('Foo, Bar, baz ---'); + }); + }); +}); diff --git a/superset/assets/visualizations/main.js b/superset/assets/visualizations/main.js index 4aaae3df2453b..5477c337b1e03 100644 --- a/superset/assets/visualizations/main.js +++ b/superset/assets/visualizations/main.js @@ -1,4 +1,5 @@ /* eslint-disable global-require */ +import nvd3Vis from './nvd3_vis'; // You ***should*** use these to reference viz_types in code export const VIZ_TYPES = { @@ -52,29 +53,29 @@ export const VIZ_TYPES = { }; const vizMap = { - [VIZ_TYPES.area]: require('./nvd3_vis.js'), - [VIZ_TYPES.bar]: require('./nvd3_vis.js'), + [VIZ_TYPES.area]: nvd3Vis, + [VIZ_TYPES.bar]: nvd3Vis, [VIZ_TYPES.big_number]: require('./big_number.js'), [VIZ_TYPES.big_number_total]: require('./big_number.js'), - [VIZ_TYPES.box_plot]: require('./nvd3_vis.js'), - [VIZ_TYPES.bubble]: require('./nvd3_vis.js'), - [VIZ_TYPES.bullet]: require('./nvd3_vis.js'), + [VIZ_TYPES.box_plot]: nvd3Vis, + [VIZ_TYPES.bubble]: nvd3Vis, + [VIZ_TYPES.bullet]: nvd3Vis, [VIZ_TYPES.cal_heatmap]: require('./cal_heatmap.js'), - [VIZ_TYPES.compare]: require('./nvd3_vis.js'), + [VIZ_TYPES.compare]: nvd3Vis, [VIZ_TYPES.directed_force]: require('./directed_force.js'), [VIZ_TYPES.chord]: require('./chord.jsx'), - [VIZ_TYPES.dist_bar]: require('./nvd3_vis.js'), + [VIZ_TYPES.dist_bar]: nvd3Vis, [VIZ_TYPES.filter_box]: require('./filter_box.jsx'), [VIZ_TYPES.heatmap]: require('./heatmap.js'), [VIZ_TYPES.histogram]: require('./histogram.js'), [VIZ_TYPES.horizon]: require('./horizon.js'), [VIZ_TYPES.iframe]: require('./iframe.js'), - [VIZ_TYPES.line]: require('./nvd3_vis.js'), - [VIZ_TYPES.time_pivot]: require('./nvd3_vis.js'), + [VIZ_TYPES.line]: nvd3Vis, + [VIZ_TYPES.time_pivot]: nvd3Vis, [VIZ_TYPES.mapbox]: require('./mapbox.jsx'), [VIZ_TYPES.markup]: require('./markup.js'), [VIZ_TYPES.para]: require('./parallel_coordinates.js'), - [VIZ_TYPES.pie]: require('./nvd3_vis.js'), + [VIZ_TYPES.pie]: nvd3Vis, [VIZ_TYPES.pivot_table]: require('./pivot_table.js'), [VIZ_TYPES.sankey]: require('./sankey.js'), [VIZ_TYPES.separator]: require('./markup.js'), @@ -85,7 +86,7 @@ const vizMap = { [VIZ_TYPES.country_map]: require('./country_map.js'), [VIZ_TYPES.word_cloud]: require('./word_cloud.js'), [VIZ_TYPES.world_map]: require('./world_map.js'), - [VIZ_TYPES.dual_line]: require('./nvd3_vis.js'), + [VIZ_TYPES.dual_line]: nvd3Vis, [VIZ_TYPES.event_flow]: require('./EventFlow.jsx'), [VIZ_TYPES.paired_ttest]: require('./paired_ttest.jsx'), [VIZ_TYPES.partition]: require('./partition.js'), diff --git a/superset/assets/visualizations/nvd3_vis.js b/superset/assets/visualizations/nvd3_vis.js index fb439ec7209ff..b479c682c43d7 100644 --- a/superset/assets/visualizations/nvd3_vis.js +++ b/superset/assets/visualizations/nvd3_vis.js @@ -83,23 +83,24 @@ function getMaxLabelSize(container, axisClass) { return Math.max(...labelDimensions); } -/* eslint-disable camelcase */ -function formatLabel(column, verbose_map) { +export function formatLabel(input, verboseMap = {}) { + // The input for label may be a string or an array of string + // When using the time shift feature, the label contains a '---' in the array + const verboseLkp = s => verboseMap[s] || s; let label; - if (Array.isArray(column) && column.length) { - label = verbose_map[column[0]] || column[0]; - if (column.length > 1) { - label += ', '; + if (Array.isArray(input) && input.length) { + const verboseLabels = input.filter(s => s !== '---').map(verboseLkp); + label = verboseLabels.join(', '); + if (input.length > verboseLabels.length) { + label += ' ---'; } - label += column.slice(1).join(', '); } else { - label = verbose_map[column] || column; + label = verboseLkp(input); } return label; } -/* eslint-enable camelcase */ -function nvd3Vis(slice, payload) { +export default function nvd3Vis(slice, payload) { let chart; let colorKey = 'key'; const isExplore = $('#explore-container').length === 1; @@ -778,5 +779,3 @@ function nvd3Vis(slice, payload) { nv.addGraph(drawGraph); } - -module.exports = nvd3Vis; diff --git a/superset/viz.py b/superset/viz.py index 7d0fabb9211d3..42e8e5a97a456 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -1061,10 +1061,11 @@ def to_series(self, df, classed='', title_suffix=''): len(self.metrics) == 1): # Removing metric from series name if only one metric series_title = series_title[1:] - if isinstance(series_title, string_types): - series_title += title_suffix - elif title_suffix and isinstance(series_title, (list, tuple)): - series_title = text_type(series_title[-1]) + title_suffix + if title_suffix: + if isinstance(series_title, string_types): + series_title = (series_title, title_suffix) + elif isinstance(series_title, (list, tuple)): + series_title = series_title + (title_suffix,) values = [] for ds in df.index: