Skip to content

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.

Sign up for GitHub

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 17 commits into from
Jun 6, 2018
Merged

Automargin fixes #2681

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
970db80
stop mutating user data to add uids
alexcjohnson May 29, 2018
26278de
some test cleanup
alexcjohnson May 29, 2018
6dd68bd
first cut at updating automargin logic
alexcjohnson May 30, 2018
fa86147
refactor colorbars for simpler connection to trace attributes
alexcjohnson May 30, 2018
95f9b33
test legend, rangeselector, rangeslider automargin updates
alexcjohnson May 30, 2018
8f3da82
provide uids to traces a little earlier - to fix carpet clip ids
alexcjohnson May 30, 2018
f9fbbc6
:hocho: colorbar.vals - seems like that was all obsolete anyway
alexcjohnson May 30, 2018
222f396
cleanup in sliders_test
alexcjohnson May 31, 2018
bd07df7
simplify inner plot size tests with custom_assertions
alexcjohnson Jun 1, 2018
84a5695
fix and test slider x positioning when they push margins
alexcjohnson Jun 1, 2018
3deab4c
move sliders to the updated automargin framework
alexcjohnson Jun 1, 2018
bb6c720
use bg rect instead of grids for assertPlotSize
alexcjohnson Jun 1, 2018
d02c76b
base fractional colorbar size on post-expanded-margin plot size
alexcjohnson Jun 2, 2018
5d31324
comment and fix on assertPlotSize with margin.pad
alexcjohnson Jun 6, 2018
16ea22a
reduce automargin padding for small explicit margins
alexcjohnson Jun 6, 2018
24aeb7c
axRange edits don't need drawMarginPushers after all
alexcjohnson Jun 6, 2018
230a782
automargin comment updates
alexcjohnson Jun 6, 2018
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
21 changes: 12 additions & 9 deletions src/plots/plots.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,9 @@ plots.supplyDataDefaults = function(dataIn, dataOut, layout, fullLayout) {

for(i = 0; i < dataIn.length; i++) {
trace = dataIn[i];
fullTrace = plots.supplyTraceDefaults(trace, colorCnt, fullLayout, i);
fullTrace = plots.supplyTraceDefaults(trace, colorCnt, fullLayout, i,
// reuse uid we may have pulled out of oldFullData
fullLayout._traceUids[i]);

fullTrace.uid = fullLayout._traceUids[i];

Expand All @@ -942,16 +944,17 @@ plots.supplyDataDefaults = function(dataIn, dataOut, layout, fullLayout) {

for(var j = 0; j < expandedTraces.length; j++) {
var expandedTrace = expandedTraces[j];
var fullExpandedTrace = plots.supplyTraceDefaults(expandedTrace, cnt, fullLayout, i);
var fullExpandedTrace = plots.supplyTraceDefaults(
expandedTrace, cnt, fullLayout, i,
// set uid using parent uid and expanded index
// to promote consistency between update calls
fullTrace.uid + j
);

// relink private (i.e. underscore) keys expanded trace to full expanded trace so
// that transform supply-default methods can set _ keys for future use.
relinkPrivateKeys(fullExpandedTrace, expandedTrace);

// set uid using parent uid and expanded index
// to promote consistency between update calls
fullExpandedTrace.uid = fullTrace.uid + j;

// add info about parent data trace
fullExpandedTrace.index = i;
fullExpandedTrace._input = trace;
Expand Down Expand Up @@ -1076,10 +1079,10 @@ plots.supplyFrameDefaults = function(frameIn) {
return frameOut;
};

plots.supplyTraceDefaults = function(traceIn, colorIndex, layout, traceInIndex) {
plots.supplyTraceDefaults = function(traceIn, colorIndex, layout, traceInIndex, uid) {
var colorway = layout.colorway || Color.defaults;
var traceOut = {},
defaultColor = colorway[colorIndex % colorway.length];
var traceOut = {uid: uid};
Copy link
Contributor

Choose a reason for hiding this comment

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

Or alternatively, I think we could move this line:

image

to just before this line:

image

Copy link
Contributor

Choose a reason for hiding this comment

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

the above comment is non-blocking.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah probably - but I thought it would be better in supplyDefaults so we don't add another order-dependence to the plot step - carpet dependents need this too. I think they're already required to plot after the carpet for other reasons but still...

Copy link
Contributor

Choose a reason for hiding this comment

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

but I thought it would be better in supplyDefaults so we don't add another order-dependence to the plot step - carpet dependents need this too.

Good point. Your way is 👌

var defaultColor = colorway[colorIndex % colorway.length];

var i;

Expand Down