base fractional colorbar size on post-expanded-margin plot size

and fix up size mode changes, along with a test that *actually* tests this
instead of just looking like it tested it...

Author: alexcjohnson

src/components/colorbar/draw.js

@@ -23,7 +23,10 @@ var Drawing = require('../drawing');
 var Color = require('../color');
 var Titles = require('../titles');
 var svgTextUtils = require('../../lib/svg_text_utils');
-var LINE_SPACING = require('../../constants/alignment').LINE_SPACING;
+var alignmentConstants = require('../../constants/alignment');
+var LINE_SPACING = alignmentConstants.LINE_SPACING;
+var FROM_TL = alignmentConstants.FROM_TL;
+var FROM_BR = alignmentConstants.FROM_BR;
 
 var handleAxisDefaults = require('../../plots/cartesian/axis_defaults');
 var handleAxisPositionDefaults = require('../../plots/cartesian/position_defaults');
@@ -122,13 +125,13 @@ module.exports = function draw(gd, id) {
         // when the colorbar itself is pushing the margins.
         // but then the fractional size is calculated based on the
         // actual graph size, so that the axes will size correctly.
-        var originalPlotHeight = fullLayout.height - fullLayout.margin.t - fullLayout.margin.b,
-            originalPlotWidth = fullLayout.width - fullLayout.margin.l - fullLayout.margin.r,
+        var plotHeight = gs.h,
+            plotWidth = gs.w,
             thickPx = Math.round(opts.thickness *
-                (opts.thicknessmode === 'fraction' ? originalPlotWidth : 1)),
+                (opts.thicknessmode === 'fraction' ? plotWidth : 1)),
             thickFrac = thickPx / gs.w,
             lenPx = Math.round(opts.len *
-                (opts.lenmode === 'fraction' ? originalPlotHeight : 1)),
+                (opts.lenmode === 'fraction' ? plotHeight : 1)),
             lenFrac = lenPx / gs.h,
             xpadFrac = opts.xpad / gs.w,
             yExtraPx = (opts.borderwidth + opts.outlinewidth) / 2,
@@ -537,14 +540,35 @@ module.exports = function draw(gd, id) {
                 'translate(' + (gs.l - xoffset) + ',' + gs.t + ')');
 
             // auto margin adjustment
