Skip to content

Multiple range sliders #1355

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Feb 23, 2017
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/components/rangeslider/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ module.exports = {
role: 'style',
description: 'Sets the border color of the range slider.'
},
autorange: {
valType: 'boolean',
dflt: true,
role: 'style',
description: [
'Determines whether or not the range slider range is',
'computed in relation to the input data.',
'If `range` is provided, then `autorange` is set to *false*.'
].join(' ')
},
range: {
valType: 'info_array',
role: 'info',
Expand Down
34 changes: 34 additions & 0 deletions src/components/rangeslider/calc_autorange.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* Copyright 2012-2017, 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';

var Axes = require('../../plots/cartesian/axes');
var constants = require('./constants');

module.exports = function calcAutorange(gd) {
var axes = Axes.list(gd, 'x', true);

// Compute new slider range using axis autorange if necessary.
//
// Copy back range to input range slider container to skip
// this step in subsequent draw calls.

for(var i = 0; i < axes.length; i++) {
var ax = axes[i],
opts = ax[constants.name];

// Don't try calling getAutoRange if _min and _max are filled in.
// This happens on updates where the calc step is skipped.

if(opts && opts.visible && opts.autorange && ax._min.length && ax._max.length) {
opts._input.autorange = true;
opts._input.range = opts.range = Axes.getAutoRange(ax);
}
}
};
2 changes: 2 additions & 0 deletions src/components/rangeslider/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,6 @@ module.exports = {
handleRadius: 1,
handleFill: '#fff',
handleStroke: '#666',

extraPad: 15
};
21 changes: 15 additions & 6 deletions src/components/rangeslider/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@

'use strict';

var isNumeric = require('fast-isnumeric');

var Lib = require('../../lib');
var attributes = require('./attributes');


module.exports = function handleDefaults(layoutIn, layoutOut, axName) {
if(!layoutIn[axName].rangeslider) return;

Expand All @@ -28,25 +29,33 @@ module.exports = function handleDefaults(layoutIn, layoutOut, axName) {
return Lib.coerce(containerIn, containerOut, attributes, attr, dflt);
}

var visible = coerce('visible');
if(!visible) return;

coerce('bgcolor', layoutOut.plot_bgcolor);
coerce('bordercolor');
coerce('borderwidth');
coerce('thickness');
coerce('visible');

coerce('autorange', !(
(containerIn.range || []).length === 2 &&
isNumeric(axOut.r2l(containerIn.range[0])) &&
isNumeric(axOut.r2l(containerIn.range[1]))
));
coerce('range');

// Expand slider range to the axis range
if(containerOut.range && !axOut.autorange) {
// TODO: what if the ranges are reversed?
// TODO: what if the ranges are reversed?
if(containerOut.range) {
var outRange = containerOut.range,
axRange = axOut.range;

outRange[0] = axOut.l2r(Math.min(axOut.r2l(outRange[0]), axOut.r2l(axRange[0])));
outRange[1] = axOut.l2r(Math.max(axOut.r2l(outRange[1]), axOut.r2l(axRange[1])));
} else {
axOut._needsExpand = true;
}

axOut.cleanRange('rangeslider.range');

// to map back range slider (auto) range
containerOut._input = containerIn;
};
55 changes: 33 additions & 22 deletions src/components/rangeslider/draw.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,20 +77,16 @@ module.exports = function(gd) {
// for all present range sliders
rangeSliders.each(function(axisOpts) {
var rangeSlider = d3.select(this),
opts = axisOpts[constants.name];

// compute new slider range using axis autorange if necessary
// copy back range to input range slider container to skip
// this step in subsequent draw calls
if(!opts.range) {
opts._input.range = opts.range = Axes.getAutoRange(axisOpts);
}
opts = axisOpts[constants.name],
oppAxisOpts = fullLayout[Axes.id2name(axisOpts.anchor)];

// update range slider dimensions

var margin = fullLayout.margin,
graphSize = fullLayout._size,
domain = axisOpts.domain;
domain = axisOpts.domain,
oppDomain = oppAxisOpts.domain,
tickHeight = (axisOpts._boundingBox || {}).height || 0;

opts._id = constants.name + axisOpts._id;
opts._clipId = opts._id + '-' + fullLayout._uid;
Expand All @@ -99,8 +95,13 @@ module.exports = function(gd) {
opts._height = (fullLayout.height - margin.b - margin.t) * opts.thickness;
opts._offsetShift = Math.floor(opts.borderwidth / 2);

var x = margin.l + (graphSize.w * domain[0]),
y = fullLayout.height - opts._height - margin.b;
var x = margin.l + (graphSize.w * domain[0]);

var y = (
margin.t + graphSize.h * (1 - oppDomain[0]) +
tickHeight +
opts._offsetShift + constants.extraPad
);
Copy link
Contributor Author

@etpinard etpinard Feb 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I got this right. I would appreciate having something take a second 👁️ at it though.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess something in here is responsible for the small position differences in the mocks? I'm not bothered by the differences, except that some of the new ones look a little bit fuzzier than the old ones. Can you try and round it so that the outer edges of the range slider border are exactly on a pixel boundary, even if you have non-integers in layout.height, the margins, rangeslider.borderwidth and anything else that comes into the calculation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexcjohnson some improvements on:

multiple-rangesliders...rangeslider-crisp-round

I think the mocks look better to my 👀 , would you mind taking a look at them?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this isn't new but I'm just noticing it now while we're here: should we switch handleStroke to be Color.defaultLineColor = '#444' instead of #666 as it is now? And to avoid duplication, handleFill = '#fff' is already available as Color.background - to explicitly match how we define zoombox corners

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5a649f0 gives

image

is that what you had in mind?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The image you've pasted here is blown up (with browser zoom?), so the rounding isn't quite right and consequently there are still some odd 1-pixel effects at the corners. When I look at the rangeslider-crisp-round branch locally it looks almost perfect.
screen shot 2017-02-22 at 10 22 28 pm

I realize this is a borderline obsessive ask (which side of the border?) so I did it myself in 29a31d5 and b49ea77. I just tweaked it a tiny bit to get the symmetry right: both handles the slightest bit more on the dark side. I suppose we could bump the width either up or down 1px so it would be possible to have them each exactly symmetric across the dark/light line, but to my eye anyway symmetry between the two handles is more important. Also included rounding of everything that goes into the grabber position before offsetting, because sometimes we were still getting a non-integer starting point, making the half-pixel offset moot. Kind of annoying but I think now it's really truly guaranteed to be an honest rectangle.

screen shot 2017-02-22 at 10 32 16 pm

Seem reasonable?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aaand in case you weren't 100% convinced I'm being obsessive - I've got a retina display on my laptop, and a non-retina external monitor. The retina display still rounds off all the corners, since it doubles the real number of pixels displayed. I'd say this looks fine, perhaps even better than the perfect sharp corners on a non-retina display - because it's a consistent look and the missing pixels are smaller than the line width so the line really looks rounded, not broken like it did on regular monitors before. Screen shot blown up so you can see the individual retina pixels:
screen shot 2017-02-22 at 11 13 49 pm


rangeSlider.attr('transform', 'translate(' + x + ',' + y + ')');

Expand Down Expand Up @@ -138,23 +139,33 @@ module.exports = function(gd) {

// update margins

var bb = axisOpts._boundingBox ? axisOpts._boundingBox.height : 0;

Plots.autoMargin(gd, opts._id, {
x: 0, y: 0, l: 0, r: 0, t: 0,
b: opts._height + fullLayout.margin.b + bb,
pad: 15 + opts._offsetShift * 2
x: domain[0],
y: oppDomain[0],
l: 0,
r: 0,
t: 0,
b: opts._height + margin.b + tickHeight,
pad: constants.extraPad + opts._offsetShift * 2
});

});
};

function makeRangeSliderData(fullLayout) {
if(!fullLayout.xaxis) return [];
if(!fullLayout.xaxis[constants.name]) return [];
if(!fullLayout.xaxis[constants.name].visible) return [];
if(fullLayout._has('gl2d')) return [];
var axes = Axes.list({ _fullLayout: fullLayout }, 'x', true),
name = constants.name,
out = [];

return [fullLayout.xaxis];
if(fullLayout._has('gl2d')) return out;

for(var i = 0; i < axes.length; i++) {
var ax = axes[i];

if(ax[name] && ax[name].visible) out.push(ax);
}

return out;
}

function setupDragElement(rangeSlider, gd, axisOpts, opts) {
Expand Down Expand Up @@ -236,7 +247,7 @@ function setDataRange(rangeSlider, gd, axisOpts, opts) {
dataMax = clamp(opts.p2d(opts._pixelMax));

window.requestAnimationFrame(function() {
Plotly.relayout(gd, 'xaxis.range', [dataMin, dataMax]);
Plotly.relayout(gd, axisOpts._name + '.range', [dataMin, dataMax]);
});
}

Expand Down
2 changes: 1 addition & 1 deletion src/components/rangeslider/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ module.exports = {

layoutAttributes: require('./attributes'),
handleDefaults: require('./defaults'),

calcAutorange: require('./calc_autorange'),
draw: require('./draw')
};
4 changes: 2 additions & 2 deletions src/plot_api/plot_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ Plotly.plot = function(gd, data, layout, config) {
}

// draw anything that can affect margins.
// currently this is legend and colorbars
function marginPushers() {
var calcdata = gd.calcdata;
var i, cd, trace;
Expand Down Expand Up @@ -253,7 +252,8 @@ Plotly.plot = function(gd, data, layout, config) {
return Lib.syncOrAsync([
Registry.getComponentMethod('shapes', 'calcAutorange'),
Registry.getComponentMethod('annotations', 'calcAutorange'),
doAutoRange
doAutoRange,
Registry.getComponentMethod('rangeslider', 'calcAutorange')
], gd);
}

Expand Down
4 changes: 3 additions & 1 deletion src/plots/cartesian/axes.js
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,9 @@ axes.saveRangeInitial = function(gd, overwrite) {
// tozero: (boolean) make sure to include zero if axis is linear,
// and make it a tight bound if possible
axes.expand = function(ax, data, options) {
if(!(ax.autorange || ax._needsExpand) || !data) return;
var needsAutorange = (ax.autorange || Lib.nestedProperty(ax, 'rangeslider.autorange'));
if(!needsAutorange || !data) return;

if(!ax._min) ax._min = [];
if(!ax._max) ax._max = [];
if(!options) options = {};
Expand Down
4 changes: 2 additions & 2 deletions src/plots/cartesian/set_convert.js
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ module.exports = function setConvert(ax) {
*/
ax.cleanRange = function(rangeAttr) {
if(!rangeAttr) rangeAttr = 'range';
var range = ax[rangeAttr],
var range = Lib.nestedProperty(ax, rangeAttr).get(),
axLetter = (ax._id || 'x').charAt(0),
i, dflt;

Expand All @@ -266,7 +266,7 @@ module.exports = function setConvert(ax) {
dflt = dflt.slice();

if(!range || range.length !== 2) {
ax[rangeAttr] = dflt;
Lib.nestedProperty(ax, rangeAttr).set(dflt);
return;
}

Expand Down
25 changes: 19 additions & 6 deletions src/plots/plots.js
Original file line number Diff line number Diff line change
Expand Up @@ -1223,7 +1223,7 @@ plots.sanitizeMargins = function(fullLayout) {
}
};

// called by legend and colorbar routines to see if we need to
// 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
Expand Down Expand Up @@ -1263,7 +1263,7 @@ plots.doAutoMargin = function(gd) {
var gs = fullLayout._size,
oldmargins = JSON.stringify(gs);

// adjust margins for outside legends and colorbars
// adjust margins for outside components
// fullLayout.margin is the requested margin,
// fullLayout._size has margins and plotsize after adjustment
var ml = Math.max(fullLayout.margin.l || 0, 0),
Expand All @@ -1273,26 +1273,37 @@ plots.doAutoMargin = function(gd) {
pm = fullLayout._pushmargin;

if(fullLayout.margin.autoexpand !== false) {

// fill in the requested margins
pm.base = {
l: {val: 0, size: ml},
r: {val: 1, size: mr},
t: {val: 1, size: mt},
b: {val: 0, size: mb}
};

// now cycle through all the combinations of l and r
// (and t and b) to find the required margins
Object.keys(pm).forEach(function(k1) {

var pmKeys = Object.keys(pm);

for(var i = 0; i < pmKeys.length; i++) {
var k1 = pmKeys[i];

var pushleft = pm[k1].l || {},
pushbottom = pm[k1].b || {},
fl = pushleft.val,
pl = pushleft.size,
fb = pushbottom.val,
pb = pushbottom.size;
Object.keys(pm).forEach(function(k2) {

for(var j = 0; j < pmKeys.length; j++) {
var k2 = pmKeys[j];

if(isNumeric(pl) && pm[k2].r) {
var fr = pm[k2].r.val,
pr = pm[k2].r.size;

if(fr > fl) {
var newl = (pl * fr +
(pr - fullLayout.width) * fl) / (fr - fl),
Expand All @@ -1304,9 +1315,11 @@ plots.doAutoMargin = function(gd) {
}
}
}

if(isNumeric(pb) && pm[k2].t) {
var ft = pm[k2].t.val,
pt = pm[k2].t.size;

if(ft > fb) {
var newb = (pb * ft +
(pt - fullLayout.height) * fb) / (ft - fb),
Expand All @@ -1318,8 +1331,8 @@ plots.doAutoMargin = function(gd) {
}
}
}
});
});
}
}
}

gs.l = Math.round(ml);
Expand Down
Loading