From fbc03a3bac3f3015930b1feaada194b54d00d9e2 Mon Sep 17 00:00:00 2001 From: archmoj Date: Wed, 12 Jun 2019 19:19:45 -0400 Subject: [PATCH 1/3] fix issue 3954 - implement hoverinfo and hovertemplate for funnel percentages --- src/components/fx/helpers.js | 3 + src/components/fx/hover.js | 16 ++++++ src/traces/funnel/attributes.js | 4 ++ src/traces/funnel/event_data.js | 25 +++++++++ src/traces/funnel/hover.js | 18 +++--- src/traces/funnel/index.js | 2 + test/jasmine/assets/custom_assertions.js | 15 +++-- test/jasmine/tests/funnel_test.js | 71 +++++++++++++++++++++++- 8 files changed, 138 insertions(+), 16 deletions(-) create mode 100644 src/traces/funnel/event_data.js diff --git a/src/components/fx/helpers.js b/src/components/fx/helpers.js index 96fa741aad2..64ac7c3d716 100644 --- a/src/components/fx/helpers.js +++ b/src/components/fx/helpers.js @@ -149,6 +149,9 @@ exports.makeEventData = function makeEventData(pt, trace, cd) { if(pt.xa) out.xaxis = pt.xa; if(pt.ya) out.yaxis = pt.ya; if(pt.zLabelVal !== undefined) out.z = pt.zLabelVal; + if(pt.percentInitial !== undefined) out.z = pt.percentInitial; + if(pt.percentPrevious !== undefined) out.z = pt.percentPrevious; + if(pt.percentTotal !== undefined) out.z = pt.percentTotal; } exports.appendArrayPointValue(out, trace, pointNumber); diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index a32ad3f0922..7c508bb9fa7 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -432,6 +432,9 @@ function _hover(gd, evt, subplot, noHoverEvent) { xLabelVal: undefined, yLabelVal: undefined, zLabelVal: undefined, + percentInitial: undefined, + percentPrevious: undefined, + percentTotal: undefined, text: undefined }; @@ -908,6 +911,16 @@ function createHoverText(hoverData, opts, gd) { text += (text ? '
' : '') + d.text; } + if(d.percentInitial !== undefined) { + text += (text ? '
' : '') + d.percentInitial + ' of initial'; + } + if(d.percentPrevious !== undefined) { + text += (text ? '
' : '') + d.percentPrevious + ' of previous'; + } + if(d.percentTotal !== undefined) { + text += (text ? '
' : '') + d.percentTotal + ' of total'; + } + // used by other modules (initially just ternary) that // manage their own hoverinfo independent of cleanPoint // the rest of this will still apply, so such modules @@ -1383,6 +1396,9 @@ function cleanPoint(d, hovermode) { if(infomode.indexOf('z') === -1) d.zLabel = undefined; if(infomode.indexOf('text') === -1) d.text = undefined; if(infomode.indexOf('name') === -1) d.name = undefined; + if(infomode.indexOf('percentInitial') === -1) d.percentInitial = undefined; + if(infomode.indexOf('percentPrevious') === -1) d.percentPrevious = undefined; + if(infomode.indexOf('percentTotal') === -1) d.percentTotal = undefined; } return d; diff --git a/src/traces/funnel/attributes.js b/src/traces/funnel/attributes.js index 045ea7bca02..0cf444ea76f 100644 --- a/src/traces/funnel/attributes.js +++ b/src/traces/funnel/attributes.js @@ -10,6 +10,7 @@ var barAttrs = require('../bar/attributes'); var lineAttrs = require('../scatter/attributes').line; +var plotAttrs = require('../../plots/attributes'); var extendFlat = require('../../lib/extend').extendFlat; var Color = require('../../components/color'); @@ -23,6 +24,9 @@ module.exports = { hovertext: barAttrs.hovertext, hovertemplate: barAttrs.hovertemplate, + hoverinfo: extendFlat({}, plotAttrs.hoverinfo, { + flags: ['percentInitial', 'percentPrevious', 'percentTotal'].concat(plotAttrs.hoverinfo.flags) + }), textinfo: { valType: 'flaglist', diff --git a/src/traces/funnel/event_data.js b/src/traces/funnel/event_data.js new file mode 100644 index 00000000000..ae94e3d28a0 --- /dev/null +++ b/src/traces/funnel/event_data.js @@ -0,0 +1,25 @@ +/** +* Copyright 2012-2019, Plotly, Inc. +* All rights reserved. +* +* This source code is licensed under the MIT license found in the +* LICENSE file in the root directory of this source tree. +*/ + +'use strict'; + +module.exports = function eventData(out, pt /* , trace, cd, pointNumber */) { + // standard cartesian event data + out.x = 'xVal' in pt ? pt.xVal : pt.x; + out.y = 'yVal' in pt ? pt.yVal : pt.y; + + // for funnel + if('percentInitial' in pt) out.percentInitial = pt.percentInitial; + if('percentPrevious' in pt) out.percentPrevious = pt.percentPrevious; + if('percentTotal' in pt) out.percentTotal = pt.percentTotal; + + if(pt.xa) out.xaxis = pt.xa; + if(pt.ya) out.yaxis = pt.ya; + + return out; +}; diff --git a/src/traces/funnel/hover.js b/src/traces/funnel/hover.js index 42935a87412..574a356c85c 100644 --- a/src/traces/funnel/hover.js +++ b/src/traces/funnel/hover.js @@ -25,16 +25,18 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { var di = cd[index]; var sizeLetter = isHorizontal ? 'x' : 'y'; - point[sizeLetter + 'LabelVal'] = di.s; - // display ratio to initial value - point.extraText = [ - formatPercent(di.begR, 1) + ' of initial', - formatPercent(di.difR, 1) + ' of previous', - formatPercent(di.sumR, 1) + ' of total' - ].join('
'); - // TODO: Should we use pieHelpers.formatPieValue instead ? + var hoverinfo = trace.hoverinfo; + if(hoverinfo !== 'none' && hoverinfo !== 'skip') { + var parts = (hoverinfo || '').split('+'); + var isAll = (hoverinfo === 'all') || (hoverinfo === undefined); + var hasFlag = function(flag) { return isAll || parts.indexOf(flag) !== -1; }; + + if(hasFlag('percentInitial')) point.percentInitial = formatPercent(di.begR, 1); + if(hasFlag('percentPrevious')) point.percentPrevious = formatPercent(di.difR, 1); + if(hasFlag('percentTotal')) point.percentTotal = formatPercent(di.sumR, 1); + } point.color = getTraceColor(trace, di); diff --git a/src/traces/funnel/index.js b/src/traces/funnel/index.js index 844f4097471..d63618e5ec4 100644 --- a/src/traces/funnel/index.js +++ b/src/traces/funnel/index.js @@ -19,6 +19,8 @@ module.exports = { plot: require('./plot'), style: require('./style').style, hoverPoints: require('./hover'), + eventData: require('./event_data'), + selectPoints: require('../bar/select'), moduleType: 'trace', diff --git a/test/jasmine/assets/custom_assertions.js b/test/jasmine/assets/custom_assertions.js index 30828bf7605..e65089c5787 100644 --- a/test/jasmine/assets/custom_assertions.js +++ b/test/jasmine/assets/custom_assertions.js @@ -98,6 +98,7 @@ function count(selector) { * - vOrder {array of number} * - hOrder {array of number} * - isRotated {boolean} + * - isEmpty {boolean} * @param {string} msg */ exports.assertHoverLabelContent = function(expectation, msg) { @@ -192,11 +193,15 @@ exports.assertHoverLabelContent = function(expectation, msg) { } }); } else { - if(expectation.nums) { - fail(ptMsg + ': expecting *nums* labels, did not find any.'); - } - if(expectation.name) { - fail(ptMsg + ': expecting *nums* labels, did not find any.'); + if(expectation.isEmpty) { + return true; + } else { + if(expectation.nums) { + fail(ptMsg + ': expecting *nums* labels, did not find any.'); + } + if(expectation.name) { + fail(ptMsg + ': expecting *nums* labels, did not find any.'); + } } } diff --git a/test/jasmine/tests/funnel_test.js b/test/jasmine/tests/funnel_test.js index 1368f46efb6..f77b1c75768 100644 --- a/test/jasmine/tests/funnel_test.js +++ b/test/jasmine/tests/funnel_test.js @@ -1326,6 +1326,68 @@ describe('funnel hover', function() { .then(done); }); + it('should turn off percentages with hoveinfo none or skip', function(done) { + gd = createGraphDiv(); + + var mock = Lib.extendDeep({}, require('@mocks/text_chart_arrays')); + mock.data.forEach(function(t, i) { + t.type = 'funnel'; + t.orientation = 'v'; + if(i === 0) { + t.hoverinfo = 'none'; + } else { + t.hoverinfo = 'skip'; + } + }); + + function _hover() { + var evt = { xpx: 125, ypx: 150 }; + Fx.hover('graph', evt, 'xy'); + } + + Plotly.plot(gd, mock) + .then(_hover) + .then(function() { + assertHoverLabelContent({ + isEmpty: true + }); + }) + .catch(failTest) + .then(done); + }); + + it('should turn on percentages with hoveinfo all', function(done) { + gd = createGraphDiv(); + + var mock = Lib.extendDeep({}, require('@mocks/text_chart_arrays')); + mock.data.forEach(function(t) { + t.type = 'funnel'; + t.orientation = 'v'; + t.hoverinfo = 'all'; + }); + + function _hover() { + var evt = { xpx: 125, ypx: 150 }; + Fx.hover('graph', evt, 'xy'); + } + + Plotly.plot(gd, mock) + .then(_hover) + .then(function() { + assertHoverLabelContent({ + nums: [ + '1\nHover text A\n100% of initial\n100% of previous\n33.3% of total', + '2\nHover text G\n100% of initial\n100% of previous\n33.3% of total', + '1.5\na (hover)\n100% of initial\n100% of previous\n33.3% of total' + ], + name: ['Lines, Marke...', 'Lines and Text', 'missing text'], + axis: '0' + }); + }) + .catch(failTest) + .then(done); + }); + it('should use hovertemplate if specified', function(done) { gd = createGraphDiv(); @@ -1333,7 +1395,7 @@ describe('funnel hover', function() { mock.data.forEach(function(t) { t.type = 'funnel'; t.orientation = 'v'; - t.hovertemplate = '%{y}'; + t.hovertemplate = 'Value: %{y}
Total percentage: %{percentTotal}
Initial percentage: %{percentInitial}
Previous percentage: %{percentPrevious}'; }); function _hover() { @@ -1345,11 +1407,14 @@ describe('funnel hover', function() { .then(_hover) .then(function() { assertHoverLabelContent({ - nums: ['1', '2', '1.5'], + nums: [ + 'Value: 1\nTotal percentage: 33.3%\nInitial percentage: 100%\nPrevious percentage: 100%', + 'Value: 2\nTotal percentage: 33.3%\nInitial percentage: 100%\nPrevious percentage: 100%', + 'Value: 1.5\nTotal percentage: 33.3%\nInitial percentage: 100%\nPrevious percentage: 100%' + ], name: ['', '', ''], axis: '0' }); - // return Plotly.restyle(gd, 'text', ['APPLE', 'BANANA', 'ORANGE']); }) .catch(failTest) .then(done); From 1701325206e7adc43ec0cf360a324d3135c3daae Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 13 Jun 2019 14:24:30 -0400 Subject: [PATCH 2/3] revise funnel hovertemplate and hoverinfo based on comments --- src/components/fx/hover.js | 16 ---------------- src/traces/funnel/attributes.js | 9 +++++++-- src/traces/funnel/constants.js | 17 +++++++++++++++++ src/traces/funnel/hover.js | 32 +++++++++++++++++++++++++------- 4 files changed, 49 insertions(+), 25 deletions(-) create mode 100644 src/traces/funnel/constants.js diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js index 7c508bb9fa7..a32ad3f0922 100644 --- a/src/components/fx/hover.js +++ b/src/components/fx/hover.js @@ -432,9 +432,6 @@ function _hover(gd, evt, subplot, noHoverEvent) { xLabelVal: undefined, yLabelVal: undefined, zLabelVal: undefined, - percentInitial: undefined, - percentPrevious: undefined, - percentTotal: undefined, text: undefined }; @@ -911,16 +908,6 @@ function createHoverText(hoverData, opts, gd) { text += (text ? '
' : '') + d.text; } - if(d.percentInitial !== undefined) { - text += (text ? '
' : '') + d.percentInitial + ' of initial'; - } - if(d.percentPrevious !== undefined) { - text += (text ? '
' : '') + d.percentPrevious + ' of previous'; - } - if(d.percentTotal !== undefined) { - text += (text ? '
' : '') + d.percentTotal + ' of total'; - } - // used by other modules (initially just ternary) that // manage their own hoverinfo independent of cleanPoint // the rest of this will still apply, so such modules @@ -1396,9 +1383,6 @@ function cleanPoint(d, hovermode) { if(infomode.indexOf('z') === -1) d.zLabel = undefined; if(infomode.indexOf('text') === -1) d.text = undefined; if(infomode.indexOf('name') === -1) d.name = undefined; - if(infomode.indexOf('percentInitial') === -1) d.percentInitial = undefined; - if(infomode.indexOf('percentPrevious') === -1) d.percentPrevious = undefined; - if(infomode.indexOf('percentTotal') === -1) d.percentTotal = undefined; } return d; diff --git a/src/traces/funnel/attributes.js b/src/traces/funnel/attributes.js index 0cf444ea76f..5e5f5f2ed27 100644 --- a/src/traces/funnel/attributes.js +++ b/src/traces/funnel/attributes.js @@ -11,6 +11,8 @@ var barAttrs = require('../bar/attributes'); var lineAttrs = require('../scatter/attributes').line; var plotAttrs = require('../../plots/attributes'); +var hovertemplateAttrs = require('../../components/fx/hovertemplate_attributes'); +var constants = require('./constants'); var extendFlat = require('../../lib/extend').extendFlat; var Color = require('../../components/color'); @@ -23,9 +25,12 @@ module.exports = { dy: barAttrs.dy, hovertext: barAttrs.hovertext, - hovertemplate: barAttrs.hovertemplate, + hovertemplate: hovertemplateAttrs({}, { + keys: constants.eventDataKeys + }), + hoverinfo: extendFlat({}, plotAttrs.hoverinfo, { - flags: ['percentInitial', 'percentPrevious', 'percentTotal'].concat(plotAttrs.hoverinfo.flags) + flags: ['name', 'x', 'y', 'text', 'percent initial', 'percent previous', 'percent total'] }), textinfo: { diff --git a/src/traces/funnel/constants.js b/src/traces/funnel/constants.js new file mode 100644 index 00000000000..590f59a2b2c --- /dev/null +++ b/src/traces/funnel/constants.js @@ -0,0 +1,17 @@ +/** +* Copyright 2012-2019, Plotly, Inc. +* All rights reserved. +* +* This source code is licensed under the MIT license found in the +* LICENSE file in the root directory of this source tree. +*/ + +'use strict'; + +module.exports = { + eventDataKeys: [ + 'percentInitial', + 'percentPrevious', + 'percentTotal' + ] +}; diff --git a/src/traces/funnel/hover.js b/src/traces/funnel/hover.js index 574a356c85c..cbc0f0e7015 100644 --- a/src/traces/funnel/hover.js +++ b/src/traces/funnel/hover.js @@ -27,16 +27,34 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) { var sizeLetter = isHorizontal ? 'x' : 'y'; point[sizeLetter + 'LabelVal'] = di.s; - var hoverinfo = trace.hoverinfo; - if(hoverinfo !== 'none' && hoverinfo !== 'skip') { - var parts = (hoverinfo || '').split('+'); - var isAll = (hoverinfo === 'all') || (hoverinfo === undefined); + point.percentInitial = di.begR; + point.percentInitialLabel = formatPercent(di.begR, 1); + + point.percentPrevious = di.difR; + point.percentPreviousLabel = formatPercent(di.difR, 1); + + point.percentTotal = di.sumR; + point.percentTotalLabel = formatPercent(di.sumR, 1); + + var hoverinfo = di.hi || trace.hoverinfo; + var text = []; + if(hoverinfo && hoverinfo !== 'none' && hoverinfo !== 'skip') { + var isAll = (hoverinfo === 'all'); + var parts = hoverinfo.split('+'); + var hasFlag = function(flag) { return isAll || parts.indexOf(flag) !== -1; }; - if(hasFlag('percentInitial')) point.percentInitial = formatPercent(di.begR, 1); - if(hasFlag('percentPrevious')) point.percentPrevious = formatPercent(di.difR, 1); - if(hasFlag('percentTotal')) point.percentTotal = formatPercent(di.sumR, 1); + if(hasFlag('percent initial')) { + text.push(point.percentInitialLabel + ' of initial'); + } + if(hasFlag('percent previous')) { + text.push(point.percentPreviousLabel + ' of previous'); + } + if(hasFlag('percent total')) { + text.push(point.percentTotalLabel + ' of total'); + } } + point.extraText = text.join('
'); point.color = getTraceColor(trace, di); From ea54842d2686d1a8bd572d60019be19ff315761b Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 13 Jun 2019 15:57:59 -0400 Subject: [PATCH 3/3] clean up fx helpers from funnel info - cleanup custom_assertions from isEmpty hack --- src/components/fx/helpers.js | 3 --- test/jasmine/assets/custom_assertions.js | 15 +++++---------- test/jasmine/tests/funnel_test.js | 4 +--- 3 files changed, 6 insertions(+), 16 deletions(-) diff --git a/src/components/fx/helpers.js b/src/components/fx/helpers.js index 64ac7c3d716..96fa741aad2 100644 --- a/src/components/fx/helpers.js +++ b/src/components/fx/helpers.js @@ -149,9 +149,6 @@ exports.makeEventData = function makeEventData(pt, trace, cd) { if(pt.xa) out.xaxis = pt.xa; if(pt.ya) out.yaxis = pt.ya; if(pt.zLabelVal !== undefined) out.z = pt.zLabelVal; - if(pt.percentInitial !== undefined) out.z = pt.percentInitial; - if(pt.percentPrevious !== undefined) out.z = pt.percentPrevious; - if(pt.percentTotal !== undefined) out.z = pt.percentTotal; } exports.appendArrayPointValue(out, trace, pointNumber); diff --git a/test/jasmine/assets/custom_assertions.js b/test/jasmine/assets/custom_assertions.js index e65089c5787..30828bf7605 100644 --- a/test/jasmine/assets/custom_assertions.js +++ b/test/jasmine/assets/custom_assertions.js @@ -98,7 +98,6 @@ function count(selector) { * - vOrder {array of number} * - hOrder {array of number} * - isRotated {boolean} - * - isEmpty {boolean} * @param {string} msg */ exports.assertHoverLabelContent = function(expectation, msg) { @@ -193,15 +192,11 @@ exports.assertHoverLabelContent = function(expectation, msg) { } }); } else { - if(expectation.isEmpty) { - return true; - } else { - if(expectation.nums) { - fail(ptMsg + ': expecting *nums* labels, did not find any.'); - } - if(expectation.name) { - fail(ptMsg + ': expecting *nums* labels, did not find any.'); - } + if(expectation.nums) { + fail(ptMsg + ': expecting *nums* labels, did not find any.'); + } + if(expectation.name) { + fail(ptMsg + ': expecting *nums* labels, did not find any.'); } } diff --git a/test/jasmine/tests/funnel_test.js b/test/jasmine/tests/funnel_test.js index f77b1c75768..cb3861833ae 100644 --- a/test/jasmine/tests/funnel_test.js +++ b/test/jasmine/tests/funnel_test.js @@ -1348,9 +1348,7 @@ describe('funnel hover', function() { Plotly.plot(gd, mock) .then(_hover) .then(function() { - assertHoverLabelContent({ - isEmpty: true - }); + expect(d3.selectAll('g.hovertext').size()).toBe(0); }) .catch(failTest) .then(done);