Merge pull request #5864 from plotly/period-hover-timezone

archmoj · web-flow · commit 129fc2a11ab5 · 2021-07-29T16:07:44.000-04:00
Fix period positioned hover to work in different time zones as well as on grouped bars
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -31,6 +31,35 @@ jobs:
           paths:
             - plotly.js
 
+  timezone-jasmine:
+    docker:
+      # need '-browsers' version to test in real (xvfb-wrapped) browsers
+      - image: circleci/node:12.22.1-browsers
+    working_directory: ~/plotly.js
+    steps:
+      - attach_workspace:
+          at: ~/
+      - run:
+          name: Run hover_label test in UTC timezone
+          environment:
+            TZ: "UTC"
+          command: date && npm run test-jasmine hover_label
+      - run:
+          name: Run hover_label test in Europe/Berlin timezone
+          environment:
+            TZ: "Europe/Berlin"
+          command: date && npm run test-jasmine hover_label
+      - run:
+          name: Run hover_label test in Asia/Tokyo timezone
+          environment:
+            TZ: "Asia/Tokyo"
+          command: date && npm run test-jasmine hover_label
+      - run:
+          name: Run hover_label test in America/Toronto timezone
+          environment:
+            TZ: "America/Toronto"
+          command: date && npm run test-jasmine hover_label
+
   no-gl-jasmine:
     docker:
       # need '-browsers' version to test in real (xvfb-wrapped) browsers
@@ -258,6 +287,9 @@ workflows:
   build-and-test:
     jobs:
       - install-and-cibuild
+      - timezone-jasmine:
+          requires:
+            - install-and-cibuild
       - bundle-jasmine:
           requires:
             - install-and-cibuild
