From e2bd40c89f98349ccf6a4c56609c13ec0c781f83 Mon Sep 17 00:00:00 2001 From: Chris Williams Date: Thu, 15 Mar 2018 15:13:24 -0700 Subject: [PATCH] [bug fixes] annotations <> x domains, zeros in text (#4194) * [bugs] account for annotations in nvd3 x scale domain, fix dynamic width explore charts, allow 0 in text control * tweak TextControl casting * [annotations] filter separately from finding data extent --- .../components/controls/TextControl.jsx | 9 ++-- superset/assets/visualizations/nvd3_vis.js | 52 +++++++++++++++---- 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/superset/assets/javascripts/explore/components/controls/TextControl.jsx b/superset/assets/javascripts/explore/components/controls/TextControl.jsx index ed1238e5097e2..a43ffc8f56fcf 100644 --- a/superset/assets/javascripts/explore/components/controls/TextControl.jsx +++ b/superset/assets/javascripts/explore/components/controls/TextControl.jsx @@ -31,11 +31,11 @@ export default class TextControl extends React.Component { this.onChange = this.onChange.bind(this); } onChange(event) { - let value = event.target.value || ''; + let value = event.target.value; // Validation & casting const errors = []; - if (this.props.isFloat) { + if (value !== '' && this.props.isFloat) { const error = v.numeric(value); if (error) { errors.push(error); @@ -43,7 +43,7 @@ export default class TextControl extends React.Component { value = parseFloat(value); } } - if (this.props.isInt) { + if (value !== '' && this.props.isInt) { const error = v.integer(value); if (error) { errors.push(error); @@ -54,7 +54,8 @@ export default class TextControl extends React.Component { this.props.onChange(value, errors); } render() { - const value = this.props.value ? this.props.value.toString() : ''; + const { value: rawValue } = this.props; + const value = typeof rawValue !== 'undefined' && rawValue !== null ? rawValue.toString() : ''; return (
diff --git a/superset/assets/visualizations/nvd3_vis.js b/superset/assets/visualizations/nvd3_vis.js index be73433d9e93a..7cfb1c332076c 100644 --- a/superset/assets/visualizations/nvd3_vis.js +++ b/superset/assets/visualizations/nvd3_vis.js @@ -544,8 +544,8 @@ function nvd3Vis(slice, payload) { // on scroll, hide tooltips. throttle to only 4x/second. $(window).scroll(throttle(hideTooltips, 250)); - const annotationLayers = (slice.formData.annotation_layers || []) - .filter(x => x.show); + const annotationLayers = (slice.formData.annotation_layers || []).filter(x => x.show); + if (isTimeSeries && annotationLayers) { // Formula annotations const formulas = annotationLayers.filter(a => a.annotationType === AnnotationTypes.FORMULA) @@ -634,13 +634,27 @@ function nvd3Vis(slice, payload) { const tip = tipFactory(e); const records = (slice.annotationData[e.name].records || []).map((r) => { - const timeColumn = new Date(moment.utc(r[e.timeColumn])); + const timeValue = new Date(moment.utc(r[e.timeColumn])); + return { ...r, - [e.timeColumn]: timeColumn, + [e.timeColumn]: timeValue, }; - }).filter(r => !Number.isNaN(r[e.timeColumn].getMilliseconds())); + }).filter(record => !Number.isNaN(record[e.timeColumn].getMilliseconds())); + + // account for the annotation in the x domain + records.forEach((record) => { + const timeValue = record[e.timeColumn]; + + xMin = Math.min(...[xMin, timeValue]); + xMax = Math.max(...[xMax, timeValue]); + }); + if (records.length) { + const domain = [xMin, xMax]; + xScale.domain(domain); + chart.xDomain(domain); + annotations.selectAll('line') .data(records) .enter() @@ -675,16 +689,32 @@ function nvd3Vis(slice, payload) { const tip = tipFactory(e); const records = (slice.annotationData[e.name].records || []).map((r) => { - const timeColumn = new Date(moment.utc(r[e.timeColumn])); - const intervalEndColumn = new Date(moment.utc(r[e.intervalEndColumn])); + const timeValue = new Date(moment.utc(r[e.timeColumn])); + const intervalEndValue = new Date(moment.utc(r[e.intervalEndColumn])); return { ...r, - [e.timeColumn]: timeColumn, - [e.intervalEndColumn]: intervalEndColumn, + [e.timeColumn]: timeValue, + [e.intervalEndColumn]: intervalEndValue, }; - }).filter(r => !Number.isNaN(r[e.timeColumn].getMilliseconds()) && - !Number.isNaN(r[e.intervalEndColumn].getMilliseconds())); + }).filter(record => ( + !Number.isNaN(record[e.timeColumn].getMilliseconds()) && + !Number.isNaN(record[e.intervalEndColumn].getMilliseconds()) + )); + + // account for the annotation in the x domain + records.forEach((record) => { + const timeValue = record[e.timeColumn]; + const intervalEndValue = record[e.intervalEndColumn]; + + xMin = Math.min(...[xMin, timeValue, intervalEndValue]); + xMax = Math.max(...[xMax, timeValue, intervalEndValue]); + }); + if (records.length) { + const domain = [xMin, xMax]; + xScale.domain(domain); + chart.xDomain(domain); + annotations.selectAll('rect') .data(records) .enter()