From f49828a05fdf593097083842b31cbc2f8f777c11 Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Tue, 10 Sep 2019 08:46:36 -0400 Subject: [PATCH 1/6] introducing transitions for bar trace --- src/traces/bar/index.js | 1 + src/traces/bar/plot.js | 50 ++++++++++++++++++++----- test/image/baselines/animation_bar.png | Bin 0 -> 9118 bytes test/image/mocks/animation_bar.json | 22 +++++++++++ 4 files changed, 64 insertions(+), 9 deletions(-) create mode 100644 test/image/baselines/animation_bar.png create mode 100644 test/image/mocks/animation_bar.json diff --git a/src/traces/bar/index.js b/src/traces/bar/index.js index 5560603cee6..aa1a7a8feec 100644 --- a/src/traces/bar/index.js +++ b/src/traces/bar/index.js @@ -28,6 +28,7 @@ module.exports = { name: 'bar', basePlotModule: require('../../plots/cartesian'), categories: ['bar-like', 'cartesian', 'svg', 'bar', 'oriented', 'errorBarsOK', 'showLegend', 'zoomScale'], + animatable: true, meta: { description: [ 'The data visualized by the span of the bars is set in `y`', diff --git a/src/traces/bar/plot.js b/src/traces/bar/plot.js index 11d8ad24f68..46bf576ea4d 100644 --- a/src/traces/bar/plot.js +++ b/src/traces/bar/plot.js @@ -51,7 +51,28 @@ function getXY(di, xa, ya, isHorizontal) { return isHorizontal ? [s, p] : [p, s]; } -function plot(gd, plotinfo, cdModule, traceLayer, opts) { +function transition(selection, opts, makeOnCompleteCallback) { + if(hasTransition(opts)) { + var onComplete; + if(makeOnCompleteCallback) { + onComplete = makeOnCompleteCallback(); + } + return selection + .transition() + .duration(opts.duration) + .ease(opts.easing) + .each('end', function() { onComplete && onComplete(); }) + .each('interrupt', function() { onComplete && onComplete(); }); + } else { + return selection; + } +} + +function hasTransition(transitionOpts) { + return transitionOpts && transitionOpts.duration > 0; +} + +function plot(gd, plotinfo, cdModule, traceLayer, opts, makeOnCompleteCallback) { var xa = plotinfo.xaxis; var ya = plotinfo.yaxis; var fullLayout = gd._fullLayout; @@ -96,7 +117,6 @@ function plot(gd, plotinfo, cdModule, traceLayer, opts) { // clipped xf/yf (2nd arg true): non-positive // log values go off-screen by plotwidth // so you see them continue if you drag the plot - var xy = getXY(di, xa, ya, isHorizontal); var x0 = xy[0][0]; @@ -118,6 +138,9 @@ function plot(gd, plotinfo, cdModule, traceLayer, opts) { } di.isBlank = isBlank; + if(isBlank && isHorizontal) x1 = x0; + if(isBlank && !isHorizontal) y1 = y0; + // in waterfall mode `between` we need to adjust bar end points to match the connector width if(adjustPixel) { if(isHorizontal) { @@ -178,12 +201,19 @@ function plot(gd, plotinfo, cdModule, traceLayer, opts) { y1 = fixpx(y1, y0); } - Lib.ensureSingle(bar, 'path') + var sel = transition(Lib.ensureSingle(bar, 'path'), opts, makeOnCompleteCallback); + sel .style('vector-effect', 'non-scaling-stroke') - .attr('d', isBlank ? 'M0,0Z' : 'M' + x0 + ',' + y0 + 'V' + y1 + 'H' + x1 + 'V' + y0 + 'Z') + .attr('display', function() { return isBlank ? 'none' : null;}) + .attr('d', 'M' + x0 + ',' + y0 + 'V' + y1 + 'H' + x1 + 'V' + y0 + 'Z') .call(Drawing.setClipUrl, plotinfo.layerClipId, gd); - appendBarText(gd, plotinfo, bar, cd, i, x0, x1, y0, y1, opts); + if(hasTransition(opts)) { + var styleFns = Drawing.makePointStyleFns(trace); + Drawing.singlePointStyle(di, sel, trace, styleFns, gd); + } + + appendBarText(gd, plotinfo, bar, cd, i, x0, x1, y0, y1, opts, makeOnCompleteCallback); if(plotinfo.layerClipId) { Drawing.hideOutsideRangePoint(di, bar.select('text'), xa, ya, trace.xcalendar, trace.ycalendar); @@ -197,10 +227,10 @@ function plot(gd, plotinfo, cdModule, traceLayer, opts) { }); // error bars are on the top - Registry.getComponentMethod('errorbars', 'plot')(gd, bartraces, plotinfo); + Registry.getComponentMethod('errorbars', 'plot')(gd, bartraces, plotinfo, opts); } -function appendBarText(gd, plotinfo, bar, calcTrace, i, x0, x1, y0, y1, opts) { +function appendBarText(gd, plotinfo, bar, calcTrace, i, x0, x1, y0, y1, opts, makeOnCompleteCallback) { var xa = plotinfo.xaxis; var ya = plotinfo.yaxis; @@ -212,7 +242,6 @@ function appendBarText(gd, plotinfo, bar, calcTrace, i, x0, x1, y0, y1, opts) { .text(text) .attr({ 'class': 'bartext bartext-' + textPosition, - transform: '', 'text-anchor': 'middle', // prohibit tex interpretation until we can handle // tex and regular text together @@ -325,9 +354,12 @@ function appendBarText(gd, plotinfo, bar, calcTrace, i, x0, x1, y0, y1, opts) { (textPosition === 'outside') ? outsideTextFont : insideTextFont); + var currentTransform = textSelection.attr('transform'); + textSelection.attr('transform', ''); textBB = Drawing.bBox(textSelection.node()), textWidth = textBB.width, textHeight = textBB.height; + textSelection.attr('transform', currentTransform); if(textWidth <= 0 || textHeight <= 0) { textSelection.remove(); @@ -360,7 +392,7 @@ function appendBarText(gd, plotinfo, bar, calcTrace, i, x0, x1, y0, y1, opts) { })); } - textSelection.attr('transform', transform); + transition(textSelection, opts, makeOnCompleteCallback).attr('transform', transform); } function getRotateFromAngle(angle) { diff --git a/test/image/baselines/animation_bar.png b/test/image/baselines/animation_bar.png new file mode 100644 index 0000000000000000000000000000000000000000..262aaed7be10a29a01f01f8fc2ae371633f5223d GIT binary patch literal 9118 zcmeHNXH=8hwoO8iASeMuL=hrg=^zM52?EkViYS6KDN>}VAU!AuNDEzxL=jMwA}A#S z0fHz+QE5_?00t20J=EMC&$;Eiao)c--gx)j^CM#<-&o&Td#}CMoOA7+m~%$j`}Q8* z3xPoP>FQ{jLLg8q?St3@o?sf}-$NiMh_0r(`7N8-Z!A|W*6ZIbdS#uzAszD})%mLx zE44-Op!Kw#y~hht`T>puoE%h-ey#^GVW%p0r&fmp<%Zn&EQ*ybT)b_j%~+DK>U<_z zMxNX3#O205vA#m={8I*(9!+mG=1_-k*KpC%x7p8DD)4S9R$h` zg}sT6SO4V+j!>c5q3i_Em_y~#2$}^3@`&M=>o_<9$uGa>7|jBSTA|rtFi8-o!<)CL zU!GtQJbw-HuT1{6C;y~ySn<~<6Y6%>hJ44~Fiy?RrX?q{SrUTr%3CupkGW}UN1Z%* zvNit5DaUQfV#UpAi{qlAvM?7~`HsXwpnQBgJ3GEhWZjLvYJ%Y7v+>EDokjtREt~~- zyd_W2jI`zJ8%4#JYb^?maXfe5M>26bPIf*ZtU$JC1qliltNr5q`SWelWepNvN-w8q zMD1@QglrM{@4U!V3nQGAlhZXZNn%y{g(1AaoZ%T|zy%j+%r#%PoKUyaF51=CcTm;G zWAK&hNUNNCU+d4_=LLCr#3&v+mfvAOCc31s@T}_gg2u)eXM@O4)wLAe6d_%G{a9;4 zuwpRN^sxK)xA7H@ZETtH*RuW)>rn8+-QBHu{mYjh%na9C?Ji|Bw1Rv&cBG5bY41^g z>G@45Q^x5Yi%KKU?}$Za^-$YrGbVc3#~(dyu{QTjl!1kZIdNusI%Q-ziC;TG$H%Aq zpD~cE8!O$$1y7y`Z{PD=*!=N0ktm!nB8z>J8>p8qZY5M%SxHACDg~_-_k8)XbuMh} z7g{nQQG82n!Z@9UH=npr%KV_DjhHQU^NU8m=!MbGgwxW&0uY(yyyLIb&;fjfD(Uk9m#P3aRn3!BXkfe_pee%-u^0fO6aeLvb6UTG zDga>Se)v*=w(xf#&@l~;0Xo`u0K-0*&gB1n=dUM{KdMoK-%eRsS;=L%c*2pbG{$)8 zz>do0NS#Gt?VvD$rj~M*${>59SyYL@cUJU>J(&Ny5OvHgYwN1JXc*W~@B`}82M|bc z%Ly$=IGD_04GRL97~9c@1VjZ*#_(Nz0E6ZUcGMwIjqK5rDrn3t8@s9~%oS)Znwt}S zJpiU@mVsye^(MoA`=$%(PTgp8g$6OVwXV0IrlhollWBP9x`LJ#v(piX=SAtwKsb&eRUh#iIIF{x0zKGFL5v9=SrHL2q2XFf&0nf^w0olDOQ zF!=?hIl{211HjC*y0lbcp!`bINqPCZYWs+e@9#_dPWOptD){6TTjNdJJ382HeP9BM zf@5%2ok|FF3BBP-A{-5!koFo@4w%DF5^-y5Ywgjj{P|^N#zwiy$D4SCm4hkXl$Rs2 zDftA#@`L$^CufwwNZ2X0zopLWY}z$}Ezwok$TzD@#aNX`rAmxDM>z z!<&2I(dlB#d>AySs9Rqdz$R>Qaz_9$12K^x)`(Ld*AX8-!L4*YA7wd>vz(vdejU0w z-`VGE4@Z9DK%sAhL_&_CQ?25rSiObN7?Eyv6g=8DR$2wt398j85EcP3@suvqZp?&0 z^9+nJNL0J{+ZBWY*aoRnj7XR!zU4j-)CFvQ85QUmj7asNZvGJi5WG+rjX;hcyb=h{ z0TQfz1&)B`eRVKM#K=Nnnwh!(Wq4e1e)nCTFyxYROD(=s zsdqwDv>E4jGDs;5?(Fm+gJW(Bye+~n|TfIUlJHNkw z%vg0J|5bH$N9pBSa{Q@b?Cx&pzc8bOuu%?{iBqDr^TBEUv}}-dKDoUN$FM+NlUcR0 zWnB`uRRYal*Vc}IdZba|MItP;qRvfx#7P<&8k(A$r&f}|x+PcYDnPQ3`$xp$4Ou}@=OrMNI1QZwmM7Y-e^e0`7Z>;4SQ=jtFAgdLnxH*dP~Q(L{ywUJCjeoA34 zyRq`I-rioi@tK*K)cTmWRhD(#W_>Q+@ZFb11uh>$?f9`8I&SSXFQo^QAgr1&$v>{t1sLK4p1?>}u0UT<& z9oT;c6$yj4e#@)hnyXzJbT4bG@%irfv!`gCzV#l5C?GK#J$?NsYX^X|T)y2>pwNJ1 z@@E3uQBX@utE;Pvucf6080TrllKacA&;d*-5(mDS3v3k*hMxrB%3}V!yj(iC>(KVW zz8uBte?qhTy_x@7`T}P#D}UtGl9Cc%%EG9(Bl1M1jqRY1QslLyTC-4U!>+i0iZnY+ z@gSnq-DU!CP6Q+X>)&*n-gl%C%B2$cYWq7cwe_jm__+M?&s$PUT^+K@{q$A~=k0(y zf~RtyMbq(@YyH{)(m3_Muphk;42qk^lcx25)yuyHCExj>woAD(KnSUQpCzY10a$4v zB_UzI>(#5+Bd0uD0UbI`_m!AhSR5?63Ydqj$V34VegH6!f9%>^?R>j?S-lWwsZxMw z+YfJjaN6H-s;i*FZ}w8AYRKyx1)nn#)-|Zp{&R8Wo}iv$dPKM*(0ge>uQmPiXL4R1 z9}u%NFb{{|Z%id~E8WlmmCVUA0Hh+&Y8f<*NAZvZueSB^Z*zygqH&&iX?eK`s8I%3 z{+}|QR^MwNnsoYvMj>$16KIShBX%T)4D{fr)x~uiFs)QU{q$BqljMmH*5Fsf$<~}`?9?%(Eq~)@U9!x+VlcO4P2_#0?s>*{)p#n6ReI-`HW`zbx1Og%M zo6Kl|*+#b^%fJ-FP1)r@s)yZkcNCf|h*mm%+G&2c-gjZ736QxIkghaf7x|mlOw!1MYn``Xu?w;3~kx~wnUPu8er@vd( z&NKk6csSQ;Qg-?Q>jGDfgbFloT2w;PZoEV*E0;-uEoD$$Vk;41BI@oSBm|V<=n~9g z_)I*9$Bin&|GDK$dKdqDO(a5B1!R36ZS1pB;^0_zcLfcde>M(cH=oN{x8M^;p!r?2HigT|5d zkpathn9)4_d-iD8FTyU|#;Dhov_>faHp-}p2QJ$L(3l@pyuhtKO_m*{iL<#&agZ3h zhJa;f{H?{YMv_j}?>gYn@n1M*{A&N0c+UzU@>wrCJ|_bb^APXQvKN$F24HxEHYp78 zYuKuET)wr!o)9nlsdRu}BelOZ*B9S5bZtI$MF3HFCbsD48EQc_wI_=&iX9ABYVv(1 z|CP6KNuW_^dlSR%G~S+OYGcEb08WNF6qk*V$MYl58H4_J7^v{C~TaY+Sg~H2pVmLB^qYAJl$TL=Pw?aKL#Mn zz7oIBtKLg)*xCC_ITaC=XjRwYeE4~J&_rmPoAOQuo~oy(R~)Y8;Lc`O^F3IT@)@Eq%9@<(QqF9_iuWM$O634A5GksOU4kqB9 z;;7KLwnv9Lu!=LBU9uO0CKt4;sHm`Q%LXhtPR-9}OiVZ{hV2GZN{U4!CG2XM`U`lC zQ6@^t1Mm9xcvtvO?#>{vVsbTpvsp8PmHMSN^(UI|?xRtMo}poUYFe7sv)qm&cKPh) zs;z4uFHE~t?8*9B`kAgoNR0WICa>dASj@0B%qRgA@9mBw510D*484M&wJZE+zBGl; z3+brL8BG*s-27<9|T9pAUt2qze{-TYEu2D zTRjs>I7?nv5?)E%x8{&{8;1g;lt#&@keArw{H!+zWJP15{Hfxf6I|=KGQ5K!@!;w9|kw1 z(jjf8<3Lumb`|J5%??(|glyhW^cjn=uJKm)oBPUDq*ep0*|A;N=u>B<`3qsz%y6qq_2$ zf1N@y3o2eq*Ni%G5GdQD0~boIlfKpZ zcdSl-aRMhRqeGVCss@ZLa(r`AsP2Ru|)sOtrBrtg0<=fnfszC42IZ}h|ZQF?8)!qxeeTzP0D6~!BlpxnB&z$XL95uB$ilu`@-Sr=+x|W?d z^kE~nGe?nMZEN5oIZAE&k@C(;_vpM){rYgwQg2zP+@!noBP(!9UeYl5v&qnF>`e?? z@V*dL_!#qaON0n-jJ zT#q<;>QqNr*v|CAot3iSanVt$C*X(e247TEJ-DQ>=;7TSVK^Y}O@Kh?ENTA*7_~yM z3mMdSxdg4vFj51h8rYNTNA9T6G3Kh?w&mHUs=b%c`&Klo(JOR08~AkpQrh;yzL{9I zyr+SI*dYm4zAUif2)GnS$%EEH-t8dm%6%K$-ETh=u2l!$!R-m!;j!~EVAA-P`Cpm& e|Gx<+YMVT@{Jp9jX5fD}5M3=J&C)Y=xBmuHh`aXy literal 0 HcmV?d00001 diff --git a/test/image/mocks/animation_bar.json b/test/image/mocks/animation_bar.json new file mode 100644 index 00000000000..60955f86ff7 --- /dev/null +++ b/test/image/mocks/animation_bar.json @@ -0,0 +1,22 @@ +{ + "data": [{ + "type": "bar", + "x": ["A", "B", "C"], + "y": [24, 5, 8], + "error_y": {"array": [3, 2, 1]} + }], + "layout": { + "width": 400, + "height": 400, + "yaxis": {"range": [0, 30]} + }, + "frames": [ + {"data": [{"y": [12, 15, 10]}]}, + {"data": [{"error_y": {"array": [5, 4, 1]}}]}, + {"data": [{"marker": {"color": ["red", "blue", "green"]}}]}, + {"data": [{"width": 0.25}]}, + {"data": [{"marker": {"line": {"width": 10}}}]}, + {"data": [{"marker": {"line": {"color": ["orange", "yellow", "blue"]}}}]}, + {"layout": {"yaxis": {"range": [0, 20]}}} + ] +} From bda810fd90be1ccb6993ec856d8ff59f007aa772 Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Tue, 10 Sep 2019 08:46:51 -0400 Subject: [PATCH 2/6] introducing checkTransition to :lock: down bar transitions --- test/jasmine/assets/check_transitions.js | 115 +++++++++++++++++++++++ test/jasmine/tests/bar_test.js | 94 ++++++++++++++++++ test/jasmine/tests/transition_test.js | 6 +- 3 files changed, 212 insertions(+), 3 deletions(-) create mode 100644 test/jasmine/assets/check_transitions.js diff --git a/test/jasmine/assets/check_transitions.js b/test/jasmine/assets/check_transitions.js new file mode 100644 index 00000000000..4040b212899 --- /dev/null +++ b/test/jasmine/assets/check_transitions.js @@ -0,0 +1,115 @@ +'use strict'; + +var Plotly = require('@lib/index'); +var Lib = require('@src/lib'); +var d3 = require('d3'); +var delay = require('./delay.js'); + +var reNumbers = /([\d\.]+)/gm; + +function promiseSerial(funcs, wait) { + return funcs.reduce(function(promise, func) { + return promise.then(function(result) { + return func().then(Array.prototype.concat.bind(result)).then(delay(wait)); + }); + }, Promise.resolve([])); +} + +function clockTick(currentNow, milliseconds) { + Date.now = function() { + return currentNow + milliseconds; + }; + d3.timer.flush(); +} + +// Using the methodology from http://eng.wealthfront.com/2017/10/26/testing-d3-transitions/ +module.exports = function checkTransition(gd, mock, animateOpts, transitionOpts, tests) { + if(!transitionOpts) { + transitionOpts = { + transition: { + duration: 500, + easing: 'linear' + }, + frame: { + duration: 500 + } + }; + } + // Prepare chain + var now = Date.now; + var startTime; + var currentTime; + var p = [ + function() { + return Plotly.newPlot(gd, mock) + .then(function() { + // Check initial states if present + for(var i = 0; i < tests.length; i++) { + if(tests[i][0] === 0) assert(tests[i]); + } + }); + }, + function() { + // Hijack Date.now + startTime = Date.now(); + currentTime = 0; + clockTick(startTime, 0); + Plotly.animate(gd, animateOpts, transitionOpts); + return Promise.resolve(true); + } + ]; + + var checkTests = tests.map(function(test) { + return function() { + if(test[0] === 0) return Promise.resolve(true); + if(test[0] !== currentTime) { + clockTick(startTime, test[0]); + currentTime = test[0]; + } + return assert(test); + }; + }); + + // Run all tasks + return promiseSerial(p.concat(checkTests)) + .then(function() { + Date.now = now; + }); +}; + +// A test array is made of +// [ms since start of transition, selector, (attr|style), name of attribute, array of values to be found] +// Ex.: [0, '.point path', 'style', 'fill', ['rgb(31, 119, 180)', 'rgb(31, 119, 180)', 'rgb(31, 119, 180)']] +function assert(test) { + var msg = 'at ' + test[0] + 'ms, selection ' + test[1] + ' has ' + test[3]; + var cur = []; + Plotly.d3.selectAll(test[1]).each(function(d, i) { + if(test[2] === 'style') cur[i] = this.style[test[3]]; + if(test[2] === 'attr') cur[i] = Plotly.d3.select(this).attr(test[3]); + }); + switch(test[3]) { + case 'd': + assertAttr(cur, test[4], round, msg); + break; + case 'transform': + assertAttr(cur, test[4], round, msg); + break; + default: + assertAttr(cur, test[4], Lib.identity, msg); + } + return Promise.resolve(true); +} + +function assertAttr(A, B, cb, msg) { + var a = cb(A); + var b = cb(B); + expect(a).withContext(msg + ' equal to ' + JSON.stringify(b)).toEqual(b); +} + +function round(str) { + return str.map(function(cur) { + return cur.replace(reNumbers, function(match) { + return Math.round(match); + }); + }); +} diff --git a/test/jasmine/tests/bar_test.js b/test/jasmine/tests/bar_test.js index 32d45cffb7f..8d26805782a 100644 --- a/test/jasmine/tests/bar_test.js +++ b/test/jasmine/tests/bar_test.js @@ -26,6 +26,7 @@ var assertClip = customAssertions.assertClip; var assertNodeDisplay = customAssertions.assertNodeDisplay; var assertHoverLabelContent = customAssertions.assertHoverLabelContent; var checkTextTemplate = require('../assets/check_texttemplate'); +var checkTransition = require('../assets/check_transitions'); var Fx = require('@src/components/fx'); var d3 = require('d3'); @@ -2548,3 +2549,96 @@ function assertTraceField(calcData, prop, expectation) { expect(values).toBeCloseToArray(expectation, undefined, '(field ' + prop + ')'); } + +describe('bar tweening', function() { + var gd; + var mock = { + 'data': [{ + 'type': 'bar', + 'x': ['A', 'B', 'C'], + 'text': ['A', 'B', 'C'], + 'textposition': 'inside', + 'y': [24, 5, 8], + 'error_y': {'array': [3, 2, 1]} + }] + }; + var transitionOpts = false; // use default + + beforeEach(function() { + gd = createGraphDiv(); + }); + + afterEach(function() { + Plotly.purge(gd); + destroyGraphDiv(); + }); + + it('for bar fill color', function(done) { + var tests = [ + [0, '.point path', 'style', 'fill', ['rgb(31, 119, 180)', 'rgb(31, 119, 180)', 'rgb(31, 119, 180)']], + [100, '.point path', 'style', 'fill', ['rgb(76, 95, 144)', 'rgb(25, 146, 144)', 'rgb(25, 95, 195)']], + [300, '.point path', 'style', 'fill', ['rgb(165, 48, 72)', 'rgb(12, 201, 72)', 'rgb(12, 48, 225)']], + [500, '.point path', 'style', 'fill', ['rgb(255, 0, 0)', 'rgb(0, 255, 0)', 'rgb(0, 0, 255)']] + ]; + var animateOpts = {'data': [{'marker': {'color': ['rgb(255, 0, 0)', 'rgb(0, 255, 0)', 'rgb(0, 0, 255)']}}]}; + + checkTransition(gd, mock, animateOpts, transitionOpts, tests) + .catch(failTest) + .then(done); + }); + + it('for bar height and and text position', function(done) { + var tests = [ + [0, '.point path', 'attr', 'd', ['M18,270V42H162V270Z', 'M198,270V222.5H342V270Z', 'M378,270V194H522V270Z']], + [0, 'text.bartext', 'attr', 'transform', ['translate(90 56)', 'translate(270 236.5)', 'translate(450 208)']], + [100, '.point path', 'attr', 'd', ['M18,270V78.1H162V270Z', 'M198,270V186.4H342V270Z', 'M378,270V186.40000000000003H522V270Z']], + [300, '.point path', 'attr', 'd', ['M18,270V150.3H162V270Z', 'M198,270V114.2H342V270Z', 'M378,270V171.2H522V270Z']], + [300, 'text.bartext', 'attr', 'transform', ['translate(90,164.3)', 'translate(270,128.20000000000002)', 'translate(450,185.2)']], + [500, '.point path', 'attr', 'd', ['M18,270V222.5H162V270Z', 'M198,270V42H342V270Z', 'M378,270V156H522V270Z']], + [600, '.point path', 'attr', 'd', ['M18,270V222.5H162V270Z', 'M198,270V42H342V270Z', 'M378,270V156H522V270Z']], + [600, 'text.bartext', 'attr', 'transform', ['translate(90 236.5)', 'translate(270 56)', 'translate(450 170)']], + ]; + var animateOpts = {data: [{y: [5, 24, 12]}]}; + + checkTransition(gd, mock, animateOpts, transitionOpts, tests) + .catch(failTest) + .then(done); + }); + + it('for bar width', function(done) { + var tests = [ + [0, '.point path', 'attr', 'd', ['M54,270V13.5H486V270Z']], + [250, '.point path', 'attr', 'd', ['M94.5,270V13.5H445.5V270Z']], + [500, '.point path', 'attr', 'd', ['M135,270V13.5H405V270Z']] + ]; + var animateOpts = {data: [{width: 0.5}]}; + + checkTransition(gd, { + data: [{ + type: 'bar', + y: [5] + }]}, + animateOpts, transitionOpts, tests) + .catch(failTest) + .then(done); + }); + + it('for error bars', function(done) { + var tests = [ + [0, 'path.yerror', 'attr', 'd', ['M266,13.5h8m-4,0V99m-4,0h8']], + [250, 'path.yerror', 'attr', 'd', ['M266,-18.56h8m-4,0V131.065m-4,0h8']], + [500, 'path.yerror', 'attr', 'd', ['M266,-50.62h8m-4,0V163.13m-4,0h8']] + ]; + var animateOpts = {data: [{error_y: {value: 50}}]}; + + checkTransition(gd, { + data: [{ + type: 'bar', + y: [2], + error_y: {value: 20} + }]}, + animateOpts, transitionOpts, tests) + .catch(failTest) + .then(done); + }); +}); diff --git a/test/jasmine/tests/transition_test.js b/test/jasmine/tests/transition_test.js index 96fdb32857c..9097392ba06 100644 --- a/test/jasmine/tests/transition_test.js +++ b/test/jasmine/tests/transition_test.js @@ -435,7 +435,7 @@ describe('Plotly.react transitions:', function() { addSpies(); var trace = { - type: 'bar', + type: 'violin', y: [1], marker: {line: {width: 1}} }; @@ -444,8 +444,8 @@ describe('Plotly.react transitions:', function() { var layout = {transition: {duration: 10}}; // sanity check that this test actually tests what was intended - var Bar = Registry.modules.bar._module; - if(Bar.animatable || Bar.attributes.marker.line.width.anim !== true) { + var Violin = Registry.modules.violin._module; + if(Violin.animatable || Violin.attributes.marker.line.width.anim !== true) { fail('Test no longer tests its indented code path:' + ' This test is meant to test that Plotly.react with' + ' *anim:true* attributes in *animatable:false* modules' + From 495d036d44636923d9e6d55d7261629c8c7a81ac Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Tue, 10 Sep 2019 11:37:05 -0400 Subject: [PATCH 3/6] bar: properly transition to blank bars --- src/traces/bar/plot.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/traces/bar/plot.js b/src/traces/bar/plot.js index 46bf576ea4d..c754cb58791 100644 --- a/src/traces/bar/plot.js +++ b/src/traces/bar/plot.js @@ -204,7 +204,10 @@ function plot(gd, plotinfo, cdModule, traceLayer, opts, makeOnCompleteCallback) var sel = transition(Lib.ensureSingle(bar, 'path'), opts, makeOnCompleteCallback); sel .style('vector-effect', 'non-scaling-stroke') - .attr('display', function() { return isBlank ? 'none' : null;}) + .attr('display', function() { + if(!isBlank) return null; + if(isBlank && !hasTransition(opts)) return 'none'; + }) .attr('d', 'M' + x0 + ',' + y0 + 'V' + y1 + 'H' + x1 + 'V' + y0 + 'Z') .call(Drawing.setClipUrl, plotinfo.layerClipId, gd); From cc871cdb5292bf7c3e75431d3936c2229966cb37 Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Tue, 10 Sep 2019 13:51:00 -0400 Subject: [PATCH 4/6] bar: test marker.line.(width|color) and horizontal bar transitions --- test/jasmine/assets/check_transitions.js | 9 +++-- test/jasmine/tests/bar_test.js | 46 ++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/test/jasmine/assets/check_transitions.js b/test/jasmine/assets/check_transitions.js index 4040b212899..ff3c3161d2f 100644 --- a/test/jasmine/assets/check_transitions.js +++ b/test/jasmine/assets/check_transitions.js @@ -72,6 +72,9 @@ module.exports = function checkTransition(gd, mock, animateOpts, transitionOpts, // Run all tasks return promiseSerial(p.concat(checkTests)) + .catch(function() { + Date.now = now; + }) .then(function() { Date.now = now; }); @@ -83,9 +86,9 @@ module.exports = function checkTransition(gd, mock, animateOpts, transitionOpts, function assert(test) { var msg = 'at ' + test[0] + 'ms, selection ' + test[1] + ' has ' + test[3]; var cur = []; - Plotly.d3.selectAll(test[1]).each(function(d, i) { + d3.selectAll(test[1]).each(function(d, i) { if(test[2] === 'style') cur[i] = this.style[test[3]]; - if(test[2] === 'attr') cur[i] = Plotly.d3.select(this).attr(test[3]); + if(test[2] === 'attr') cur[i] = d3.select(this).attr(test[3]); }); switch(test[3]) { case 'd': @@ -103,7 +106,7 @@ function assert(test) { function assertAttr(A, B, cb, msg) { var a = cb(A); var b = cb(B); - expect(a).withContext(msg + ' equal to ' + JSON.stringify(b)).toEqual(b); + expect(a).withContext(msg + ' equal to ' + JSON.stringify(a)).toEqual(b); } function round(str) { diff --git a/test/jasmine/tests/bar_test.js b/test/jasmine/tests/bar_test.js index 8d26805782a..2b36d728017 100644 --- a/test/jasmine/tests/bar_test.js +++ b/test/jasmine/tests/bar_test.js @@ -2587,7 +2587,7 @@ describe('bar tweening', function() { .then(done); }); - it('for bar height and and text position', function(done) { + it('for vertical bar height and text position', function(done) { var tests = [ [0, '.point path', 'attr', 'd', ['M18,270V42H162V270Z', 'M198,270V222.5H342V270Z', 'M378,270V194H522V270Z']], [0, 'text.bartext', 'attr', 'transform', ['translate(90 56)', 'translate(270 236.5)', 'translate(450 208)']], @@ -2605,7 +2605,7 @@ describe('bar tweening', function() { .then(done); }); - it('for bar width', function(done) { + it('for vertical bar width', function(done) { var tests = [ [0, '.point path', 'attr', 'd', ['M54,270V13.5H486V270Z']], [250, '.point path', 'attr', 'd', ['M94.5,270V13.5H445.5V270Z']], @@ -2623,6 +2623,48 @@ describe('bar tweening', function() { .then(done); }); + it('for horizontal bar length and text position', function(done) { + var mockCopy = Lib.extendDeep({}, mock); + mockCopy.data[0].orientation = 'h'; + mockCopy.data[0].x = mock.data[0].y.slice(); + mockCopy.data[0].y = mock.data[0].x.slice(); + var tests = [ + [0, '.point path', 'attr', 'd', ['M0,261V189H107V261Z', 'M0,171V99H513V171Z', 'M0,81V9H257V81Z']], + [0, 'text.bartext', 'attr', 'transform', ['translate(100 229)', 'translate(506 139)', 'translate(249 49)']], + [150, '.point path', 'attr', 'd', ['M0,261V189H171V261Z', 'M0,171V99H455V171Z', 'M0,81V9H276V81Z']], + [300, '.point path', 'attr', 'd', ['M0,261V189H235V261Z', 'M0,171V99H398V171Z', 'M0,81V9H295V81Z']], + [300, 'text.bartext', 'attr', 'transform', ['translate(228,229)', 'translate(391,139)', 'translate(287,49)']], + [450, '.point path', 'attr', 'd', ['M0,261V189H299V261Z', 'M0,171V99H340V171Z', 'M0,81V9H314V81Z']], + [600, '.point path', 'attr', 'd', ['M0,261V189H321V261Z', 'M0,171V99H321V171Z', 'M0,81V9H321V81Z']], + [600, 'text.bartext', 'attr', 'transform', ['translate(314 229)', 'translate(314 139)', 'translate(313 49)']] + ]; + var animateOpts = {data: [{x: [15, 15, 15]}]}; + + checkTransition(gd, mockCopy, animateOpts, transitionOpts, tests) + .catch(failTest) + .then(done); + }); + + it('for bar line width and color', function(done) { + var tests = [ + [0, '.point path', 'style', 'stroke', ['', '', '']], + [0, '.point path', 'style', 'stroke-width', ['0px', '0px', '0px']], + [150, '.point path', 'style', 'stroke', ['rgb(77, 0, 0)', 'rgb(0, 77, 0)', 'rgb(0, 0, 77)']], + [150, '.point path', 'style', 'stroke-width', ['6px', '6px', '6px']], + [300, '.point path', 'style', 'stroke', ['rgb(153, 0, 0)', 'rgb(0, 153, 0)', 'rgb(0, 0, 153)']], + [300, '.point path', 'style', 'stroke-width', ['12px', '12px', '12px']], + [450, '.point path', 'style', 'stroke', ['rgb(230, 0, 0)', 'rgb(0, 230, 0)', 'rgb(0, 0, 230)']], + [450, '.point path', 'style', 'stroke-width', ['18px', '18px', '18px']], + [600, '.point path', 'style', 'stroke', ['rgb(255, 0, 0)', 'rgb(0, 255, 0)', 'rgb(0, 0, 255)']], + [600, '.point path', 'style', 'stroke-width', ['20px', '20px', '20px']] + ]; + var animateOpts = {'data': [{'marker': {'line': {'width': 20, 'color': ['rgb(255, 0, 0)', 'rgb(0, 255, 0)', 'rgb(0, 0, 255)']}}}]}; + + checkTransition(gd, mock, animateOpts, transitionOpts, tests) + .catch(failTest) + .then(done); + }); + it('for error bars', function(done) { var tests = [ [0, 'path.yerror', 'attr', 'd', ['M266,13.5h8m-4,0V99m-4,0h8']], From 16c88f8ff93b401ec21d52a7d6a9cd07e139a96c Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Tue, 10 Sep 2019 15:53:48 -0400 Subject: [PATCH 5/6] checkTransition: add tolerance to transform check --- test/jasmine/assets/check_transitions.js | 29 ++++++++++++++++++------ 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/test/jasmine/assets/check_transitions.js b/test/jasmine/assets/check_transitions.js index ff3c3161d2f..35f4df88bb2 100644 --- a/test/jasmine/assets/check_transitions.js +++ b/test/jasmine/assets/check_transitions.js @@ -72,8 +72,9 @@ module.exports = function checkTransition(gd, mock, animateOpts, transitionOpts, // Run all tasks return promiseSerial(p.concat(checkTests)) - .catch(function() { + .catch(function(err) { Date.now = now; + return Promise.reject(err); }) .then(function() { Date.now = now; @@ -92,25 +93,39 @@ function assert(test) { }); switch(test[3]) { case 'd': - assertAttr(cur, test[4], round, msg); + assertEqual(cur, test[4], round, msg); break; case 'transform': - assertAttr(cur, test[4], round, msg); + assertCloseTo(cur, test[4], 3, extractNumbers, msg); break; default: - assertAttr(cur, test[4], Lib.identity, msg); + assertEqual(cur, test[4], Lib.identity, msg); } return Promise.resolve(true); } -function assertAttr(A, B, cb, msg) { +function assertEqual(A, B, cb, msg) { var a = cb(A); var b = cb(B); expect(a).withContext(msg + ' equal to ' + JSON.stringify(a)).toEqual(b); } -function round(str) { - return str.map(function(cur) { +function assertCloseTo(A, B, tolerance, cb, msg) { + var a = cb(A).flat(); + var b = cb(B).flat(); + expect(a).withContext(msg + ' equal to ' + JSON.stringify(A)).toBeWithinArray(b, tolerance); +} + +function extractNumbers(array) { + return array.map(function(d) { + return d.match(reNumbers).map(function(n) { + return parseFloat(n); + }); + }); +} + +function round(array) { + return array.map(function(cur) { return cur.replace(reNumbers, function(match) { return Math.round(match); }); From 709ee2fb8126c07367151cdc2d617b37f45d85c5 Mon Sep 17 00:00:00 2001 From: Antoine Roy-Gobeil Date: Tue, 10 Sep 2019 16:02:06 -0400 Subject: [PATCH 6/6] bar: set stroke-width to 0 when bar is blank --- src/components/drawing/index.js | 2 +- src/traces/bar/plot.js | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/components/drawing/index.js b/src/components/drawing/index.js index 7343d3bb58b..eafda1a4331 100644 --- a/src/components/drawing/index.js +++ b/src/components/drawing/index.js @@ -442,7 +442,7 @@ drawing.singlePointStyle = function(d, sel, trace, fns, gd) { fill: 'none' }); } else { - sel.style('stroke-width', lineWidth + 'px'); + sel.style('stroke-width', (d.isBlank ? 0 : lineWidth) + 'px'); var markerGradient = marker.gradient; diff --git a/src/traces/bar/plot.js b/src/traces/bar/plot.js index c754cb58791..0ba64571ec8 100644 --- a/src/traces/bar/plot.js +++ b/src/traces/bar/plot.js @@ -142,7 +142,7 @@ function plot(gd, plotinfo, cdModule, traceLayer, opts, makeOnCompleteCallback) if(isBlank && !isHorizontal) y1 = y0; // in waterfall mode `between` we need to adjust bar end points to match the connector width - if(adjustPixel) { + if(adjustPixel && !isBlank) { if(isHorizontal) { x0 -= dirSign(x0, x1) * adjustPixel; x1 += dirSign(x0, x1) * adjustPixel; @@ -204,10 +204,6 @@ function plot(gd, plotinfo, cdModule, traceLayer, opts, makeOnCompleteCallback) var sel = transition(Lib.ensureSingle(bar, 'path'), opts, makeOnCompleteCallback); sel .style('vector-effect', 'non-scaling-stroke') - .attr('display', function() { - if(!isBlank) return null; - if(isBlank && !hasTransition(opts)) return 'none'; - }) .attr('d', 'M' + x0 + ',' + y0 + 'V' + y1 + 'H' + x1 + 'V' + y0 + 'Z') .call(Drawing.setClipUrl, plotinfo.layerClipId, gd);