diff --git a/draftlogs/5864_fix.md b/draftlogs/5864_fix.md
@@ -0,0 +1 @@
+ - Fix period positioned hover to work in different time zones as well as on grouped bars [[#5864](https://github.com/plotly/plotly.js/pull/5864)]
diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js
@@ -1993,13 +1993,32 @@ function getCoord(axLetter, winningPoint, fullLayout) {
     var ax = winningPoint[axLetter + 'a'];
     var val = winningPoint[axLetter + 'Val'];
 
+    var cd0 = winningPoint.cd[0];
+
     if(ax.type === 'category') val = ax._categoriesMap[val];
     else if(ax.type === 'date') {
-        var period = winningPoint[axLetter + 'Period'];
-        val = ax.d2c(period !== undefined ? period : val);
+        var periodalignment = winningPoint.trace[axLetter + 'periodalignment'];
+        if(periodalignment) {
+            var d = winningPoint.cd[winningPoint.index];
+
+            var start = d[axLetter + 'Start'];
+            if(start === undefined) start = d[axLetter];
+
+            var end = d[axLetter + 'End'];
+            if(end === undefined) end = d[axLetter];
+
+            var diff = end - start;
+
+            if(periodalignment === 'end') {
+                val += diff;
+            } else if(periodalignment === 'middle') {
+                val += diff / 2;
+            }
+        }
+
+        val = ax.d2c(val);
     }
 
-    var cd0 = winningPoint.cd[winningPoint.index];
     if(cd0 && cd0.t && cd0.t.posLetter === ax._id) {
         if(
             fullLayout.boxmode === 'group' ||
diff --git a/src/traces/bar/calc.js b/src/traces/bar/calc.js
@@ -10,7 +10,7 @@ var calcSelection = require('../scatter/calc_selection');
 module.exports = function calc(gd, trace) {
     var xa = Axes.getFromId(gd, trace.xaxis || 'x');
     var ya = Axes.getFromId(gd, trace.yaxis || 'y');
-    var size, pos, origPos, pObj, hasPeriod;
+    var size, pos, origPos, pObj, hasPeriod, pLetter;
 
     var sizeOpts = {
         msUTC: !!(trace.base || trace.base === 0)
@@ -21,11 +21,13 @@ module.exports = function calc(gd, trace) {
         origPos = ya.makeCalcdata(trace, 'y');
         pObj = alignPeriod(trace, ya, 'y', origPos);
         hasPeriod = !!trace.yperiodalignment;
+        pLetter = 'y';
     } else {
         size = ya.makeCalcdata(trace, 'y', sizeOpts);
         origPos = xa.makeCalcdata(trace, 'x');
         pObj = alignPeriod(trace, xa, 'x', origPos);
         hasPeriod = !!trace.xperiodalignment;
+        pLetter = 'x';
     }
     pos = pObj.vals;
 
@@ -39,8 +41,8 @@ module.exports = function calc(gd, trace) {
 
         if(hasPeriod) {
             cd[i].orig_p = origPos[i]; // used by hover
-            cd[i].pEnd = pObj.ends[i];
-            cd[i].pStart = pObj.starts[i];
+            cd[i][pLetter + 'End'] = pObj.ends[i];
+            cd[i][pLetter + 'Start'] = pObj.starts[i];
         }
 
         if(trace.ids) {
diff --git a/src/traces/bar/cross_trace_calc.js b/src/traces/bar/cross_trace_calc.js
@@ -436,20 +436,12 @@ function setBarCenterAndWidth(pa, sieve) {
         var barwidth = t.barwidth;
         var barwidthIsArray = Array.isArray(barwidth);
 
-        var trace = calcTrace[0].trace;
-        var isPeriod = !!trace[pLetter + 'periodalignment'];
-
         for(var j = 0; j < calcTrace.length; j++) {
             var calcBar = calcTrace[j];
 
             // store the actual bar width and position, for use by hover
             var width = calcBar.w = barwidthIsArray ? barwidth[j] : barwidth;
             calcBar[pLetter] = calcBar.p + (poffsetIsArray ? poffset[j] : poffset) + width / 2;
-
-            if(isPeriod) {
-                calcBar.wPeriod =
-                    calcBar.pEnd - calcBar.pStart;
-            }
         }
     }
 }
diff --git a/src/traces/bar/hover.js b/src/traces/bar/hover.js
@@ -52,18 +52,26 @@ function hoverOnBars(pointData, xval, yval, hovermode, opts) {
     }
 
     var period = trace[posLetter + 'period'];
+    var isClosestOrPeriod = isClosest || period;
 
     function thisBarMinPos(di) { return thisBarExtPos(di, -1); }
     function thisBarMaxPos(di) { return thisBarExtPos(di, 1); }
 
     function thisBarExtPos(di, sgn) {
-        var w = (period) ? di.wPeriod : di.w;
+        var w = di.w;
 
         return di[posLetter] + sgn * w / 2;
     }
 
-    var minPos = isClosest || period ?
-        thisBarMinPos :
+    function periodLength(di) {
+        return di[posLetter + 'End'] - di[posLetter + 'Start'];
+    }
+
+    var minPos = isClosest ?
+        thisBarMinPos : period ?
+        function(di) {
+            return di.p - periodLength(di) / 2;
+        } :
         function(di) {
             /*
              * In compare mode, accept a bar if you're on it *or* its group.
@@ -80,8 +88,11 @@ function hoverOnBars(pointData, xval, yval, hovermode, opts) {
             return Math.min(thisBarMinPos(di), di.p - t.bardelta / 2);
         };
 
-    var maxPos = isClosest || period ?
-        thisBarMaxPos :
+    var maxPos = isClosest ?
+        thisBarMaxPos : period ?
+        function(di) {
+            return di.p + periodLength(di) / 2;
+        } :
         function(di) {
             return Math.max(thisBarMaxPos(di), di.p + t.bardelta / 2);
         };
@@ -156,7 +167,7 @@ function hoverOnBars(pointData, xval, yval, hovermode, opts) {
     // if we get here and we're not in 'closest' mode, push min/max pos back
     // onto the group - even though that means occasionally the mouse will be
     // over the hover label.
-    if(!isClosest) {
+    if(!isClosestOrPeriod) {
         minPos = function(di) {
             return Math.min(thisBarMinPos(di), di.p - t.bargroupwidth / 2);
         };
@@ -179,9 +190,6 @@ function hoverOnBars(pointData, xval, yval, hovermode, opts) {
 
     var hasPeriod = di.orig_p !== undefined;
     pointData[posLetter + 'LabelVal'] = hasPeriod ? di.orig_p : di.p;
-    if(hasPeriod) {
-        pointData[posLetter + 'Period'] = di.p;
-    }
 
     pointData.labelLabel = hoverLabelText(pa, pointData[posLetter + 'LabelVal'], trace[posLetter + 'hoverformat']);
     pointData.valueLabel = hoverLabelText(sa, pointData[sizeLetter + 'LabelVal'], trace[sizeLetter + 'hoverformat']);
diff --git a/src/traces/funnel/calc.js b/src/traces/funnel/calc.js
@@ -9,18 +9,20 @@ var BADNUM = require('../../constants/numerical').BADNUM;
 module.exports = function calc(gd, trace) {
     var xa = Axes.getFromId(gd, trace.xaxis || 'x');
     var ya = Axes.getFromId(gd, trace.yaxis || 'y');
-    var size, pos, origPos, pObj, hasPeriod, i, cdi;
+    var size, pos, origPos, pObj, hasPeriod, pLetter, i, cdi;
 
     if(trace.orientation === 'h') {
         size = xa.makeCalcdata(trace, 'x');
         origPos = ya.makeCalcdata(trace, 'y');
         pObj = alignPeriod(trace, ya, 'y', origPos);
         hasPeriod = !!trace.yperiodalignment;
+        pLetter = 'y';
     } else {
         size = ya.makeCalcdata(trace, 'y');
         origPos = xa.makeCalcdata(trace, 'x');
         pObj = alignPeriod(trace, xa, 'x', origPos);
         hasPeriod = !!trace.xperiodalignment;
+        pLetter = 'x';
     }
     pos = pObj.vals;
 
@@ -55,8 +57,8 @@ module.exports = function calc(gd, trace) {
 
         if(hasPeriod) {
             cd[i].orig_p = origPos[i]; // used by hover
-            cd[i].pEnd = pObj.ends[i];
-            cd[i].pStart = pObj.starts[i];
+            cd[i][pLetter + 'End'] = pObj.ends[i];
+            cd[i][pLetter + 'Start'] = pObj.starts[i];
         }
 
         if(trace.ids) {
diff --git a/src/traces/scatter/hover.js b/src/traces/scatter/hover.js
@@ -112,9 +112,6 @@ module.exports = function hoverPoints(pointData, xval, yval, hovermode) {
                 hovertemplate: trace.hovertemplate
             });
 
-            if(trace.xperiodalignment === 'end') pointData.xPeriod = di.x;
-            if(trace.yperiodalignment === 'end') pointData.yPeriod = di.y;
-
             fillText(di, trace, pointData);
             Registry.getComponentMethod('errorbars', 'hoverInfo')(di, trace, pointData);
 
diff --git a/src/traces/scattergl/hover.js b/src/traces/scattergl/hover.js
@@ -202,9 +202,6 @@ function calcHover(pointData, x, y, trace) {
         hovertemplate: di.ht
     });
 
-    if(trace.xperiodalignment === 'end') pointData2.xPeriod = di.x;
-    if(trace.yperiodalignment === 'end') pointData2.yPeriod = di.y;
-
     if(di.htx) pointData2.text = di.htx;
     else if(di.tx) pointData2.text = di.tx;
     else if(trace.text) pointData2.text = trace.text;
diff --git a/src/traces/waterfall/calc.js b/src/traces/waterfall/calc.js
@@ -17,18 +17,20 @@ function isTotal(a) {
 module.exports = function calc(gd, trace) {
     var xa = Axes.getFromId(gd, trace.xaxis || 'x');
     var ya = Axes.getFromId(gd, trace.yaxis || 'y');
-    var size, pos, origPos, pObj, hasPeriod;
+    var size, pos, origPos, pObj, hasPeriod, pLetter;
 
     if(trace.orientation === 'h') {
         size = xa.makeCalcdata(trace, 'x');
         origPos = ya.makeCalcdata(trace, 'y');
         pObj = alignPeriod(trace, ya, 'y', origPos);
         hasPeriod = !!trace.yperiodalignment;
+        pLetter = 'y';
     } else {
         size = ya.makeCalcdata(trace, 'y');
         origPos = xa.makeCalcdata(trace, 'x');
         pObj = alignPeriod(trace, xa, 'x', origPos);
         hasPeriod = !!trace.xperiodalignment;
+        pLetter = 'x';
     }
     pos = pObj.vals;
 
@@ -85,8 +87,8 @@ module.exports = function calc(gd, trace) {
 
         if(hasPeriod) {
             cd[i].orig_p = origPos[i]; // used by hover
-            cd[i].pEnd = pObj.ends[i];
-            cd[i].pStart = pObj.starts[i];
+            cd[i][pLetter + 'End'] = pObj.ends[i];
+            cd[i][pLetter + 'Start'] = pObj.starts[i];
         }
 
         if(trace.ids) {
diff --git a/test/jasmine/tests/hover_label_test.js b/test/jasmine/tests/hover_label_test.js
@@ -5433,14 +5433,14 @@ describe('hovermode: (x|y)unified', function() {
                 }
             })
             .then(function(gd) {
-                _hover(gd, { xpx: 50, ypx: 200 });
+                _hover(gd, { xpx: 100, ypx: 200 });
                 assertLabel({title: 'Jan', items: [
                     'bar : 1',
                     'one : 1',
                     'two : 1',
                 ]});
 
-                _hover(gd, { xpx: 350, ypx: 200 });
+                _hover(gd, { xpx: 300, ypx: 200 });
                 assertLabel({title: 'Feb', items: [
                     'bar : 2',
                     'one : 2',
@@ -5581,12 +5581,48 @@ describe('hovermode: (x|y)unified', function() {
 
                 Plotly.newPlot(gd, fig)
                 .then(function(gd) {
+                    _hover(gd, { xpx: 50, ypx: 200 });
+                    assertLabel({title: 'Jan 1, 1970', items: [
+                        'trace 0 : 11',
+                        'trace 1 : 1'
+                    ]});
+
                     _hover(gd, { xpx: 100, ypx: 200 });
                     assertLabel({title: 'Jan 1, 1970', items: [
                         'trace 0 : 11',
                         'trace 1 : 1'
                     ]});
 
+                    _hover(gd, { xpx: 150, ypx: 200 });
+                    assertLabel({title: 'Jul 1, 1970', items: [
+                        'trace 0 : 12',
+                        'trace 1 : 2'
+                    ]});
+
+                    _hover(gd, { xpx: 200, ypx: 200 });
+                    assertLabel({title: 'Jul 1, 1970', items: [
+                        'trace 0 : 12',
+                        'trace 1 : 2'
+                    ]});
+
+                    _hover(gd, { xpx: 250, ypx: 200 });
+                    assertLabel({title: 'Jul 1, 1970', items: [
+                        'trace 0 : 12',
+                        'trace 1 : 2'
+                    ]});
+
+                    _hover(gd, { xpx: 300, ypx: 200 });
+                    assertLabel({title: 'Jan 1, 1971', items: [
+                        'trace 0 : 13',
+                        'trace 1 : 3'
+                    ]});
+
+                    _hover(gd, { xpx: 350, ypx: 200 });
+                    assertLabel({title: 'Jan 1, 1971', items: [
+                        'trace 0 : 13',
+                        'trace 1 : 3'
+                    ]});
+
                     _hover(gd, { xpx: 400, ypx: 200 });
                     assertLabel({title: 'Jan 1, 1971', items: [
                         'trace 0 : 13',

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+ - Fix period positioned hover to work in different time zones as well as on grouped bars [[#5864](https://github.com/plotly/plotly.js/pull/5864)]`
Original file line number	Diff line number	Diff line change
`@@ -436,20 +436,12 @@ function setBarCenterAndWidth(pa, sieve) {`
`436`	`436`	`var barwidth = t.barwidth;`
`437`	`437`	`var barwidthIsArray = Array.isArray(barwidth);`
`438`	`438`
`439`		`- var trace = calcTrace[0].trace;`
`440`		`- var isPeriod = !!trace[pLetter + 'periodalignment'];`
`441`		`-`
`442`	`439`	`for(var j = 0; j < calcTrace.length; j++) {`
`443`	`440`	`var calcBar = calcTrace[j];`
`444`	`441`
`445`	`442`	`// store the actual bar width and position, for use by hover`
`446`	`443`	`var width = calcBar.w = barwidthIsArray ? barwidth[j] : barwidth;`
`447`	`444`	`calcBar[pLetter] = calcBar.p + (poffsetIsArray ? poffset[j] : poffset) + width / 2;`
`448`		`-`
`449`		`- if(isPeriod) {`
`450`		`- calcBar.wPeriod =`
`451`		`- calcBar.pEnd - calcBar.pStart;`
`452`		`- }`
`453`	`445`	`}`
`454`	`446`	`}`
`455`	`447`	`}`