-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Automargin fixes #2681
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
Automargin fixes #2681
Changes from 15 commits
970db80
26278de
6dd68bd
fa86147
95f9b33
8f3da82
f9fbbc6
222f396
bd07df7
84a5695
3deab4c
bb6c720
d02c76b
5d31324
16ea22a
24aeb7c
230a782
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
/** | ||
* Copyright 2012-2018, 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 Colorscale = require('../colorscale'); | ||
var drawColorbar = require('./draw'); | ||
|
||
/** | ||
* connectColorbar: create a colorbar from a trace, using its module to | ||
* describe the connection. | ||
* | ||
* @param {DOM element} gd | ||
* | ||
* @param {Array} cd | ||
* calcdata entry for this trace. cd[0].trace is the trace itself, and the | ||
* colorbar object will be stashed in cd[0].t.cb | ||
* | ||
* @param {object|function} moduleOpts | ||
* may be a function(gd, cd) to override the standard handling below. If | ||
* an object, should have these keys: | ||
* @param {Optional(string)} moduleOpts.container | ||
* name of the container inside the trace where the colorbar and colorscale | ||
* attributes live (ie 'marker', 'line') - omit if they're at the trace root. | ||
* @param {string} moduleOpts.min | ||
* name of the attribute holding the value of the minimum color | ||
* @param {string} moduleOpts.max | ||
* name of the attribute holding the value of the maximum color | ||
* @param {Optional(string)} moduleOpts.vals | ||
* name of the attribute holding the (numeric) color data | ||
* used only if min/max fail. May be omitted if these are always | ||
* pre-calculated. | ||
*/ | ||
module.exports = function connectColorbar(gd, cd, moduleOpts) { | ||
if(typeof moduleOpts === 'function') return moduleOpts(gd, cd); | ||
|
||
var trace = cd[0].trace; | ||
var cbId = 'cb' + trace.uid; | ||
var containerName = moduleOpts.container; | ||
var container = containerName ? trace[containerName] : trace; | ||
|
||
gd._fullLayout._infolayer.selectAll('.' + cbId).remove(); | ||
if(!container || !container.showscale) return; | ||
|
||
var zmin = container[moduleOpts.min]; | ||
var zmax = container[moduleOpts.max]; | ||
|
||
var cb = cd[0].t.cb = drawColorbar(gd, cbId); | ||
var sclFunc = Colorscale.makeColorScaleFunc( | ||
Colorscale.extractScale( | ||
container.colorscale, | ||
zmin, | ||
zmax | ||
), | ||
{ noNumericCheck: true } | ||
); | ||
|
||
cb.fillcolor(sclFunc) | ||
.filllevels({start: zmin, end: zmax, size: (zmax - zmin) / 254}) | ||
.options(container.colorbar)(); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
|
@@ -447,12 +450,10 @@ module.exports = function draw(gd, id) { | |
} | ||
|
||
function drawTitle(titleClass, titleOpts) { | ||
var trace = getTrace(), | ||
propName; | ||
if(Registry.traceIs(trace, 'markerColorscale')) { | ||
propName = 'marker.colorbar.title'; | ||
} | ||
else propName = 'colorbar.title'; | ||
var trace = getTrace(); | ||
var propName = 'colorbar.title'; | ||
var containerName = trace._module.colorbar.container; | ||
if(containerName) propName = containerName + '.' + propName; | ||
|
||
var dfltTitleOpts = { | ||
propContainer: cbAxisOut, | ||
|
@@ -539,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? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you expand on that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah thanks, that was just for me to check during development, it's correct as zeros -> removed in 230a782 |
||
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([ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,15 +61,9 @@ module.exports = function(gd) { | |
|
||
// remove exiting sliders and their corresponding clip paths | ||
rangeSliders.exit().each(function(axisOpts) { | ||
var rangeSlider = d3.select(this), | ||
opts = axisOpts[constants.name]; | ||
|
||
rangeSlider.remove(); | ||
var opts = axisOpts[constants.name]; | ||
fullLayout._topdefs.select('#' + opts._clipId).remove(); | ||
}); | ||
|
||
// remove push margin object(s) | ||
if(rangeSliders.exit().size()) clearPushMargins(gd); | ||
}).remove(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ha |
||
|
||
// return early if no range slider is visible | ||
if(rangeSliderData.length === 0) return; | ||
|
@@ -602,16 +596,3 @@ function drawGrabbers(rangeSlider, gd, axisOpts, opts) { | |
}); | ||
grabAreaMax.attr('height', opts._height); | ||
} | ||
|
||
function clearPushMargins(gd) { | ||
var pushMargins = gd._fullLayout._pushmargin || {}, | ||
keys = Object.keys(pushMargins); | ||
|
||
for(var i = 0; i < keys.length; i++) { | ||
var k = keys[i]; | ||
|
||
if(k.indexOf(constants.name) !== -1) { | ||
Plots.autoMargin(gd, k); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,10 +36,20 @@ module.exports = function draw(gd) { | |
.classed(constants.containerClassName, true) | ||
.style('cursor', 'ew-resize'); | ||
|
||
sliders.exit().remove(); | ||
function clearSlider(sliderOpts) { | ||
if(sliderOpts._commandObserver) { | ||
sliderOpts._commandObserver.remove(); | ||
delete sliderOpts._commandObserver; | ||
} | ||
|
||
Plots.autoMargin(gd, autoMarginId(sliderOpts)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we add a comment similar to what's above https://github.com/plotly/plotly.js/pull/2681/files#r191879632 here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure -> 230a782 |
||
} | ||
|
||
// If no more sliders, clear the margisn: | ||
if(sliders.exit().size()) clearPushMargins(gd); | ||
sliders.exit().each(function() { | ||
d3.select(this).selectAll('g.' + constants.groupClassName) | ||
.each(clearSlider); | ||
}) | ||
.remove(); | ||
|
||
// Return early if no menus visible: | ||
if(sliderData.length === 0) return; | ||
|
@@ -50,14 +60,9 @@ module.exports = function draw(gd) { | |
sliderGroups.enter().append('g') | ||
.classed(constants.groupClassName, true); | ||
|
||
sliderGroups.exit().each(function(sliderOpts) { | ||
d3.select(this).remove(); | ||
|
||
sliderOpts._commandObserver.remove(); | ||
delete sliderOpts._commandObserver; | ||
|
||
Plots.autoMargin(gd, constants.autoMarginIdRoot + sliderOpts._index); | ||
}); | ||
sliderGroups.exit() | ||
.each(clearSlider) | ||
.remove(); | ||
|
||
// Find the dimensions of the sliders: | ||
for(var i = 0; i < sliderData.length; i++) { | ||
|
@@ -92,6 +97,10 @@ module.exports = function draw(gd) { | |
}); | ||
}; | ||
|
||
function autoMarginId(sliderOpts) { | ||
return constants.autoMarginIdRoot + sliderOpts._index; | ||
} | ||
|
||
// This really only just filters by visibility: | ||
function makeSliderData(fullLayout, gd) { | ||
var contOpts = fullLayout[constants.name], | ||
|
@@ -221,14 +230,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, autoMarginId(sliderOpts), marginOpts); | ||
} | ||
|
||
function drawSlider(gd, sliderGroup, sliderOpts) { | ||
|
@@ -594,16 +614,3 @@ function drawRail(sliderGroup, sliderOpts) { | |
(dims.inputAreaWidth - constants.railWidth) * 0.5 + dims.currentValueTotalHeight | ||
); | ||
} | ||
|
||
function clearPushMargins(gd) { | ||
var pushMargins = gd._fullLayout._pushmargin || {}, | ||
keys = Object.keys(pushMargins); | ||
|
||
for(var i = 0; i < keys.length; i++) { | ||
var k = keys[i]; | ||
|
||
if(k.indexOf(constants.autoMarginIdRoot) !== -1) { | ||
Plots.autoMargin(gd, k); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, contour still gets its own version on
module._colorbar
inhttps://github.com/plotly/plotly.js/blob/fa8614750586a2901ccf65a520521bb570eb7026/src/traces/contour/colorbar.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... and that's the only exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, contour and those that inherit from it (
contourcarpet
,contourgl
,histogram2dcontour
).I should note that there's one other place
module.colorbar
gets used, that's incolorbar.draw
for getting thenestedProperty
string for title editing. If contour colorbars weren't at the module root we'd need to set acontainer
property of the function which would be a bit hacky... alternatively we could either a) make this function bemodule.colorbar.func
, or b) incorporate contour into this routine directly, triggered by some other property ofmodule.colorbar
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds cleaner to me. But no need to do this in this PR.