From 01205f8d3f5e84086319f18f57140cbb4625a7dd Mon Sep 17 00:00:00 2001 From: etimberg Date: Sun, 9 Oct 2016 21:24:34 -0400 Subject: [PATCH 1/6] Initial refactor and adding mode that uses nearest item. --- src/core/core.controller.js | 21 +++---- src/core/core.tooltip.js | 118 +++++++++++++++++++++++++----------- 2 files changed, 93 insertions(+), 46 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index 6302dd11cef..0340716bc60 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -713,25 +713,22 @@ module.exports = function(Chart) { // Built in Tooltips if (tooltipsOptions.enabled || tooltipsOptions.custom) { tooltip._active = me.tooltipActive; + tooltip._eventPosition = helpers.getRelativePosition(e, me.chart); } // Hover animations if (!me.animating) { // If entering, leaving, or changing elements, animate the change via pivot - if (!helpers.arrayEquals(me.active, me.lastActive) || - !helpers.arrayEquals(me.tooltipActive, me.lastTooltipActive)) { - - me.stop(); - - if (tooltipsOptions.enabled || tooltipsOptions.custom) { - tooltip.update(true); - tooltip.pivot(); - } + me.stop(); - // We only need to render at this point. Updating will cause scales to be - // recomputed generating flicker & using more memory than necessary. - me.render(hoverOptions.animationDuration, true); + if (tooltipsOptions.enabled || tooltipsOptions.custom) { + tooltip.update(true); + tooltip.pivot(); } + + // We only need to render at this point. Updating will cause scales to be + // recomputed generating flicker & using more memory than necessary. + me.render(hoverOptions.animationDuration, true); } // Remember Last Actives diff --git a/src/core/core.tooltip.js b/src/core/core.tooltip.js index dcb9e89d31e..c4c9b110e93 100755 --- a/src/core/core.tooltip.js +++ b/src/core/core.tooltip.js @@ -16,6 +16,7 @@ module.exports = function(Chart) { enabled: true, custom: null, mode: 'nearest', + positionMode: 'average', intersect: true, backgroundColor: 'rgba(0,0,0,0.8)', titleFontStyle: 'bold', @@ -104,39 +105,6 @@ module.exports = function(Chart) { return base; } - function getAveragePosition(elements) { - if (!elements.length) { - return false; - } - - var i, len; - var xPositions = []; - var yPositions = []; - - for (i = 0, len = elements.length; i < len; ++i) { - var el = elements[i]; - if (el && el.hasValue()) { - var pos = el.tooltipPosition(); - xPositions.push(pos.x); - yPositions.push(pos.y); - } - } - - var x = 0, - y = 0; - for (i = 0; i < xPositions.length; ++i) { - if (xPositions[i]) { - x += xPositions[i]; - y += yPositions[i]; - } - } - - return { - x: Math.round(x / xPositions.length), - y: Math.round(y / xPositions.length) - }; - } - // Private helper to create a tooltip iteam model // @param element : the chart element (point, arc, bar) to create the tooltip item for // @return : new tooltip item @@ -504,7 +472,7 @@ module.exports = function(Chart) { model.opacity = 1; var labelColors = [], - tooltipPosition = getAveragePosition(active); + tooltipPosition = Chart.Tooltip.modes[opts.positionMode](active, me._eventPosition); var tooltipItems = []; for (i = 0, len = active.length; i < len; ++i) { @@ -772,4 +740,86 @@ module.exports = function(Chart) { } } }); + + /** + * @namespace Chart.Tooltip.modes + */ + Chart.Tooltip.modes = { + /** + * Average mode places the tooltip at the average position of the elements shown + * @function Chart.Tooltip.modes.average + * @param elements {ChartElement[]} the elements being displayed in the tooltip + * @returns {Point} tooltip position + */ + average: function(elements) { + if (!elements.length) { + return false; + } + + var i, len; + var xPositions = []; + var yPositions = []; + + for (i = 0, len = elements.length; i < len; ++i) { + var el = elements[i]; + if (el && el.hasValue()) { + var pos = el.tooltipPosition(); + xPositions.push(pos.x); + yPositions.push(pos.y); + } + } + + var x = 0, + y = 0; + for (i = 0; i < xPositions.length; ++i) { + if (xPositions[i]) { + x += xPositions[i]; + y += yPositions[i]; + } + } + + return { + x: Math.round(x / xPositions.length), + y: Math.round(y / xPositions.length) + }; + }, + + /** + * Gets the tooltip position nearest of the item nearest to the event position + * @param elements {Chart.Element[]} the tooltip elements + * @param eventPosition {Point} the position of the event in canvas coordinates + * @returns {Point} the tooltip position + */ + nearest: function(elements, eventPosition) { + var x = eventPosition.x; + var y = eventPosition.y; + + var nearestElement; + var minDistance = Number.POSITIVE_INFINITY; + var i, len; + for (i = 0, len = elements.length; i < len; ++i) { + var el = elements[i]; + if (el && el.hasValue()) { + var center = el.getCenterPoint(); + var d = helpers.distanceBetweenPoints(eventPosition, center); + + if (d < minDistance) { + minDistance = d; + nearestElement = el; + } + } + } + + if (nearestElement) { + var tp = nearestElement.tooltipPosition(); + x = tp.x; + y = tp.y; + } + + return { + x: x, + y: y + }; + } + }; }; From fc717905b4db471971621004f5e50f821d34ee2c Mon Sep 17 00:00:00 2001 From: etimberg Date: Sun, 9 Oct 2016 21:27:33 -0400 Subject: [PATCH 2/6] documentation of new option --- docs/01-Chart-Configuration.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/01-Chart-Configuration.md b/docs/01-Chart-Configuration.md index 1aeaab940fb..aedd4616709 100644 --- a/docs/01-Chart-Configuration.md +++ b/docs/01-Chart-Configuration.md @@ -214,6 +214,7 @@ enabled | Boolean | true | Are tooltips enabled custom | Function | null | See [section](#advanced-usage-external-tooltips) below mode | String | 'nearest' | Sets which elements appear in the tooltip. See [Interaction Modes](#interaction-modes) for details intersect | Boolean | true | if true, the tooltip mode applies only when the mouse position intersects with an element. If false, the mode will be applied at all times. +positionMode | String | 'average' | The mode for positioning the tooltip. 'average' mode will place the tooltip at the average position of the items displayed in the tooltip. 'nearest' will place the tooltip at the position of the element closest to the event position itemSort | Function | undefined | Allows sorting of [tooltip items](#chart-configuration-tooltip-item-interface). Must implement at minimum a function that can be passed to [Array.prototype.sort](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort). This function can also accept a third parameter that is the data object passed to the chart. backgroundColor | Color | 'rgba(0,0,0,0.8)' | Background color of the tooltip titleFontFamily | String | "'Helvetica Neue', 'Helvetica', 'Arial', sans-serif" | Font family for tooltip title inherited from global font family From c3bd19bc13e3e0a78faee2018d9b6d20b7f04b08 Mon Sep 17 00:00:00 2001 From: etimberg Date: Sun, 9 Oct 2016 21:36:44 -0400 Subject: [PATCH 3/6] Refactoring average position implementation for less looping --- src/core/core.tooltip.js | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/src/core/core.tooltip.js b/src/core/core.tooltip.js index c4c9b110e93..56a81bce81a 100755 --- a/src/core/core.tooltip.js +++ b/src/core/core.tooltip.js @@ -757,30 +757,23 @@ module.exports = function(Chart) { } var i, len; - var xPositions = []; - var yPositions = []; + var x = 0; + var y = 0; + var count = 0; for (i = 0, len = elements.length; i < len; ++i) { var el = elements[i]; if (el && el.hasValue()) { var pos = el.tooltipPosition(); - xPositions.push(pos.x); - yPositions.push(pos.y); - } - } - - var x = 0, - y = 0; - for (i = 0; i < xPositions.length; ++i) { - if (xPositions[i]) { - x += xPositions[i]; - y += yPositions[i]; + x += pos.x; + y += pos.y; + ++count; } } return { - x: Math.round(x / xPositions.length), - y: Math.round(y / xPositions.length) + x: Math.round(x / count), + y: Math.round(y / count) }; }, From 442bb044de206c4dcd6dac977c264faf53ae46fd Mon Sep 17 00:00:00 2001 From: etimberg Date: Mon, 10 Oct 2016 09:00:26 -0400 Subject: [PATCH 4/6] Update field names per code review comments --- docs/01-Chart-Configuration.md | 2 +- src/core/core.tooltip.js | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/01-Chart-Configuration.md b/docs/01-Chart-Configuration.md index aedd4616709..e3b54091f23 100644 --- a/docs/01-Chart-Configuration.md +++ b/docs/01-Chart-Configuration.md @@ -214,7 +214,7 @@ enabled | Boolean | true | Are tooltips enabled custom | Function | null | See [section](#advanced-usage-external-tooltips) below mode | String | 'nearest' | Sets which elements appear in the tooltip. See [Interaction Modes](#interaction-modes) for details intersect | Boolean | true | if true, the tooltip mode applies only when the mouse position intersects with an element. If false, the mode will be applied at all times. -positionMode | String | 'average' | The mode for positioning the tooltip. 'average' mode will place the tooltip at the average position of the items displayed in the tooltip. 'nearest' will place the tooltip at the position of the element closest to the event position +position | String | 'average' | The mode for positioning the tooltip. 'average' mode will place the tooltip at the average position of the items displayed in the tooltip. 'nearest' will place the tooltip at the position of the element closest to the event position. New modes can be defined by adding functions to the Chart.Tooltip.positioners map. itemSort | Function | undefined | Allows sorting of [tooltip items](#chart-configuration-tooltip-item-interface). Must implement at minimum a function that can be passed to [Array.prototype.sort](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort). This function can also accept a third parameter that is the data object passed to the chart. backgroundColor | Color | 'rgba(0,0,0,0.8)' | Background color of the tooltip titleFontFamily | String | "'Helvetica Neue', 'Helvetica', 'Arial', sans-serif" | Font family for tooltip title inherited from global font family diff --git a/src/core/core.tooltip.js b/src/core/core.tooltip.js index 56a81bce81a..3cfc2106067 100755 --- a/src/core/core.tooltip.js +++ b/src/core/core.tooltip.js @@ -16,7 +16,7 @@ module.exports = function(Chart) { enabled: true, custom: null, mode: 'nearest', - positionMode: 'average', + position: 'average', intersect: true, backgroundColor: 'rgba(0,0,0,0.8)', titleFontStyle: 'bold', @@ -472,7 +472,7 @@ module.exports = function(Chart) { model.opacity = 1; var labelColors = [], - tooltipPosition = Chart.Tooltip.modes[opts.positionMode](active, me._eventPosition); + tooltipPosition = Chart.Tooltip.positioners[opts.position](active, me._eventPosition); var tooltipItems = []; for (i = 0, len = active.length; i < len; ++i) { @@ -742,12 +742,12 @@ module.exports = function(Chart) { }); /** - * @namespace Chart.Tooltip.modes + * @namespace Chart.Tooltip.positioners */ - Chart.Tooltip.modes = { + Chart.Tooltip.positioners = { /** * Average mode places the tooltip at the average position of the elements shown - * @function Chart.Tooltip.modes.average + * @function Chart.Tooltip.positioners.average * @param elements {ChartElement[]} the elements being displayed in the tooltip * @returns {Point} tooltip position */ @@ -779,6 +779,7 @@ module.exports = function(Chart) { /** * Gets the tooltip position nearest of the item nearest to the event position + * @function Chart.Tooltip.positioners.nearest * @param elements {Chart.Element[]} the tooltip elements * @param eventPosition {Point} the position of the event in canvas coordinates * @returns {Point} the tooltip position From 4dffc360eeaf64bf2432390836b034752e5d282e Mon Sep 17 00:00:00 2001 From: etimberg Date: Tue, 11 Oct 2016 19:07:14 -0400 Subject: [PATCH 5/6] Refactor event handling to know when parts change and only re-render then. --- src/core/core.controller.js | 84 +++++++++++++++++-------------------- src/core/core.legend.js | 12 +++++- src/core/core.tooltip.js | 38 +++++++++++++++++ 3 files changed, 87 insertions(+), 47 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index 0340716bc60..fc3b7b5f181 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -156,6 +156,7 @@ module.exports = function(Chart) { me.chart = instance; me.config = config; me.options = config.options; + me._bufferedUpdate = false; // Add the chart instance to the global namespace Chart.instances[me.id] = me; @@ -403,7 +404,9 @@ module.exports = function(Chart) { // Do this before render so that any plugins that need final scale updates can use it Chart.plugins.notify('afterUpdate', [me]); - me.render(animationDuration, lazy); + if (!me._bufferedUpdate) { + me.render(animationDuration, lazy); + } }, /** @@ -644,20 +647,6 @@ module.exports = function(Chart) { var method = enabled? 'setHoverStyle' : 'removeHoverStyle'; var element, i, ilen; - switch (mode) { - case 'single': - elements = [elements[0]]; - break; - case 'label': - case 'dataset': - case 'x-axis': - // elements = elements; - break; - default: - // unsupported mode - return; - } - for (i=0, ilen=elements.length; i Date: Fri, 14 Oct 2016 17:30:11 -0400 Subject: [PATCH 6/6] Rename _bufferedUpdate to _bufferedRender --- src/core/core.controller.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index fc3b7b5f181..5e3632e4fde 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -156,7 +156,7 @@ module.exports = function(Chart) { me.chart = instance; me.config = config; me.options = config.options; - me._bufferedUpdate = false; + me._bufferedRender = false; // Add the chart instance to the global namespace Chart.instances[me.id] = me; @@ -404,7 +404,7 @@ module.exports = function(Chart) { // Do this before render so that any plugins that need final scale updates can use it Chart.plugins.notify('afterUpdate', [me]); - if (!me._bufferedUpdate) { + if (!me._bufferedRender) { me.render(animationDuration, lazy); } }, @@ -660,7 +660,7 @@ module.exports = function(Chart) { var hoverOptions = me.options.hover; // Buffer any update calls so that renders do not occur - me._bufferedUpdate = true; + me._bufferedRender = true; var changed = me.handleEvent(e); changed |= me.legend.handleEvent(e); @@ -675,7 +675,7 @@ module.exports = function(Chart) { me.render(hoverOptions.animationDuration, true); } - me._bufferedUpdate = false; + me._bufferedRender = false; return me; },