Skip to content

Commit 230a782

Browse files
committed
automargin comment updates
1 parent 24aeb7c commit 230a782

File tree

4 files changed

+9
-5
lines changed

4 files changed

+9
-5
lines changed

src/components/colorbar/draw.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,7 @@ module.exports = function draw(gd, id) {
549549
marginOpts.b = outerheight * bFrac;
550550
}
551551
else {
552-
marginOpts.t = marginOpts.b = 0; // TODO - not zero?
552+
marginOpts.t = marginOpts.b = 0;
553553
marginOpts.yt = opts.y + opts.len * tFrac;
554554
marginOpts.yb = opts.y - opts.len * bFrac;
555555
}

src/components/sliders/draw.js

+3
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ module.exports = function draw(gd) {
4242
delete sliderOpts._commandObserver;
4343
}
4444

45+
// Most components don't need to explicitly remove autoMargin, because
46+
// marginPushers does this - but slider updates don't go through
47+
// a full replot so we need to explicitly remove it.
4548
Plots.autoMargin(gd, autoMarginId(sliderOpts));
4649
}
4750

src/components/updatemenus/draw.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ module.exports = function draw(gd) {
7272
// marginPushers does this - but updatemenu updates don't go through
7373
// a full replot so we need to explicitly remove it.
7474
// This is for removing *all* updatemenus, removing individuals is
75-
// handled below, in hederGroups.exit
75+
// handled below, in headerGroups.exit
7676
d3.select(this).selectAll('g.' + constants.headerGroupClassName)
7777
.each(clearAutoMargin);
7878
}).remove();

src/plot_api/plot_api.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -361,9 +361,10 @@ exports.plot = function(gd, data, layout, config) {
361361
initInteractions,
362362
Plots.addLinks,
363363
Plots.rehover,
364-
// TODO: shouldn't really need doAutoMargin here, but without it some
365-
// edits don't manage to tweak the margins the way they should
366-
// in the tests, the only one that presently breaks is axis.position
364+
// TODO: doAutoMargin is only needed here for axis automargin, which
365+
// happens outside of marginPushers where all the other automargins are
366+
// calculated. Would be much better to separate margin calculations from
367+
// component drawing - see https://github.com/plotly/plotly.js/issues/2704
367368
Plots.doAutoMargin,
368369
Plots.previousPromises
369370
);

0 commit comments

Comments
 (0)