From 8a528dcd970a546447f91a2cd398a9a469592ff5 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Tue, 29 Nov 2016 11:31:37 -0500 Subject: [PATCH] Tagcloud fixes (#9194) * Improve robustness Tagcloud was subsceptible to race conditions, potentially yielding stale clouds. - Introduced queue to explicitly keep track of outstanding jobs and avoid clobbering of the state. - Expanded unit tests * ensure tagcloud expands to full height in reporting * remove magic numbers --- .../tagcloud/public/__tests__/tag_cloud.js | 217 +++++++++++++++--- src/core_plugins/tagcloud/public/tag_cloud.js | 148 +++++++----- .../tagcloud/public/tag_cloud.less | 2 +- .../tagcloud/public/tag_cloud_controller.js | 3 +- .../tagcloud/public/tag_cloud_vis.js | 1 + 5 files changed, 275 insertions(+), 96 deletions(-) diff --git a/src/core_plugins/tagcloud/public/__tests__/tag_cloud.js b/src/core_plugins/tagcloud/public/__tests__/tag_cloud.js index 1b78b02b9aff3..35394c6af46eb 100644 --- a/src/core_plugins/tagcloud/public/__tests__/tag_cloud.js +++ b/src/core_plugins/tagcloud/public/__tests__/tag_cloud.js @@ -1,6 +1,7 @@ import expect from 'expect.js'; import _ from 'lodash'; import TagCloud from 'plugins/tagcloud/tag_cloud'; +import d3 from 'd3'; describe('tag cloud', function () { @@ -22,11 +23,14 @@ describe('tag cloud', function () { }); - const baseTestConfig = { + const minValue = 1; + const maxValue = 9; + const midValue = (minValue + maxValue) / 2; + const baseTest = { data: [ - {text: 'foo', size: 1}, - {text: 'bar', size: 5}, - {text: 'foobar', size: 9}, + {text: 'foo', value: minValue}, + {text: 'bar', value: midValue}, + {text: 'foobar', value: maxValue}, ], options: { orientation: 'single', @@ -50,31 +54,47 @@ describe('tag cloud', function () { ] }; - const singleLayout = _.cloneDeep(baseTestConfig); - const rightAngleLayout = _.cloneDeep(baseTestConfig); - rightAngleLayout.options.orientation = 'right angled'; - const multiLayout = _.cloneDeep(baseTestConfig); - multiLayout.options.orientation = 'multiple'; - const logScale = _.cloneDeep(baseTestConfig); - logScale.options.scale = 'log'; - logScale.expected[1].fontSize = '31px'; - const sqrtScale = _.cloneDeep(baseTestConfig); - sqrtScale.options.scale = 'square root'; - sqrtScale.expected[1].fontSize = '27px'; - const biggerFont = _.cloneDeep(baseTestConfig); - biggerFont.options.minFontSize = 36; - biggerFont.options.maxFontSize = 72; - biggerFont.expected[0].fontSize = '36px'; - biggerFont.expected[1].fontSize = '54px'; - biggerFont.expected[2].fontSize = '72px'; + const singleLayoutTest = _.cloneDeep(baseTest); + + const rightAngleLayoutTest = _.cloneDeep(baseTest); + rightAngleLayoutTest.options.orientation = 'right angled'; + + const multiLayoutTest = _.cloneDeep(baseTest); + multiLayoutTest.options.orientation = 'multiple'; + + const mapWithLog = d3.scale.log(); + mapWithLog.range([baseTest.options.minFontSize, baseTest.options.maxFontSize]); + mapWithLog.domain([minValue, maxValue]); + const logScaleTest = _.cloneDeep(baseTest); + logScaleTest.options.scale = 'log'; + logScaleTest.expected[1].fontSize = Math.round(mapWithLog(midValue)) + 'px'; + + const mapWithSqrt = d3.scale.sqrt(); + mapWithSqrt.range([baseTest.options.minFontSize, baseTest.options.maxFontSize]); + mapWithSqrt.domain([minValue, maxValue]); + const sqrtScaleTest = _.cloneDeep(baseTest); + sqrtScaleTest.options.scale = 'square root'; + sqrtScaleTest.expected[1].fontSize = Math.round(mapWithSqrt(midValue)) + 'px'; + + const biggerFontTest = _.cloneDeep(baseTest); + biggerFontTest.options.minFontSize = 36; + biggerFontTest.options.maxFontSize = 72; + biggerFontTest.expected[0].fontSize = '36px'; + biggerFontTest.expected[1].fontSize = '54px'; + biggerFontTest.expected[2].fontSize = '72px'; + + const trimDataTest = _.cloneDeep(baseTest); + trimDataTest.data.splice(1, 1); + trimDataTest.expected.splice(1, 1); [ - singleLayout, - rightAngleLayout, - multiLayout, - logScale, - sqrtScale, - biggerFont + singleLayoutTest, + rightAngleLayoutTest, + multiLayoutTest, + logScaleTest, + sqrtScaleTest, + biggerFontTest, + trimDataTest ].forEach((test, index) => { it(`should position elements correctly: ${index}`, done => { @@ -91,14 +111,143 @@ describe('tag cloud', function () { }); }); - it(`should not put elements in view when container to small`, function (done) { + + it('should use the latest state before notifying (modifying options)', function (done) { + + const tagCloud = new TagCloud(domNode); + tagCloud.setData(baseTest.data); + tagCloud.setOptions(baseTest.options); + tagCloud.setOptions(logScaleTest.options); + tagCloud.on('renderComplete', function onRender() { + tagCloud.removeListener('renderComplete', onRender); + const textElements = domNode.querySelectorAll('text'); + verifyTagProperties(logScaleTest.expected, textElements); + expect(tagCloud.getStatus()).to.equal(TagCloud.STATUS.COMPLETE); + done(); + }); + + }); + + + it('should use the latest state before notifying (modifying data)', function (done) { + + const tagCloud = new TagCloud(domNode); + tagCloud.setData(baseTest.data); + tagCloud.setOptions(baseTest.options); + tagCloud.setData(trimDataTest.data); + + tagCloud.on('renderComplete', function onRender() { + tagCloud.removeListener('renderComplete', onRender); + const textElements = domNode.querySelectorAll('text'); + verifyTagProperties(trimDataTest.expected, textElements); + expect(tagCloud.getStatus()).to.equal(TagCloud.STATUS.COMPLETE); + done(); + }); + + }); + + [ + 5, + 100, + 200, + 300, + 500 + ].forEach(function (timeout, index) { + it(`should only send single renderComplete event: ${index}`, function (done) { + //TagCloud takes at least 600ms to complete (due to d3 animation) + //renderComplete should only notify at the last one + const tagCloud = new TagCloud(domNode); + tagCloud.on('renderComplete', function onRender() { + tagCloud.removeListener('renderComplete', onRender); + const textElements = domNode.querySelectorAll('text'); + verifyTagProperties(logScaleTest.expected, textElements); + expect(tagCloud.getStatus()).to.equal(TagCloud.STATUS.COMPLETE); + done(); + }); + tagCloud.setData(baseTest.data); + tagCloud.setOptions(baseTest.options); + setTimeout(() => { + tagCloud.setOptions(logScaleTest.options); + }, timeout); + }); + }); + + it('should not get multiple render-events', function (done) { + + const tagCloud = new TagCloud(domNode); + tagCloud.setData(baseTest.data); + tagCloud.setOptions(baseTest.options); + + setTimeout(() => { + //this should be overridden by later changes + tagCloud.setData(sqrtScaleTest.data); + tagCloud.setOptions(sqrtScaleTest.options); + }, 100); + + setTimeout(() => { + tagCloud.setData(logScaleTest.data); + tagCloud.setOptions(logScaleTest.options); + }, 300); + + let counter = 0; + + function onRender() { + if (counter > 0) { + throw new Error('Should not get multiple render events'); + } + counter += 1; + const textElements = domNode.querySelectorAll('text'); + verifyTagProperties(logScaleTest.expected, textElements); + expect(tagCloud.getStatus()).to.equal(TagCloud.STATUS.COMPLETE); + } + + tagCloud.on('renderComplete', onRender); + setTimeout(function () { + tagCloud.removeListener('renderComplete', onRender); + done(); + }, 1500); + + }); + + it('should show correct data when state-updates are interleaved with resize event', function (done) { + + const tagCloud = new TagCloud(domNode); + tagCloud.setData(logScaleTest.data); + tagCloud.setOptions(logScaleTest.options); + + setTimeout(() => { + domNode.style.width = '600px'; + domNode.style.height = '600px'; + + tagCloud.resize(); + + setTimeout(() => { + tagCloud.setData(baseTest.data); + tagCloud.setOptions(baseTest.options); + }, 200); + + + tagCloud.on('renderComplete', function onRender() { + tagCloud.removeListener('renderComplete', onRender); + const textElements = domNode.querySelectorAll('text'); + verifyTagProperties(baseTest.expected, textElements); + expect(tagCloud.getStatus()).to.equal(TagCloud.STATUS.COMPLETE); + done(); + }); + }, 1000); + + + }); + + + it(`should not put elements in view when container is too small`, function (done) { domNode.style.width = '1px'; domNode.style.height = '1px'; const tagCloud = new TagCloud(domNode); - tagCloud.setData(baseTestConfig.data); - tagCloud.setOptions(baseTestConfig.options); + tagCloud.setData(baseTest.data); + tagCloud.setOptions(baseTest.options); tagCloud.on('renderComplete', function onRender() { tagCloud.removeListener('renderComplete', onRender); expect(tagCloud.getStatus()).to.equal(TagCloud.STATUS.INCOMPLETE); @@ -118,8 +267,8 @@ describe('tag cloud', function () { domNode.style.height = '1px'; const tagCloud = new TagCloud(domNode); - tagCloud.setData(baseTestConfig.data); - tagCloud.setOptions(baseTestConfig.options); + tagCloud.setData(baseTest.data); + tagCloud.setOptions(baseTest.options); tagCloud.on('renderComplete', function onRender() { tagCloud.removeListener('renderComplete', onRender); expect(tagCloud.getStatus()).to.equal(TagCloud.STATUS.INCOMPLETE); @@ -138,8 +287,8 @@ describe('tag cloud', function () { it(`tags should no longer fit after making container smaller`, function (done) { const tagCloud = new TagCloud(domNode); - tagCloud.setData(baseTestConfig.data); - tagCloud.setOptions(baseTestConfig.options); + tagCloud.setData(baseTest.data); + tagCloud.setOptions(baseTest.options); tagCloud.on('renderComplete', function onRender() { tagCloud.removeListener('renderComplete', onRender); expect(tagCloud.getStatus()).to.equal(TagCloud.STATUS.COMPLETE); diff --git a/src/core_plugins/tagcloud/public/tag_cloud.js b/src/core_plugins/tagcloud/public/tag_cloud.js index 712826f6e23f4..0b8c299769a75 100644 --- a/src/core_plugins/tagcloud/public/tag_cloud.js +++ b/src/core_plugins/tagcloud/public/tag_cloud.js @@ -15,9 +15,9 @@ const ORIENTATIONS = { } }; const D3_SCALING_FUNCTIONS = { - 'linear': d3.scale.linear(), - 'log': d3.scale.log(), - 'square root': d3.scale.sqrt() + 'linear': () => d3.scale.linear(), + 'log': () => d3.scale.log(), + 'square root': () => d3.scale.sqrt() }; @@ -27,31 +27,43 @@ class TagCloud extends EventEmitter { super(); + //DOM this._element = domNode; this._d3SvgContainer = d3.select(this._element).append('svg'); this._svgGroup = this._d3SvgContainer.append('g'); this._size = [1, 1]; this.resize(); + //SETTING (non-configurable) this._fontFamily = 'Impact'; this._fontStyle = 'normal'; this._fontWeight = 'normal'; + this._spiral = 'archimedean';//layout shape + this._timeInterval = 1000;//time allowed for layout algorithm + this._padding = 5; + + //OPTIONS this._orientation = 'single'; this._minFontSize = 10; this._maxFontSize = 36; this._textScale = 'linear'; - this._spiral = 'archimedean';//layout shape - this._timeInterval = 1000;//time allowed for layout algorithm - this._padding = 5; + this._optionsAsString = null; - } + //DATA + this._words = null; + //UTIL + this._handle = null; + this._queue = []; + this._allInViewBox = false; + this._inFlight = false; + + } setOptions(options) { if (JSON.stringify(options) === this._optionsAsString) { return; } - this._optionsAsString = JSON.stringify(options); this._orientation = options.orientation; this._minFontSize = Math.min(options.minFontSize, options.maxFontSize); @@ -62,24 +74,19 @@ class TagCloud extends EventEmitter { resize() { - const newWidth = this._element.offsetWidth; const newHeight = this._element.offsetHeight; - if (newWidth < 1 || newHeight < 1) { return; } - if (newWidth === this._size[0] && newHeight === this._size[1]) { return; } const wasInside = this._size[0] >= this._cloudWidth && this._size[1] >= this._cloudHeight; const willBeInside = this._cloudWidth <= newWidth && this._cloudHeight <= newHeight; - this._size[0] = newWidth; this._size[1] = newHeight; - if (wasInside && willBeInside && this._allInViewBox) { this._invalidate(true); } else { @@ -90,13 +97,11 @@ class TagCloud extends EventEmitter { setData(data) { this._words = data.map(toWordTag); - this._makeTextSizeMapper(); this._invalidate(false); } - destroy() { - clearTimeout(this._timeoutHandle); + clearTimeout(this._handle); this._element.innerHTML = ''; } @@ -111,24 +116,35 @@ class TagCloud extends EventEmitter { this._svgGroup.attr('height', this._size[1]); } - _washWords() { - if (!this._words) { + _processQueue() { + + if (!this._queue.length) { + this.emit('renderComplete'); + return; + } + + if (this._inFlight) { return; } - //the tagCloudLayoutGenerator clobbers the word-object with metadata about positioning. - //This can causes corrupt states in the layout-generator - //where words get collapsed to the same location and do not reposition correctly. - //=> we recreate an empty word object without the metadata - this._words = this._words.map(toWordTag); - this._makeTextSizeMapper(); + const job = this._queue.pop(); + this._inFlight = true; + this._onLayoutEnd(job); + + } - _onLayoutEnd() { + _onLayoutEnd(job) { + + if (this._handle !== null) {//a new configuration is coming, no need to update + this._processQueue(); + return; + } + this._currentJob = null; const affineTransform = positionWord.bind(null, this._element.offsetWidth / 2, this._element.offsetHeight / 2); const svgTextNodes = this._svgGroup.selectAll('text'); - const stage = svgTextNodes.data(this._words, getText); + const stage = svgTextNodes.data(job.words, getText); const enterSelection = stage.enter(); const enteringTags = enterSelection.append('text'); @@ -180,9 +196,9 @@ class TagCloud extends EventEmitter { cloudBBox.x + cloudBBox.width <= this._element.offsetWidth && cloudBBox.y + cloudBBox.height <= this._element.offsetHeight; - this._dirtyPromise = null; - this._resolve(true); - this.emit('renderComplete'); + this._inFlight = false; + this._currentJob = job; + this._processQueue(); } }; exitTransition.each(_ => exits++); @@ -198,48 +214,60 @@ class TagCloud extends EventEmitter { }; - _makeTextSizeMapper() { - this._mapSizeToFontSize = D3_SCALING_FUNCTIONS[this._textScale]; - if (this._words.length === 1) { - this._mapSizeToFontSize.range([this._maxFontSize, this._maxFontSize]); - } else { - this._mapSizeToFontSize.range([this._minFontSize, this._maxFontSize]); - } - + const mapSizeToFontSize = D3_SCALING_FUNCTIONS[this._textScale](); + const range = this._words.length === 1 ? [this._maxFontSize, this._maxFontSize] : [this._minFontSize, this._maxFontSize]; + mapSizeToFontSize.range(range); if (this._words) { - this._mapSizeToFontSize.domain(d3.extent(this._words, getSize)); + mapSizeToFontSize.domain(d3.extent(this._words, getValue)); } + return mapSizeToFontSize; } - _invalidate(keepLayout) { + _makeJob() { + return { + words: this._words.map(toWordTag) + }; + } + _invalidate(keepLayout) { if (!this._words) { return; } - if (!this._dirtyPromise) { - this._dirtyPromise = new Promise((resolve, reject) => { - this._resolve = resolve; - }); - } - - clearTimeout(this._timeoutHandle); - this._timeoutHandle = requestAnimationFrame(() => { - this._timeoutHandle = null; + clearTimeout(this._handle); + this._handle = setTimeout(() => { + this._handle = null; this._updateContainerSize(); - if (keepLayout) { - this._onLayoutEnd(); + if (keepLayout && this._currentJob && this._queue.length === 0) { + this._scheduleLayout({ + words: this._currentJob.words.map(tag => { + return { + x: tag.x, + y: tag.y, + rotate: tag.rotate, + size: tag.size, + text: tag.text + }; + }) + }); } else { - this._washWords(); this._updateLayout(); } - }); + }, 0);//unhook from callstack. this avoids kicking off multiple layouts if multiple changes come in succession + } + + _scheduleLayout(job) { + this._queue.unshift(job); + this._processQueue(); } _updateLayout() { + const job = this._makeJob(); + const mapSizeToFontSize = this._makeTextSizeMapper(); + const tagCloudLayoutGenerator = d3TagCloud(); tagCloudLayoutGenerator.size(this._size); tagCloudLayoutGenerator.padding(this._padding); @@ -247,15 +275,16 @@ class TagCloud extends EventEmitter { tagCloudLayoutGenerator.font(this._fontFamily); tagCloudLayoutGenerator.fontStyle(this._fontStyle); tagCloudLayoutGenerator.fontWeight(this._fontWeight); - tagCloudLayoutGenerator.fontSize(tag => this._mapSizeToFontSize(tag.size)); + tagCloudLayoutGenerator.fontSize(tag => { + return mapSizeToFontSize(tag.value); + }); tagCloudLayoutGenerator.random(seed); tagCloudLayoutGenerator.spiral(this._spiral); - tagCloudLayoutGenerator.words(this._words); + tagCloudLayoutGenerator.words(job.words); tagCloudLayoutGenerator.text(getText); tagCloudLayoutGenerator.timeInterval(this._timeInterval); - tagCloudLayoutGenerator.on('end', this._onLayoutEnd.bind(this)); + tagCloudLayoutGenerator.on('end', () =>this._scheduleLayout(job)); tagCloudLayoutGenerator.start(); - } } @@ -267,7 +296,7 @@ function seed() { } function toWordTag(word) { - return {size: word.size, text: word.text}; + return {value: word.value, text: word.text}; } @@ -278,14 +307,15 @@ function getText(word) { function positionWord(xTranslate, yTranslate, word) { if (isNaN(word.x) || isNaN(word.y) || isNaN(word.rotate)) { + //move off-screen return `translate(${xTranslate * 3}, ${yTranslate * 3})rotate(0)`; } return `translate(${word.x + xTranslate}, ${word.y + yTranslate})rotate(${word.rotate})`; } -function getSize(tag) { - return tag.size; +function getValue(tag) { + return tag.value; } function getSizeInPixels(tag) { diff --git a/src/core_plugins/tagcloud/public/tag_cloud.less b/src/core_plugins/tagcloud/public/tag_cloud.less index c17a378d573c6..b13d3dd2c9abb 100644 --- a/src/core_plugins/tagcloud/public/tag_cloud.less +++ b/src/core_plugins/tagcloud/public/tag_cloud.less @@ -1,5 +1,5 @@ .tagcloud-vis { - position: relative; + position: absolute; top: 0; left: 0; width: 100%; diff --git a/src/core_plugins/tagcloud/public/tag_cloud_controller.js b/src/core_plugins/tagcloud/public/tag_cloud_controller.js index c745831b68f12..4b5ac87773c69 100644 --- a/src/core_plugins/tagcloud/public/tag_cloud_controller.js +++ b/src/core_plugins/tagcloud/public/tag_cloud_controller.js @@ -21,7 +21,6 @@ module.controller('KbnTagCloudController', function ($scope, $element, Private, clickHandler({point: {aggConfigResult: aggConfigResult}}); }); tagCloud.on('renderComplete', () => { - const bucketName = containerNode.querySelector('.tagcloud-custom-label'); bucketName.innerHTML = `${$scope.vis.aggs[0].makeLabel()} - ${$scope.vis.aggs[1].makeLabel()}`; @@ -57,7 +56,7 @@ module.controller('KbnTagCloudController', function ($scope, $element, Private, const tags = buckets.map((bucket) => { return { text: bucket.key, - size: getValue(metricsAgg, bucket) + value: getValue(metricsAgg, bucket) }; }); diff --git a/src/core_plugins/tagcloud/public/tag_cloud_vis.js b/src/core_plugins/tagcloud/public/tag_cloud_vis.js index 6c026ae54eb9e..9766d7934f81c 100644 --- a/src/core_plugins/tagcloud/public/tag_cloud_vis.js +++ b/src/core_plugins/tagcloud/public/tag_cloud_vis.js @@ -13,6 +13,7 @@ visTypes.register(function TagCloudProvider(Private) { return new TemplateVisType({ name: 'tagcloud', title: 'Tag cloud', + implementsRenderComplete: true, description: 'A tag cloud visualization is a visual representation of text data, ' + 'typically used to visualize free form text. Tags are usually single words. The font size of word corresponds' + 'with its importance.',