From 1ccd426a5670b6eedb271c8bbe3896aa8f1e0497 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Tue, 11 Sep 2018 12:24:28 +0200 Subject: [PATCH 01/14] [ML] Updated method to determine ticks. --- .../explorer_charts/explorer_chart.js | 36 +++++++++++++++++-- .../styles/explorer_chart.less | 8 +++-- x-pack/plugins/ml/public/util/chart_utils.js | 35 ++++++++++++++++++ 3 files changed, 74 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js index 5932ba8786d27..e321584631943 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js +++ b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js @@ -23,7 +23,7 @@ import moment from 'moment'; // because it won't work with the jest tests import { formatValue } from '../../formatters/format_value'; import { getSeverityWithLow } from '../../../common/util/anomaly_utils'; -import { drawLineChartDots, numTicksForDateFormat } from '../../util/chart_utils'; +import { drawLineChartDots, getTickValues } from '../../util/chart_utils'; import { TimeBuckets } from 'ui/time_buckets'; import { LoadingIndicator } from '../../components/loading_indicator/loading_indicator'; import { mlEscape } from '../../util/string_utils'; @@ -176,12 +176,18 @@ export class ExplorerChart extends React.Component { timeBuckets.setInterval('auto'); const xAxisTickFormat = timeBuckets.getScaledDateFormat(); + const emphasisStart = Math.max(config.selectedEarliest, config.plotEarliest); + const emphasisEnd = Math.min(config.selectedLatest, config.plotLatest); + // +1 ms to account for the ms that was substracted for query aggregations. + const interval = emphasisEnd - emphasisStart + 1; + const tickValues = getTickValues(emphasisStart, interval, config.plotEarliest, config.plotLatest); + const xAxis = d3.svg.axis().scale(lineChartXScale) .orient('bottom') .innerTickSize(-chartHeight) .outerTickSize(0) .tickPadding(10) - .ticks(numTicksForDateFormat(vizWidth, xAxisTickFormat)) + .tickValues(tickValues) .tickFormat(d => moment(d).format(xAxisTickFormat)); const yAxis = d3.svg.axis().scale(lineChartYScale) @@ -196,7 +202,7 @@ export class ExplorerChart extends React.Component { const axes = lineChartGroup.append('g'); - axes.append('g') + const gAxis = axes.append('g') .attr('class', 'x axis') .attr('transform', 'translate(0,' + chartHeight + ')') .call(xAxis); @@ -204,6 +210,30 @@ export class ExplorerChart extends React.Component { axes.append('g') .attr('class', 'y axis') .call(yAxis); + + // remove overlapping labels + let overlapCheck = 0; + gAxis.selectAll('g.tick').each(function () { + const tick = d3.select(this); + const xTransform = d3.transform(tick.attr('transform')).translate[0]; + const tickWidth = tick.select('text').node().getBBox().width; + const xMinOffset = xTransform - (tickWidth / 2); + const xMaxOffset = xTransform + (tickWidth / 2); + // if the tick label overlaps the previous label + // (or overflows the chart to the left), remove it; + // otherwise pick that label's offset as the new offset to check against + if (xMinOffset < overlapCheck) { + tick.select('text').remove(); + } else { + overlapCheck = xTransform + (tickWidth / 2); + tick.select('line').classed('ml-tick-emphasis', true); + } + // if the last tick label overflows the chart to the right, remove it + if (xMaxOffset > vizWidth) { + tick.select('text').remove(); + } + }); + } function drawLineChartHighlightedSpan() { diff --git a/x-pack/plugins/ml/public/explorer/explorer_charts/styles/explorer_chart.less b/x-pack/plugins/ml/public/explorer/explorer_charts/styles/explorer_chart.less index aad07728dd931..54d586da7c657 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_charts/styles/explorer_chart.less +++ b/x-pack/plugins/ml/public/explorer/explorer_charts/styles/explorer_chart.less @@ -31,12 +31,16 @@ ml-explorer-chart, shape-rendering: crispEdges; } + .axis .tick line.ml-tick-emphasis { + stroke: rgba(0, 0, 0, 0.2); + } + .axis text { - fill: #000; + fill: #888; } .axis .tick line { - stroke: rgba(0, 0, 0, 0.1); + stroke: rgba(0, 0, 0, 0.05); stroke-width: 1px; } diff --git a/x-pack/plugins/ml/public/util/chart_utils.js b/x-pack/plugins/ml/public/util/chart_utils.js index 260635ef14163..76b647f048e91 100644 --- a/x-pack/plugins/ml/public/util/chart_utils.js +++ b/x-pack/plugins/ml/public/util/chart_utils.js @@ -187,3 +187,38 @@ export function numTicksForDateFormat(axisWidth, dateFormat) { const tickWidth = calculateTextWidth(moment().format(dateFormat), false); return axisWidth / (1.75 * tickWidth); } + +// Based on a fixed starting timestamp and an interval, get tick values within +// the bounds of earliest and latest. This is useful for the Anomaly Explorer Charts +// to align axis ticks with the gray area resembling the swimlane cell selection. +export function getTickValues(startTs, tickInterval, earliest, latest) { + const tickValues = []; + + function addTicks(ts, operator) { + let newTick; + let addAnotherTick; + + switch (operator) { + case 'previous': + newTick = ts - tickInterval; + addAnotherTick = newTick >= earliest; + break; + case 'next': + newTick = ts + tickInterval; + addAnotherTick = newTick <= latest; + break; + } + + if (addAnotherTick) { + tickValues.push(newTick); + addTicks(newTick, operator); + } + } + + addTicks(startTs, 'previous'); + addTicks(startTs, 'next'); + + tickValues.sort(); + + return tickValues; +} From 953eff55c6a5dd62dd25096fe7748bcf6015ebf2 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Tue, 11 Sep 2018 12:37:48 +0200 Subject: [PATCH 02/14] [ML] Tweak chart container padding. --- .../public/explorer/explorer_charts/styles/explorer_chart.less | 1 + .../explorer_charts/styles/explorer_charts_container.less | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/ml/public/explorer/explorer_charts/styles/explorer_chart.less b/x-pack/plugins/ml/public/explorer/explorer_charts/styles/explorer_chart.less index 54d586da7c657..fc601b9e97846 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_charts/styles/explorer_chart.less +++ b/x-pack/plugins/ml/public/explorer/explorer_charts/styles/explorer_chart.less @@ -1,6 +1,7 @@ ml-explorer-chart, .ml-explorer-chart-container { display: block; + padding-bottom: 10px; svg { font-size: 12px; diff --git a/x-pack/plugins/ml/public/explorer/explorer_charts/styles/explorer_charts_container.less b/x-pack/plugins/ml/public/explorer/explorer_charts/styles/explorer_charts_container.less index bc8256b977384..9d1f205f9b224 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_charts/styles/explorer_charts_container.less +++ b/x-pack/plugins/ml/public/explorer/explorer_charts/styles/explorer_charts_container.less @@ -107,7 +107,8 @@ .explorer-chart-label-fields { vertical-align: top; - max-width: calc(~"100% - 15px"); + /* account 80px for the "View" link */ + max-width: calc(~"100% - 80px"); text-overflow: ellipsis; overflow: hidden; white-space: nowrap; From 8b0b67a46bb69397b294d8ca2b0bb27e4f6c0186 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Tue, 11 Sep 2018 12:54:44 +0200 Subject: [PATCH 03/14] [ML] Don't forget to add initial ts to tickValues. --- x-pack/plugins/ml/public/util/chart_utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/ml/public/util/chart_utils.js b/x-pack/plugins/ml/public/util/chart_utils.js index 76b647f048e91..4422d3620418e 100644 --- a/x-pack/plugins/ml/public/util/chart_utils.js +++ b/x-pack/plugins/ml/public/util/chart_utils.js @@ -192,7 +192,7 @@ export function numTicksForDateFormat(axisWidth, dateFormat) { // the bounds of earliest and latest. This is useful for the Anomaly Explorer Charts // to align axis ticks with the gray area resembling the swimlane cell selection. export function getTickValues(startTs, tickInterval, earliest, latest) { - const tickValues = []; + const tickValues = [startTs]; function addTicks(ts, operator) { let newTick; From 678bdb9a9345efe4578ee32de1b24fd36b55822d Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Tue, 11 Sep 2018 16:07:16 +0200 Subject: [PATCH 04/14] [ML] Axis label removal is done based on starting with the one at the beginning of the selected time span. --- .../explorer_charts/explorer_chart.js | 123 +++++++++++++++--- .../explorer_charts/explorer_chart.test.js | 2 +- 2 files changed, 105 insertions(+), 20 deletions(-) diff --git a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js index e321584631943..2de685ca7359b 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js +++ b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js @@ -212,27 +212,112 @@ export class ExplorerChart extends React.Component { .call(yAxis); // remove overlapping labels - let overlapCheck = 0; - gAxis.selectAll('g.tick').each(function () { - const tick = d3.select(this); - const xTransform = d3.transform(tick.attr('transform')).translate[0]; - const tickWidth = tick.select('text').node().getBBox().width; - const xMinOffset = xTransform - (tickWidth / 2); - const xMaxOffset = xTransform + (tickWidth / 2); - // if the tick label overlaps the previous label - // (or overflows the chart to the left), remove it; - // otherwise pick that label's offset as the new offset to check against - if (xMinOffset < overlapCheck) { - tick.select('text').remove(); - } else { - overlapCheck = xTransform + (tickWidth / 2); - tick.select('line').classed('ml-tick-emphasis', true); + function removeLabelOverlap(startTs, tickInterval, earliest, latest) { + // Put emphasis on all tick lines, will again de-emphasize the + // ones where we remove the label in the next steps. + gAxis.selectAll('g.tick').select('line').classed('ml-tick-emphasis', true); + + function getTickData(ts, operator) { + const filteredTicks = gAxis.selectAll('g.tick').filter(d => d === ts); + + if (filteredTicks[0].length === 0) { + return false; + } + + const tick = d3.selectAll(filteredTicks[0]); + const textNode = tick.select('text').node(); + + if (textNode === null) { + switch (operator) { + case 'previous': + return getTickData(ts - tickInterval, operator); + case 'next': + return getTickData(ts + tickInterval, operator); + default: + return false; + } + } + + const tickWidth = textNode.getBBox().width; + const padding = 15; + const xTransform = d3.transform(tick.attr('transform')).translate[0]; + const xMinOffset = xTransform - (tickWidth / 2 + padding); + const xMaxOffset = xTransform + (tickWidth / 2 + padding); + + return { + operator, + tick, + ts, + xTransform, + xMinOffset, + xMaxOffset + }; } - // if the last tick label overflows the chart to the right, remove it - if (xMaxOffset > vizWidth) { - tick.select('text').remove(); + + function checkTicks(ts, operator) { + + const currentTick = ts; + const currentTickData = getTickData(ts, operator); + + if (currentTickData === false) { + return; + } + + let newTickData; + let checkAnotherTick; + + switch (operator) { + case 'previous': + newTickData = getTickData(ts - tickInterval, operator); + checkAnotherTick = newTickData.ts >= earliest; + break; + case 'next': + newTickData = getTickData(ts + tickInterval, operator); + checkAnotherTick = newTickData.ts <= latest; + break; + } + const newTick = newTickData.ts; + + if (checkAnotherTick && newTickData !== false) { + + let remove = false; + + switch (operator) { + case 'previous': + if (newTickData.xMaxOffset > currentTickData.xMinOffset) { + remove = true; + } + if (newTickData.xMinOffset < 0) { + remove = true; + } + break; + case 'next': + if (newTickData.xMinOffset < currentTickData.xMaxOffset) { + remove = true; + } + if (newTickData.xMaxOffset > vizWidth) { + remove = true; + } + break; + } + + if (remove === true) { + newTickData.tick.select('text').remove(); + newTickData.tick.select('line').classed('ml-tick-emphasis', false); + checkTicks(currentTick, operator); + } else { + checkTicks(newTick, operator); + } + + } } - }); + + checkTicks(startTs, 'previous'); + checkTicks(startTs, 'next'); + return; + } + + removeLabelOverlap(emphasisStart, interval, config.plotEarliest, config.plotLatest); } diff --git a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.test.js b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.test.js index cf28917c47a9a..57a65475acdc3 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.test.js +++ b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.test.js @@ -112,7 +112,7 @@ describe('ExplorerChart', () => { expect(+selectedInterval.getAttribute('height')).toBe(169); const xAxisTicks = wrapper.getDOMNode().querySelector('.x').querySelectorAll('.tick'); - expect([...xAxisTicks]).toHaveLength(0); + expect([...xAxisTicks]).toHaveLength(1); const yAxisTicks = wrapper.getDOMNode().querySelector('.y').querySelectorAll('.tick'); expect([...yAxisTicks]).toHaveLength(10); From a8a2c9d817e6c973ea29ae570936b13076556b36 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Tue, 11 Sep 2018 16:17:38 +0200 Subject: [PATCH 05/14] [ML] Use special tick treatment only if tooManyBuckes is false. --- .../explorer_charts/explorer_chart.js | 19 +++++++++++++++---- .../explorer_charts/explorer_chart.test.js | 2 +- .../explorer_charts_container.js | 1 + 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js index 2de685ca7359b..54414d7be3cb6 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js +++ b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js @@ -23,7 +23,7 @@ import moment from 'moment'; // because it won't work with the jest tests import { formatValue } from '../../formatters/format_value'; import { getSeverityWithLow } from '../../../common/util/anomaly_utils'; -import { drawLineChartDots, getTickValues } from '../../util/chart_utils'; +import { drawLineChartDots, getTickValues, numTicksForDateFormat } from '../../util/chart_utils'; import { TimeBuckets } from 'ui/time_buckets'; import { LoadingIndicator } from '../../components/loading_indicator/loading_indicator'; import { mlEscape } from '../../util/string_utils'; @@ -34,6 +34,7 @@ const CONTENT_WRAPPER_HEIGHT = 215; export class ExplorerChart extends React.Component { static propTypes = { + tooManyBuckets: PropTypes.bool, seriesConfig: PropTypes.object, mlSelectSeverityService: PropTypes.object.isRequired } @@ -48,6 +49,7 @@ export class ExplorerChart extends React.Component { renderChart() { const { + tooManyBuckets, mlSelectSeverityService } = this.props; @@ -187,9 +189,17 @@ export class ExplorerChart extends React.Component { .innerTickSize(-chartHeight) .outerTickSize(0) .tickPadding(10) - .tickValues(tickValues) .tickFormat(d => moment(d).format(xAxisTickFormat)); + // With tooManyBuckets the chart would end up with no x-axis labels + // because the ticks are based on the span of the emphasis section, + // but that one spans the whole chart width with tooManyBuckets. + if (tooManyBuckets === false) { + xAxis.tickValues(tickValues); + } else { + xAxis.ticks(numTicksForDateFormat(vizWidth, xAxisTickFormat)); + } + const yAxis = d3.svg.axis().scale(lineChartYScale) .orient('left') .innerTickSize(0) @@ -317,8 +327,9 @@ export class ExplorerChart extends React.Component { return; } - removeLabelOverlap(emphasisStart, interval, config.plotEarliest, config.plotLatest); - + if (tooManyBuckets === false) { + removeLabelOverlap(emphasisStart, interval, config.plotEarliest, config.plotLatest); + } } function drawLineChartHighlightedSpan() { diff --git a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.test.js b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.test.js index 57a65475acdc3..cf28917c47a9a 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.test.js +++ b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.test.js @@ -112,7 +112,7 @@ describe('ExplorerChart', () => { expect(+selectedInterval.getAttribute('height')).toBe(169); const xAxisTicks = wrapper.getDOMNode().querySelector('.x').querySelectorAll('.tick'); - expect([...xAxisTicks]).toHaveLength(1); + expect([...xAxisTicks]).toHaveLength(0); const yAxisTicks = wrapper.getDOMNode().querySelector('.y').querySelectorAll('.tick'); expect([...yAxisTicks]).toHaveLength(10); diff --git a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_charts_container.js b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_charts_container.js index b4ec174a69907..c029d7db16aaf 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_charts_container.js +++ b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_charts_container.js @@ -65,6 +65,7 @@ export function ExplorerChartsContainer({ From 11fe5726dce2ef3fa05fc628d184954892a82d0f Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Tue, 11 Sep 2018 16:35:05 +0200 Subject: [PATCH 06/14] [ML] Change style of selected-interval to resemble selected swimlane cell. --- .../public/explorer/explorer_charts/explorer_chart.js | 10 ++++++---- .../explorer/explorer_charts/explorer_chart.test.js | 4 ++-- .../explorer_charts/styles/explorer_chart.less | 5 ++++- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js index 54414d7be3cb6..40ad6fc8b2b98 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js +++ b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js @@ -342,10 +342,12 @@ export class ExplorerChart extends React.Component { lineChartGroup.append('rect') .attr('class', 'selected-interval') - .attr('x', lineChartXScale(new Date(rectStart))) - .attr('y', 1) - .attr('width', rectWidth) - .attr('height', chartHeight - 1); + .attr('x', lineChartXScale(new Date(rectStart)) + 2) + .attr('y', 2) + .attr('rx', 3) + .attr('ry', 3) + .attr('width', rectWidth - 4) + .attr('height', chartHeight - 4); } function drawLineChartPaths(data) { diff --git a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.test.js b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.test.js index cf28917c47a9a..fbf4d97a89bce 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.test.js +++ b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.test.js @@ -108,8 +108,8 @@ describe('ExplorerChart', () => { const selectedInterval = rects[1]; expect(selectedInterval.getAttribute('class')).toBe('selected-interval'); - expect(+selectedInterval.getAttribute('y')).toBe(1); - expect(+selectedInterval.getAttribute('height')).toBe(169); + expect(+selectedInterval.getAttribute('y')).toBe(2); + expect(+selectedInterval.getAttribute('height')).toBe(166); const xAxisTicks = wrapper.getDOMNode().querySelector('.x').querySelectorAll('.tick'); expect([...xAxisTicks]).toHaveLength(0); diff --git a/x-pack/plugins/ml/public/explorer/explorer_charts/styles/explorer_chart.less b/x-pack/plugins/ml/public/explorer/explorer_charts/styles/explorer_chart.less index fc601b9e97846..de8e351685f7e 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_charts/styles/explorer_chart.less +++ b/x-pack/plugins/ml/public/explorer/explorer_charts/styles/explorer_chart.less @@ -14,7 +14,10 @@ ml-explorer-chart, } rect.selected-interval { - fill: rgba(200, 200, 200, 0.25); + fill: rgba(200, 200, 200, 0.1); + stroke: #6b6b6b; + stroke-width: 2px; + stroke-opacity: 0.8; } rect.scheduled-event-marker { From aa48d6b7d17a4f3824f33698506fa0bfe8a441c8 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Tue, 11 Sep 2018 18:08:14 +0200 Subject: [PATCH 07/14] [ML] Code cleanup. --- .../explorer_charts/explorer_chart.js | 141 ++++++++---------- 1 file changed, 61 insertions(+), 80 deletions(-) diff --git a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js index 40ad6fc8b2b98..614e5b640e7fc 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js +++ b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js @@ -32,6 +32,11 @@ import { mlChartTooltipService } from '../../components/chart_tooltip/chart_tool const CONTENT_WRAPPER_HEIGHT = 215; +const TICK_DIRECTION = { + NEXT: 'next', + PREVIOUS: 'previous' +}; + export class ExplorerChart extends React.Component { static propTypes = { tooManyBuckets: PropTypes.bool, @@ -222,113 +227,89 @@ export class ExplorerChart extends React.Component { .call(yAxis); // remove overlapping labels - function removeLabelOverlap(startTs, tickInterval, earliest, latest) { + function removeLabelOverlap(axis, startTs, tickInterval) { // Put emphasis on all tick lines, will again de-emphasize the // ones where we remove the label in the next steps. - gAxis.selectAll('g.tick').select('line').classed('ml-tick-emphasis', true); + axis.selectAll('g.tick').select('line').classed('ml-tick-emphasis', true); + + function getNeighourTickFactory(operator) { + return function (ts) { + switch (operator) { + case TICK_DIRECTION.PREVIOUS: + return ts - tickInterval; + case TICK_DIRECTION.NEXT: + return ts + tickInterval; + } + }; + } - function getTickData(ts, operator) { - const filteredTicks = gAxis.selectAll('g.tick').filter(d => d === ts); + function getTickDataFactory(operator) { + const getNeighourTick = getNeighourTickFactory(operator); + const fn = function (ts) { + const filteredTicks = axis.selectAll('g.tick').filter(d => d === ts); - if (filteredTicks[0].length === 0) { - return false; - } + if (filteredTicks[0].length === 0) { + return false; + } - const tick = d3.selectAll(filteredTicks[0]); - const textNode = tick.select('text').node(); + const tick = d3.selectAll(filteredTicks[0]); + const textNode = tick.select('text').node(); - if (textNode === null) { - switch (operator) { - case 'previous': - return getTickData(ts - tickInterval, operator); - case 'next': - return getTickData(ts + tickInterval, operator); - default: - return false; + if (textNode === null) { + return fn(getNeighourTick(ts)); } - } - const tickWidth = textNode.getBBox().width; - const padding = 15; - const xTransform = d3.transform(tick.attr('transform')).translate[0]; - const xMinOffset = xTransform - (tickWidth / 2 + padding); - const xMaxOffset = xTransform + (tickWidth / 2 + padding); - - return { - operator, - tick, - ts, - xTransform, - xMinOffset, - xMaxOffset + const tickWidth = textNode.getBBox().width; + const padding = 15; + const xTransform = d3.transform(tick.attr('transform')).translate[0]; + const xMinOffset = xTransform - (tickWidth / 2 + padding); + const xMaxOffset = xTransform + (tickWidth / 2 + padding); + + return { + tick, + ts, + xMinOffset, + xMaxOffset + }; }; + return fn; } function checkTicks(ts, operator) { - - const currentTick = ts; - const currentTickData = getTickData(ts, operator); + const getTickData = getTickDataFactory(operator); + const currentTickData = getTickData(ts); if (currentTickData === false) { return; } - let newTickData; - let checkAnotherTick; - - switch (operator) { - case 'previous': - newTickData = getTickData(ts - tickInterval, operator); - checkAnotherTick = newTickData.ts >= earliest; - break; - case 'next': - newTickData = getTickData(ts + tickInterval, operator); - checkAnotherTick = newTickData.ts <= latest; - break; - } - const newTick = newTickData.ts; - - if (checkAnotherTick && newTickData !== false) { - - let remove = false; - - switch (operator) { - case 'previous': - if (newTickData.xMaxOffset > currentTickData.xMinOffset) { - remove = true; - } - if (newTickData.xMinOffset < 0) { - remove = true; - } - break; - case 'next': - if (newTickData.xMinOffset < currentTickData.xMaxOffset) { - remove = true; - } - if (newTickData.xMaxOffset > vizWidth) { - remove = true; - } - break; - } - - if (remove === true) { + const getNeighourTick = getNeighourTickFactory(operator); + const newTickData = getTickData(getNeighourTick(ts)); + + if ( + newTickData !== false + ) { + if ( + newTickData.xMinOffset < 0 || + newTickData.xMaxOffset > vizWidth || + (newTickData.xMaxOffset > currentTickData.xMinOffset && operator === TICK_DIRECTION.PREVIOUS) || + (newTickData.xMinOffset < currentTickData.xMaxOffset && operator === TICK_DIRECTION.NEXT) + ) { newTickData.tick.select('text').remove(); newTickData.tick.select('line').classed('ml-tick-emphasis', false); - checkTicks(currentTick, operator); + checkTicks(currentTickData.ts, operator); } else { - checkTicks(newTick, operator); + checkTicks(newTickData.ts, operator); } - } } - checkTicks(startTs, 'previous'); - checkTicks(startTs, 'next'); - return; + checkTicks(startTs, TICK_DIRECTION.PREVIOUS); + checkTicks(startTs, TICK_DIRECTION.NEXT); } if (tooManyBuckets === false) { - removeLabelOverlap(emphasisStart, interval, config.plotEarliest, config.plotLatest); + removeLabelOverlap(gAxis, emphasisStart, interval); } } From 64c4ded11214bb14bb9afadecbf22707080121d0 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Tue, 11 Sep 2018 18:28:49 +0200 Subject: [PATCH 08/14] [ML] Added tests for getTickValues. --- .../ml/public/util/chart_utils.test.js | 68 ++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/ml/public/util/chart_utils.test.js b/x-pack/plugins/ml/public/util/chart_utils.test.js index 5bec58b9be0c9..b4fdfdfcae819 100644 --- a/x-pack/plugins/ml/public/util/chart_utils.test.js +++ b/x-pack/plugins/ml/public/util/chart_utils.test.js @@ -40,7 +40,10 @@ jest.mock('ui/timefilter/lib/parse_querystring', import moment from 'moment'; import { timefilter } from 'ui/timefilter'; -import { getExploreSeriesLink } from './chart_utils'; +import { + getExploreSeriesLink, + getTickValues +} from './chart_utils'; timefilter.enableTimeRangeSelector(); timefilter.enableAutoRefreshSelector(); @@ -61,3 +64,66 @@ describe('getExploreSeriesLink', () => { expect(link).toBe(expectedLink); }); }); + +describe('getTickValues', () => { + test('farequote sample data', () => { + const tickValues = getTickValues(1486656000000, 14400000, 1486606500000, 1486719900000); + + expect(tickValues).toEqual([ + 1486612800000, + 1486627200000, + 1486641600000, + 1486656000000, + 1486670400000, + 1486684800000, + 1486699200000, + 1486713600000 + ]); + }); + + test('filebeat sample data', () => { + const tickValues = getTickValues(1486080000000, 14400000, 1485860400000, 1486314000000); + expect(tickValues).toEqual([ + 1485864000000, + 1485878400000, + 1485892800000, + 1485907200000, + 1485921600000, + 1485936000000, + 1485950400000, + 1485964800000, + 1485979200000, + 1485993600000, + 1486008000000, + 1486022400000, + 1486036800000, + 1486051200000, + 1486065600000, + 1486080000000, + 1486094400000, + 1486108800000, + 1486123200000, + 1486137600000, + 1486152000000, + 1486166400000, + 1486180800000, + 1486195200000, + 1486209600000, + 1486224000000, + 1486238400000, + 1486252800000, + 1486267200000, + 1486281600000, + 1486296000000, + 1486310400000 + ]); + }); + + test('gallery sample data', () => { + const tickValues = getTickValues(1518652800000, 604800000, 1518274800000, 1519635600000); + expect(tickValues).toEqual([ + 1518652800000, + 1519257600000 + ]); + }); +}); From 592a9752578614da4d98988c7e97bbf8f905d288 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Tue, 11 Sep 2018 18:33:23 +0200 Subject: [PATCH 09/14] [ML] Move removeLabelOverlap to chart_utils. --- .../explorer_charts/explorer_chart.js | 96 ++----------------- x-pack/plugins/ml/public/util/chart_utils.js | 86 +++++++++++++++++ 2 files changed, 93 insertions(+), 89 deletions(-) diff --git a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js index 614e5b640e7fc..ed60871664e54 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js +++ b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js @@ -23,7 +23,12 @@ import moment from 'moment'; // because it won't work with the jest tests import { formatValue } from '../../formatters/format_value'; import { getSeverityWithLow } from '../../../common/util/anomaly_utils'; -import { drawLineChartDots, getTickValues, numTicksForDateFormat } from '../../util/chart_utils'; +import { + drawLineChartDots, + getTickValues, + numTicksForDateFormat, + removeLabelOverlap +} from '../../util/chart_utils'; import { TimeBuckets } from 'ui/time_buckets'; import { LoadingIndicator } from '../../components/loading_indicator/loading_indicator'; import { mlEscape } from '../../util/string_utils'; @@ -32,11 +37,6 @@ import { mlChartTooltipService } from '../../components/chart_tooltip/chart_tool const CONTENT_WRAPPER_HEIGHT = 215; -const TICK_DIRECTION = { - NEXT: 'next', - PREVIOUS: 'previous' -}; - export class ExplorerChart extends React.Component { static propTypes = { tooManyBuckets: PropTypes.bool, @@ -226,90 +226,8 @@ export class ExplorerChart extends React.Component { .attr('class', 'y axis') .call(yAxis); - // remove overlapping labels - function removeLabelOverlap(axis, startTs, tickInterval) { - // Put emphasis on all tick lines, will again de-emphasize the - // ones where we remove the label in the next steps. - axis.selectAll('g.tick').select('line').classed('ml-tick-emphasis', true); - - function getNeighourTickFactory(operator) { - return function (ts) { - switch (operator) { - case TICK_DIRECTION.PREVIOUS: - return ts - tickInterval; - case TICK_DIRECTION.NEXT: - return ts + tickInterval; - } - }; - } - - function getTickDataFactory(operator) { - const getNeighourTick = getNeighourTickFactory(operator); - const fn = function (ts) { - const filteredTicks = axis.selectAll('g.tick').filter(d => d === ts); - - if (filteredTicks[0].length === 0) { - return false; - } - - const tick = d3.selectAll(filteredTicks[0]); - const textNode = tick.select('text').node(); - - if (textNode === null) { - return fn(getNeighourTick(ts)); - } - - const tickWidth = textNode.getBBox().width; - const padding = 15; - const xTransform = d3.transform(tick.attr('transform')).translate[0]; - const xMinOffset = xTransform - (tickWidth / 2 + padding); - const xMaxOffset = xTransform + (tickWidth / 2 + padding); - - return { - tick, - ts, - xMinOffset, - xMaxOffset - }; - }; - return fn; - } - - function checkTicks(ts, operator) { - const getTickData = getTickDataFactory(operator); - const currentTickData = getTickData(ts); - - if (currentTickData === false) { - return; - } - - const getNeighourTick = getNeighourTickFactory(operator); - const newTickData = getTickData(getNeighourTick(ts)); - - if ( - newTickData !== false - ) { - if ( - newTickData.xMinOffset < 0 || - newTickData.xMaxOffset > vizWidth || - (newTickData.xMaxOffset > currentTickData.xMinOffset && operator === TICK_DIRECTION.PREVIOUS) || - (newTickData.xMinOffset < currentTickData.xMaxOffset && operator === TICK_DIRECTION.NEXT) - ) { - newTickData.tick.select('text').remove(); - newTickData.tick.select('line').classed('ml-tick-emphasis', false); - checkTicks(currentTickData.ts, operator); - } else { - checkTicks(newTickData.ts, operator); - } - } - } - - checkTicks(startTs, TICK_DIRECTION.PREVIOUS); - checkTicks(startTs, TICK_DIRECTION.NEXT); - } - if (tooManyBuckets === false) { - removeLabelOverlap(gAxis, emphasisStart, interval); + removeLabelOverlap(gAxis, emphasisStart, interval, vizWidth); } } diff --git a/x-pack/plugins/ml/public/util/chart_utils.js b/x-pack/plugins/ml/public/util/chart_utils.js index 4422d3620418e..a95fa6a373b8b 100644 --- a/x-pack/plugins/ml/public/util/chart_utils.js +++ b/x-pack/plugins/ml/public/util/chart_utils.js @@ -222,3 +222,89 @@ export function getTickValues(startTs, tickInterval, earliest, latest) { return tickValues; } + +const TICK_DIRECTION = { + NEXT: 'next', + PREVIOUS: 'previous' +}; + +export function removeLabelOverlap(axis, startTs, tickInterval, width) { + // Put emphasis on all tick lines, will again de-emphasize the + // ones where we remove the label in the next steps. + axis.selectAll('g.tick').select('line').classed('ml-tick-emphasis', true); + + function getNeighourTickFactory(operator) { + return function (ts) { + switch (operator) { + case TICK_DIRECTION.PREVIOUS: + return ts - tickInterval; + case TICK_DIRECTION.NEXT: + return ts + tickInterval; + } + }; + } + + function getTickDataFactory(operator) { + const getNeighourTick = getNeighourTickFactory(operator); + const fn = function (ts) { + const filteredTicks = axis.selectAll('g.tick').filter(d => d === ts); + + if (filteredTicks[0].length === 0) { + return false; + } + + const tick = d3.selectAll(filteredTicks[0]); + const textNode = tick.select('text').node(); + + if (textNode === null) { + return fn(getNeighourTick(ts)); + } + + const tickWidth = textNode.getBBox().width; + const padding = 15; + const xTransform = d3.transform(tick.attr('transform')).translate[0]; + const xMinOffset = xTransform - (tickWidth / 2 + padding); + const xMaxOffset = xTransform + (tickWidth / 2 + padding); + + return { + tick, + ts, + xMinOffset, + xMaxOffset + }; + }; + return fn; + } + + function checkTicks(ts, operator) { + const getTickData = getTickDataFactory(operator); + const currentTickData = getTickData(ts); + + if (currentTickData === false) { + return; + } + + const getNeighourTick = getNeighourTickFactory(operator); + const newTickData = getTickData(getNeighourTick(ts)); + + if ( + newTickData !== false + ) { + if ( + newTickData.xMinOffset < 0 || + newTickData.xMaxOffset > width || + (newTickData.xMaxOffset > currentTickData.xMinOffset && operator === TICK_DIRECTION.PREVIOUS) || + (newTickData.xMinOffset < currentTickData.xMaxOffset && operator === TICK_DIRECTION.NEXT) + ) { + newTickData.tick.select('text').remove(); + newTickData.tick.select('line').classed('ml-tick-emphasis', false); + checkTicks(currentTickData.ts, operator); + } else { + checkTicks(newTickData.ts, operator); + } + } + } + + checkTicks(startTs, TICK_DIRECTION.PREVIOUS); + checkTicks(startTs, TICK_DIRECTION.NEXT); +} From 51599f6e73ae0fd63a0b8b9f2b83c98bade56323 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Wed, 12 Sep 2018 10:56:50 +0200 Subject: [PATCH 10/14] [ML] jest test coverage for removeLabelOverlap. --- x-pack/plugins/ml/public/util/chart_utils.js | 12 +- .../ml/public/util/chart_utils.test.js | 116 +++++++++++++++++- 2 files changed, 125 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/ml/public/util/chart_utils.js b/x-pack/plugins/ml/public/util/chart_utils.js index a95fa6a373b8b..367ba71076994 100644 --- a/x-pack/plugins/ml/public/util/chart_utils.js +++ b/x-pack/plugins/ml/public/util/chart_utils.js @@ -228,6 +228,10 @@ const TICK_DIRECTION = { PREVIOUS: 'previous' }; +// This removes overlapping x-axis labels by starting off from a specific label +// that is required/wanted to show up. The code then traverses to both sides along the axis +// and decides which labels to keep or remove. All vertical tick lines will be kept visible, +// but those which still have their text label will be emphasized using the ml-tick-emphasis class. export function removeLabelOverlap(axis, startTs, tickInterval, width) { // Put emphasis on all tick lines, will again de-emphasize the // ones where we remove the label in the next steps. @@ -247,7 +251,7 @@ export function removeLabelOverlap(axis, startTs, tickInterval, width) { function getTickDataFactory(operator) { const getNeighourTick = getNeighourTickFactory(operator); const fn = function (ts) { - const filteredTicks = axis.selectAll('g.tick').filter(d => d === ts); + const filteredTicks = axis.selectAll('.tick').filter(d => d === ts); if (filteredTicks[0].length === 0) { return false; @@ -262,7 +266,11 @@ export function removeLabelOverlap(axis, startTs, tickInterval, width) { const tickWidth = textNode.getBBox().width; const padding = 15; - const xTransform = d3.transform(tick.attr('transform')).translate[0]; + // To get xTransform it would be nicer to use d3.transform, but that doesn't play well with JSDOM. + // So this uses a regex variant because we definitely want test coverage for the label removal. + // Once JSDOM supports SVGAnimatedTransformList we can use the simpler version. + // const xTransform = d3.transform(tick.attr('transform')).translate[0]; + const xTransform = +(/translate\(\s*([^\s,)]+)[ ,]([^\s,)]+)/.exec(tick.attr('transform'))[1]); const xMinOffset = xTransform - (tickWidth / 2 + padding); const xMaxOffset = xTransform + (tickWidth / 2 + padding); diff --git a/x-pack/plugins/ml/public/util/chart_utils.test.js b/x-pack/plugins/ml/public/util/chart_utils.test.js index b4fdfdfcae819..fec20f5e11188 100644 --- a/x-pack/plugins/ml/public/util/chart_utils.test.js +++ b/x-pack/plugins/ml/public/util/chart_utils.test.js @@ -37,12 +37,17 @@ jest.mock('ui/timefilter/lib/parse_querystring', }, }), { virtual: true }); +import d3 from 'd3'; import moment from 'moment'; +import { mount } from 'enzyme'; +import React from 'react'; + import { timefilter } from 'ui/timefilter'; import { getExploreSeriesLink, - getTickValues + getTickValues, + removeLabelOverlap } from './chart_utils'; timefilter.enableTimeRangeSelector(); @@ -127,3 +132,112 @@ describe('getTickValues', () => { ]); }); }); + +describe('removeLabelOverlap', () => { + const originalGetBBox = SVGElement.prototype.getBBox; + + // This resembles how ExplorerChart renders its x axis. + // We set up this boilerplate so we can then run removeLabelOverlap() + // on some "real" structure. + function axisSetup({ + interval, + plotEarliest, + plotLatest, + startTs, + xAxisTickFormat + }) { + const wrapper = mount(
); + const node = wrapper.getDOMNode(); + + const chartHeight = 170; + const margin = { top: 10, right: 0, bottom: 30, left: 60 }; + const svgWidth = 500; + const svgHeight = chartHeight + margin.top + margin.bottom; + const vizWidth = 500; + + const chartElement = d3.select(node); + + const lineChartXScale = d3.time.scale() + .range([0, vizWidth]) + .domain([plotEarliest, plotLatest]); + + const xAxis = d3.svg.axis().scale(lineChartXScale) + .orient('bottom') + .innerTickSize(-chartHeight) + .outerTickSize(0) + .tickPadding(10) + .tickFormat(d => moment(d).format(xAxisTickFormat)); + + const tickValues = getTickValues(startTs, interval, plotEarliest, plotLatest); + xAxis.tickValues(tickValues); + + const svg = chartElement.append('svg') + .attr('width', svgWidth) + .attr('height', svgHeight); + + const axes = svg.append('g'); + + const gAxis = axes.append('g') + .attr('class', 'x axis') + .attr('transform', 'translate(0,' + chartHeight + ')') + .call(xAxis); + + return { + gAxis, + node, + vizWidth + }; + } + + test('farequote sample data', () => { + const mockedGetBBox = { width: 27.21875 }; + SVGElement.prototype.getBBox = () => mockedGetBBox; + + const startTs = 1486656000000; + const interval = 14400000; + + const { gAxis, node, vizWidth } = axisSetup({ + interval, + plotEarliest: 1486606500000, + plotLatest: 1486719900000, + startTs, + xAxisTickFormat: 'HH:mm' + }); + + expect(node.getElementsByTagName('text')).toHaveLength(8); + + removeLabelOverlap(gAxis, startTs, interval, vizWidth); + + // at the vizWidth of 500, the most left and right tick label + // will get removed because it overflows the chart area + expect(node.getElementsByTagName('text')).toHaveLength(6); + + SVGElement.prototype.getBBox = originalGetBBox; + }); + + test('filebeat sample data', () => { + const mockedGetBBox = { width: 85.640625 }; + SVGElement.prototype.getBBox = () => mockedGetBBox; + + const startTs = 1486080000000; + const interval = 14400000; + + const { gAxis, node, vizWidth } = axisSetup({ + interval, + plotEarliest: 1485860400000, + plotLatest: 1486314000000, + startTs, + xAxisTickFormat: 'YYYY-MM-DD HH:mm' + }); + + expect(node.getElementsByTagName('text')).toHaveLength(32); + + removeLabelOverlap(gAxis, startTs, interval, vizWidth); + + // In this case labels get reduced significantly because of the wider + // labels (full dates + time) and the narrow interval. + expect(node.getElementsByTagName('text')).toHaveLength(3); + + SVGElement.prototype.getBBox = originalGetBBox; + }); +}); From 64114b628dd5d3412742038dfe2b25a11b742512 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Wed, 12 Sep 2018 11:35:23 +0200 Subject: [PATCH 11/14] [ML] Tweaked regex. --- x-pack/plugins/ml/public/util/chart_utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/ml/public/util/chart_utils.js b/x-pack/plugins/ml/public/util/chart_utils.js index 367ba71076994..66517d69c2da6 100644 --- a/x-pack/plugins/ml/public/util/chart_utils.js +++ b/x-pack/plugins/ml/public/util/chart_utils.js @@ -270,7 +270,7 @@ export function removeLabelOverlap(axis, startTs, tickInterval, width) { // So this uses a regex variant because we definitely want test coverage for the label removal. // Once JSDOM supports SVGAnimatedTransformList we can use the simpler version. // const xTransform = d3.transform(tick.attr('transform')).translate[0]; - const xTransform = +(/translate\(\s*([^\s,)]+)[ ,]([^\s,)]+)/.exec(tick.attr('transform'))[1]); + const xTransform = +(/translate\(\s*([^\s,)]+)[ ,]([^\s,)]+)\)/.exec(tick.attr('transform'))[1]); const xMinOffset = xTransform - (tickWidth / 2 + padding); const xMaxOffset = xTransform + (tickWidth / 2 + padding); From 49c1d1b052c836e4fcc42ea07141f918e21dbb2a Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Wed, 12 Sep 2018 11:38:18 +0200 Subject: [PATCH 12/14] [ML] Code tweaks including review feedback. --- x-pack/plugins/ml/public/util/chart_utils.js | 22 +++++++++----------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/ml/public/util/chart_utils.js b/x-pack/plugins/ml/public/util/chart_utils.js index 66517d69c2da6..5a7e682da720d 100644 --- a/x-pack/plugins/ml/public/util/chart_utils.js +++ b/x-pack/plugins/ml/public/util/chart_utils.js @@ -188,6 +188,11 @@ export function numTicksForDateFormat(axisWidth, dateFormat) { return axisWidth / (1.75 * tickWidth); } +const TICK_DIRECTION = { + NEXT: 'next', + PREVIOUS: 'previous' +}; + // Based on a fixed starting timestamp and an interval, get tick values within // the bounds of earliest and latest. This is useful for the Anomaly Explorer Charts // to align axis ticks with the gray area resembling the swimlane cell selection. @@ -199,11 +204,11 @@ export function getTickValues(startTs, tickInterval, earliest, latest) { let addAnotherTick; switch (operator) { - case 'previous': + case TICK_DIRECTION.PREVIOUS: newTick = ts - tickInterval; addAnotherTick = newTick >= earliest; break; - case 'next': + case TICK_DIRECTION.NEXT: newTick = ts + tickInterval; addAnotherTick = newTick <= latest; break; @@ -215,19 +220,14 @@ export function getTickValues(startTs, tickInterval, earliest, latest) { } } - addTicks(startTs, 'previous'); - addTicks(startTs, 'next'); + addTicks(startTs, TICK_DIRECTION.PREVIOUS); + addTicks(startTs, TICK_DIRECTION.NEXT); tickValues.sort(); return tickValues; } -const TICK_DIRECTION = { - NEXT: 'next', - PREVIOUS: 'previous' -}; - // This removes overlapping x-axis labels by starting off from a specific label // that is required/wanted to show up. The code then traverses to both sides along the axis // and decides which labels to keep or remove. All vertical tick lines will be kept visible, @@ -295,9 +295,7 @@ export function removeLabelOverlap(axis, startTs, tickInterval, width) { const getNeighourTick = getNeighourTickFactory(operator); const newTickData = getTickData(getNeighourTick(ts)); - if ( - newTickData !== false - ) { + if (newTickData !== false) { if ( newTickData.xMinOffset < 0 || newTickData.xMaxOffset > width || From bf3e296cd666f6e09e7584c9222a463a058194de Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Wed, 12 Sep 2018 11:51:45 +0200 Subject: [PATCH 13/14] [ML] Comment wording improvement. --- .../ml/public/explorer/explorer_charts/explorer_chart.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js index ed60871664e54..8687579d9349e 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js +++ b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart.js @@ -198,7 +198,7 @@ export class ExplorerChart extends React.Component { // With tooManyBuckets the chart would end up with no x-axis labels // because the ticks are based on the span of the emphasis section, - // but that one spans the whole chart width with tooManyBuckets. + // and the highlighted area spans the whole chart. if (tooManyBuckets === false) { xAxis.tickValues(tickValues); } else { From 070d1c61234d9e7bd0196027572cac3039b2b96c Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Wed, 12 Sep 2018 12:24:55 +0200 Subject: [PATCH 14/14] [ML] Some renaming after review feedback. --- x-pack/plugins/ml/public/util/chart_utils.js | 24 +++++++++---------- .../ml/public/util/chart_utils.test.js | 16 ++++++------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/x-pack/plugins/ml/public/util/chart_utils.js b/x-pack/plugins/ml/public/util/chart_utils.js index 5a7e682da720d..158db91ebdacf 100644 --- a/x-pack/plugins/ml/public/util/chart_utils.js +++ b/x-pack/plugins/ml/public/util/chart_utils.js @@ -196,8 +196,8 @@ const TICK_DIRECTION = { // Based on a fixed starting timestamp and an interval, get tick values within // the bounds of earliest and latest. This is useful for the Anomaly Explorer Charts // to align axis ticks with the gray area resembling the swimlane cell selection. -export function getTickValues(startTs, tickInterval, earliest, latest) { - const tickValues = [startTs]; +export function getTickValues(startTimeMs, tickInterval, earliest, latest) { + const tickValues = [startTimeMs]; function addTicks(ts, operator) { let newTick; @@ -220,8 +220,8 @@ export function getTickValues(startTs, tickInterval, earliest, latest) { } } - addTicks(startTs, TICK_DIRECTION.PREVIOUS); - addTicks(startTs, TICK_DIRECTION.NEXT); + addTicks(startTimeMs, TICK_DIRECTION.PREVIOUS); + addTicks(startTimeMs, TICK_DIRECTION.NEXT); tickValues.sort(); @@ -232,12 +232,12 @@ export function getTickValues(startTs, tickInterval, earliest, latest) { // that is required/wanted to show up. The code then traverses to both sides along the axis // and decides which labels to keep or remove. All vertical tick lines will be kept visible, // but those which still have their text label will be emphasized using the ml-tick-emphasis class. -export function removeLabelOverlap(axis, startTs, tickInterval, width) { +export function removeLabelOverlap(axis, startTimeMs, tickInterval, width) { // Put emphasis on all tick lines, will again de-emphasize the // ones where we remove the label in the next steps. axis.selectAll('g.tick').select('line').classed('ml-tick-emphasis', true); - function getNeighourTickFactory(operator) { + function getNeighborTickFactory(operator) { return function (ts) { switch (operator) { case TICK_DIRECTION.PREVIOUS: @@ -249,7 +249,7 @@ export function removeLabelOverlap(axis, startTs, tickInterval, width) { } function getTickDataFactory(operator) { - const getNeighourTick = getNeighourTickFactory(operator); + const getNeighborTick = getNeighborTickFactory(operator); const fn = function (ts) { const filteredTicks = axis.selectAll('.tick').filter(d => d === ts); @@ -261,7 +261,7 @@ export function removeLabelOverlap(axis, startTs, tickInterval, width) { const textNode = tick.select('text').node(); if (textNode === null) { - return fn(getNeighourTick(ts)); + return fn(getNeighborTick(ts)); } const tickWidth = textNode.getBBox().width; @@ -292,8 +292,8 @@ export function removeLabelOverlap(axis, startTs, tickInterval, width) { return; } - const getNeighourTick = getNeighourTickFactory(operator); - const newTickData = getTickData(getNeighourTick(ts)); + const getNeighborTick = getNeighborTickFactory(operator); + const newTickData = getTickData(getNeighborTick(ts)); if (newTickData !== false) { if ( @@ -311,6 +311,6 @@ export function removeLabelOverlap(axis, startTs, tickInterval, width) { } } - checkTicks(startTs, TICK_DIRECTION.PREVIOUS); - checkTicks(startTs, TICK_DIRECTION.NEXT); + checkTicks(startTimeMs, TICK_DIRECTION.PREVIOUS); + checkTicks(startTimeMs, TICK_DIRECTION.NEXT); } diff --git a/x-pack/plugins/ml/public/util/chart_utils.test.js b/x-pack/plugins/ml/public/util/chart_utils.test.js index fec20f5e11188..d4beee8484b9b 100644 --- a/x-pack/plugins/ml/public/util/chart_utils.test.js +++ b/x-pack/plugins/ml/public/util/chart_utils.test.js @@ -143,7 +143,7 @@ describe('removeLabelOverlap', () => { interval, plotEarliest, plotLatest, - startTs, + startTimeMs, xAxisTickFormat }) { const wrapper = mount(
); @@ -168,7 +168,7 @@ describe('removeLabelOverlap', () => { .tickPadding(10) .tickFormat(d => moment(d).format(xAxisTickFormat)); - const tickValues = getTickValues(startTs, interval, plotEarliest, plotLatest); + const tickValues = getTickValues(startTimeMs, interval, plotEarliest, plotLatest); xAxis.tickValues(tickValues); const svg = chartElement.append('svg') @@ -193,20 +193,20 @@ describe('removeLabelOverlap', () => { const mockedGetBBox = { width: 27.21875 }; SVGElement.prototype.getBBox = () => mockedGetBBox; - const startTs = 1486656000000; + const startTimeMs = 1486656000000; const interval = 14400000; const { gAxis, node, vizWidth } = axisSetup({ interval, plotEarliest: 1486606500000, plotLatest: 1486719900000, - startTs, + startTimeMs, xAxisTickFormat: 'HH:mm' }); expect(node.getElementsByTagName('text')).toHaveLength(8); - removeLabelOverlap(gAxis, startTs, interval, vizWidth); + removeLabelOverlap(gAxis, startTimeMs, interval, vizWidth); // at the vizWidth of 500, the most left and right tick label // will get removed because it overflows the chart area @@ -219,20 +219,20 @@ describe('removeLabelOverlap', () => { const mockedGetBBox = { width: 85.640625 }; SVGElement.prototype.getBBox = () => mockedGetBBox; - const startTs = 1486080000000; + const startTimeMs = 1486080000000; const interval = 14400000; const { gAxis, node, vizWidth } = axisSetup({ interval, plotEarliest: 1485860400000, plotLatest: 1486314000000, - startTs, + startTimeMs, xAxisTickFormat: 'YYYY-MM-DD HH:mm' }); expect(node.getElementsByTagName('text')).toHaveLength(32); - removeLabelOverlap(gAxis, startTs, interval, vizWidth); + removeLabelOverlap(gAxis, startTimeMs, interval, vizWidth); // In this case labels get reduced significantly because of the wider // labels (full dates + time) and the narrow interval.