-            Plots.autoMargin(gd, id, {
-                x: opts.x,
-                y: opts.y,
-                l: outerwidth * ({right: 1, center: 0.5}[opts.xanchor] || 0),
-                r: outerwidth * ({left: 1, center: 0.5}[opts.xanchor] || 0),
-                t: outerheight * ({bottom: 1, middle: 0.5}[opts.yanchor] || 0),
-                b: outerheight * ({top: 1, middle: 0.5}[opts.yanchor] || 0)
-            });
+            var marginOpts = {};
+            var tFrac = FROM_TL[opts.yanchor];
+            var bFrac = FROM_BR[opts.yanchor];
+            if(opts.lenmode === 'pixels') {
+                marginOpts.y = opts.y;
+                marginOpts.t = outerheight * tFrac;
+                marginOpts.b = outerheight * bFrac;
+            }
+            else {
+                marginOpts.t = marginOpts.b = 0; // TODO - not zero?
+                marginOpts.yt = opts.y + opts.len * tFrac;
+                marginOpts.yb = opts.y - opts.len * bFrac;
+            }
+
+            var lFrac = FROM_TL[opts.xanchor];
+            var rFrac = FROM_BR[opts.xanchor];
+            if(opts.thicknessmode === 'pixels') {
+                marginOpts.x = opts.x;
+                marginOpts.l = outerwidth * lFrac;
+                marginOpts.r = outerwidth * rFrac;
+            }
+            else {
+                var extraThickness = outerwidth - thickPx;
+                marginOpts.l = extraThickness * lFrac;
+                marginOpts.r = extraThickness * rFrac;
+                marginOpts.xl = opts.x - opts.thickness * lFrac;
+                marginOpts.xr = opts.x + opts.thickness * rFrac;
+            }
+            Plots.autoMargin(gd, id, marginOpts);
         }
 
         var cbDone = Lib.syncOrAsync([

src/plot_api/plot_api.js

@@ -1453,7 +1453,8 @@ function _restyle(gd, aobj, traces) {
             var finalPart = param.parts[param.parts.length - 1];
             var prefix = ai.substr(0, ai.length - finalPart.length - 1);
             var prefixDot = prefix ? prefix + '.' : '';
-            var innerContFull = prefix ? Lib.nestedProperty(contFull, prefix) : contFull;
+            var innerContFull = prefix ?
+                Lib.nestedProperty(contFull, prefix).get() : contFull;
 
             valObject = PlotSchema.getTraceValObject(contFull, param.parts);
 
@@ -1473,19 +1474,16 @@ function _restyle(gd, aobj, traces) {
                     (newVal === 'fraction' || newVal === 'pixels') &&
                     innerContFull
             ) {
+                var gs = fullLayout._size;
+                var orient = innerContFull.orient;
+                var topOrBottom = (orient === 'top') || (orient === 'bottom');
                 if(finalPart === 'thicknessmode') {
-                    var thicknorm =
-                        ['top', 'bottom'].indexOf(innerContFull.orient) !== -1 ?
-                            (fullLayout.height - fullLayout.margin.t - fullLayout.margin.b) :
-                            (fullLayout.width - fullLayout.margin.l - fullLayout.margin.r);
+                    var thicknorm = topOrBottom ? gs.h : gs.w;
                     doextra(prefixDot + 'thickness', innerContFull.thickness *
                         (newVal === 'fraction' ? 1 / thicknorm : thicknorm), i);
                 }
                 else {
-                    var lennorm =
-                        ['top', 'bottom'].indexOf(innerContFull.orient) !== -1 ?
-                            (fullLayout.width - fullLayout.margin.l - fullLayout.margin.r) :
-                            (fullLayout.height - fullLayout.margin.t - fullLayout.margin.b);
+                    var lennorm = topOrBottom ? gs.w : gs.h;
                     doextra(prefixDot + 'len', innerContFull.len *
                         (newVal === 'fraction' ? 1 / lennorm : lennorm), i);
                 }

test/jasmine/tests/colorbar_test.js

@@ -64,18 +64,33 @@ describe('Test colorbar:', function() {
 
         // let heatmap stand in for all traces with trace.{showscale, colorbar}
         // also test impliedEdits for colorbars at the trace root
-        it('can show and hide heatmap colorbars', function(done) {
-            function assertCB(present, expandedMargin) {
+        it('can show and hide heatmap colorbars and sizes correctly with automargin', function(done) {
+            function assertCB(msg, present, expandedMarginR, expandedMarginT, cbTop, cbHeight) {
                 var colorbars = d3.select(gd).selectAll('.colorbar');
                 expect(colorbars.size()).toBe(present ? 1 : 0);
 
+                var cbbg = colorbars.selectAll('.colorbar .cbbg');
+
                 // check that the displayed object has the right size,
                 // not just that fullLayout._size changed
-                var heatmap = d3.select(gd).selectAll('.hm image');
-                expect(heatmap.size()).toBe(1);
-                var hmWidth = +heatmap.attr('width');
-                if(expandedMargin) expect(hmWidth).toBeLessThan(400);
-                else expect(hmWidth).toBe(400);
+                var plotSizeTest = {};
+                if(expandedMarginR) plotSizeTest.widthLessThan = 400;
+                else plotSizeTest.width = 400;
+
+                if(expandedMarginT) plotSizeTest.heightLessThan = 400;
+                else plotSizeTest.height = 400;
+
+                assertPlotSize(plotSizeTest);
+
+                if(present) {
+                    if(!cbHeight) cbHeight = 400;
+                    var bgHeight = +cbbg.attr('height');
+                    if(expandedMarginT) expect(bgHeight).toBeLessThan(cbHeight - 2);
+                    else expect(bgHeight).toBeWithin(cbHeight, 2);
+
+                    var topShift = cbbg.node().getBoundingClientRect().top - gd.getBoundingClientRect().top;
+                    expect(topShift).toBeWithin(cbTop, 2);
+                }
             }
 
             var thickPx, lenFrac;
@@ -89,17 +104,32 @@ describe('Test colorbar:', function() {
                 margin: {l: 50, r: 50, t: 50, b: 50}
             })
             .then(function() {
-                assertCB(true, true);
+                assertCB('initial', true, true, false, 50);
 
                 return Plotly.restyle(gd, {showscale: false});
             })
             .then(function() {
-                assertCB(false, false);
+                assertCB('hidden', false, false, false);
 
-                return Plotly.restyle(gd, {showscale: true, 'colorbar.x': 0.7});
+                return Plotly.restyle(gd, {showscale: true, 'colorbar.y': 0.8});
             })
             .then(function() {
-                assertCB(true, false);
+                assertCB('up high', true, true, true, 12);
+
+                return Plotly.restyle(gd, {'colorbar.y': 0.7});
+            })
+            .then(function() {
+                assertCB('a little lower', true, true, true, 12);
+
+                return Plotly.restyle(gd, {
+                    'colorbar.x': 0.7,
+                    'colorbar.y': 0.5,
+                    'colorbar.thickness': 50,
+                    'colorbar.len': 0.5
+                });
+            })
+            .then(function() {
+                assertCB('mid-plot', true, false, false, 150, 200);
 
                 thickPx = gd._fullData[0].colorbar.thickness;
                 lenFrac = gd._fullData[0].colorbar.len;
@@ -116,7 +146,7 @@ describe('Test colorbar:', function() {
                 expect(gd._fullData[0].colorbar.len)
                     .toBeCloseTo(lenFrac * 400, 3);
 
-                assertCB(true, true);
+                assertCB('changed size modes', true, true, false, 150, 200);
             })
             .catch(failTest)
             .then(done);