fix and test slider x positioning when they push margins

Also includes a generalization of `Plots.autoMargin` so an object can
have extent in plot fraction, as opposed to just position in plot fraction
and extent in pixels. We could use this to allow for example colorbars
to be sized based on the actual margins, not the pre-margin-pushing plot size
as they are now, see note at colorbar/draw:119-124

Author: alexcjohnson

src/components/sliders/draw.js

@@ -221,14 +221,25 @@ function findDimensions(gd, sliderOpts) {
     dims.lx = Math.round(dims.lx);
     dims.ly = Math.round(dims.ly);
 
-    Plots.autoMargin(gd, constants.autoMarginIdRoot + sliderOpts._index, {
-        x: sliderOpts.x,
+    var marginOpts = {
         y: sliderOpts.y,
-        l: dims.outerLength * FROM_TL[xanchor],
-        r: dims.outerLength * FROM_BR[xanchor],
         b: dims.height * FROM_BR[yanchor],
         t: dims.height * FROM_TL[yanchor]
-    });
+    };
+
+    if(sliderOpts.lenmode === 'fraction') {
+        marginOpts.l = 0;
+        marginOpts.xl = sliderOpts.x - sliderOpts.len * FROM_TL[xanchor];
+        marginOpts.r = 0;
+        marginOpts.xr = sliderOpts.x + sliderOpts.len * FROM_BR[xanchor];
+    }
+    else {
+        marginOpts.x = sliderOpts.x;
+        marginOpts.l = dims.outerLength * FROM_TL[xanchor];
+        marginOpts.r = dims.outerLength * FROM_BR[xanchor];
+    }
+
+    Plots.autoMargin(gd, constants.autoMarginIdRoot + sliderOpts._index, marginOpts);
 }
 
 function drawSlider(gd, sliderGroup, sliderOpts) {

src/plots/plots.js

@@ -1603,11 +1603,21 @@ function setupAutoMargin(fullLayout) {
     if(!fullLayout._pushmarginIds) fullLayout._pushmarginIds = {};
 }
 
-// called by components to see if we need to
-// expand the margins to show them
-// o is {x,l,r,y,t,b} where x and y are plot fractions,
-// the rest are pixels in each direction
-// or leave o out to delete this entry (like if it's hidden)
+/**
+ * autoMargin: called by components that may need to expand the margins to
+ * be rendered on-plot.
+ *
+ * @param {DOM element} gd
+ * @param {string} id - an identifier unique (within this plot) to this object,
+ *     so we can remove a previous margin expansion from the same object.
+ * @param {object} o - the margin requirements of this object, or omit to delete
+ *     this entry (like if it's hidden). Keys are:
+ *     x, y: plot fraction of the anchor point.
+ *     xl, xr, yt, yb: if the object has an extent defined in plot fraction,
+ *         you can specify both edges as plot fractions in each dimension
+ *     l, r, t, b: the pixels to pad past the plot fraction x[l|r] and y[t|b]
+ *     pad: extra pixels to add in all directions, default 12 (why?)
+ */
 plots.autoMargin = function(gd, id, o) {
     var fullLayout = gd._fullLayout;
 
@@ -1629,11 +1639,16 @@ plots.autoMargin = function(gd, id, o) {
             if(o.l + o.r > fullLayout.width * 0.5) o.l = o.r = 0;
             if(o.b + o.t > fullLayout.height * 0.5) o.b = o.t = 0;
 
+            var xl = o.xl !== undefined ? o.xl : o.x;
+            var xr = o.xr !== undefined ? o.xr : o.x;
+            var yt = o.yt !== undefined ? o.yt : o.y;
+            var yb = o.yb !== undefined ? o.yb : o.y;
+
             pushMargin[id] = {
-                l: {val: o.x, size: o.l + pad},
-                r: {val: o.x, size: o.r + pad},
-                b: {val: o.y, size: o.b + pad},
-                t: {val: o.y, size: o.t + pad}
+                l: {val: xl, size: o.l + pad},
+                r: {val: xr, size: o.r + pad},
+                b: {val: yb, size: o.b + pad},
+                t: {val: yt, size: o.t + pad}
             };
             pushMarginIds[id] = 1;
         }

test/jasmine/tests/sliders_test.js

@@ -312,7 +312,7 @@ describe('sliders interactions', function() {
     beforeEach(function(done) {
         gd = createGraphDiv();
 
-        mockCopy = Lib.extendDeep({}, mock);
+        mockCopy = Lib.extendDeep({}, mock, {layout: {sliders: [{x: 0.25}, {}]}});
 
         Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(done);
     });
@@ -322,6 +322,37 @@ describe('sliders interactions', function() {
         destroyGraphDiv();
     });
 
+    it('positions sliders repeatably when they push margins', function(done) {
+        function checkPositions(msg) {
+            d3.select(gd).selectAll('.slider-group').each(function(d, i) {
+                var sliderBB = this.getBoundingClientRect();
+                var gdBB = gd.getBoundingClientRect();
+                if(i === 0) {
+                    expect(sliderBB.left - gdBB.left)
+                        .toBeWithin(12, 3, 'left: ' + msg);
+                }
+                else {
+                    expect(gdBB.bottom - sliderBB.bottom)
+                        .toBeWithin(8, 3, 'bottom: ' + msg);
+                }
+            });
+        }
+
+        checkPositions('initial');
+
+        Plotly.relayout(gd, {'sliders[0].x': 0.35, 'sliders[1].y': -0.3})
+        .then(function() {
+            checkPositions('increased left & bottom');
+
+            return Plotly.relayout(gd, {'sliders[0].x': 0.1, 'sliders[1].y': -0.1});
+        })
+        .then(function() {
+            checkPositions('back to original');
+        })
+        .catch(failTest)
+        .then(done);
+    });
+
     it('should draw only visible sliders', function(done) {
         expect(gd._fullLayout._pushmargin['slider-0']).toBeDefined();
         expect(gd._fullLayout._pushmargin['slider-1']).toBeDefined();