Skip to content

Adding automargin support to plot titles #6428

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
hannahker merged 63 commits into from
Mar 15, 2023
Merged

Adding automargin support to plot titles #6428

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
68c6165
Initial pass with hard-coded automargin params
hannahker Jan 6, 2023
ad6eb00
Remove role from schema
hannahker Jan 7, 2023
0826ddd
Merge branch 'master' into automargin-title
hannahker Jan 18, 2023
45d9cbf
Move automargin call
hannahker Jan 26, 2023
c6e31be
lint
archmoj Jan 27, 2023
f46014f
rename mock and generate baseline
archmoj Jan 27, 2023
587dfcb
update pie_automargin-margin0 baseline
archmoj Jan 27, 2023
3dae109
adjust pie test
archmoj Jan 27, 2023
61487a8
Simplify automargin title to use existing functions
hannahker Jan 30, 2023
8679e7d
Account for padding around title
hannahker Feb 6, 2023
e8e49a5
Adjust mock to yref=paper
hannahker Feb 7, 2023
2b59e2c
Temp fix for yref=paper
hannahker Feb 7, 2023
5e4ab42
Merge branch 'master' into automargin-title
hannahker Feb 16, 2023
4bb109e
Improve handling for paper ref and setting defaults
hannahker Feb 16, 2023
7ce1234
Fix formatting
hannahker Feb 16, 2023
b274642
Add jasmine test
hannahker Feb 16, 2023
cc15ff6
Simplify mock
hannahker Feb 17, 2023
938d52d
Fix test formatting
hannahker Feb 17, 2023
75636e2
Add simple implementation for container behaviour
hannahker Feb 18, 2023
b7e3295
Apply review comments
hannahker Feb 28, 2023
04f8825
Add more tests for yref=container
hannahker Feb 28, 2023
ad202d9
Ajust default setting
hannahker Feb 28, 2023
9611672
Remove outdated mock
hannahker Mar 1, 2023
c4d1076
Update mock baselines
hannahker Mar 1, 2023
21a9e59
Fix syntax
hannahker Mar 1, 2023
78cecff
Remove f from test
hannahker Mar 1, 2023
fd111d5
Set yanchor properly when title is at bottom
hannahker Mar 6, 2023
2a47c00
Make defaults setting logic more readable
hannahker Mar 7, 2023
52b37c0
Add reservedMargins logic
hannahker Mar 7, 2023
4322c7b
Revert some changes to test
hannahker Mar 7, 2023
73ccaf4
Fix bugs
hannahker Mar 8, 2023
a3462d2
Fix more bugs from redraw
hannahker Mar 8, 2023
33bbbef
Apply review comments
hannahker Mar 9, 2023
9af5cbd
Add jasmine test for interaction with x axis
hannahker Mar 9, 2023
67cf8da
Push pie test fix
hannahker Mar 9, 2023
16c0bcf
Fix scooting behaviour to account for reserved margin space
hannahker Mar 9, 2023
17c3244
Check if automargin is necessary before applying
hannahker Mar 10, 2023
c014702
Update tests
hannahker Mar 10, 2023
e59b18f
Warn when title.y is being set to 1
hannahker Mar 10, 2023
ddaf013
Fix syntax and another mock update
hannahker Mar 10, 2023
de7ca48
Apply review comments
hannahker Mar 10, 2023
e77126c
Calc bb for title
hannahker Mar 10, 2023
26ca553
Fix warning log
hannahker Mar 10, 2023
e5e8a2a
Update parameter description
hannahker Mar 10, 2023
d70fb46
Reposition properly
hannahker Mar 10, 2023
616057f
Merge branch 'master' into automargin-title
hannahker Mar 10, 2023
3523406
Only reposition title if necessary
hannahker Mar 10, 2023
90bd374
Fix syntax
hannahker Mar 10, 2023
158791f
Fix tests
hannahker Mar 10, 2023
c4e706e
Merge branch 'automargin-title-dev' into automargin-title
hannahker Mar 10, 2023
087c162
Adjust tests to match rendering in CI
hannahker Mar 13, 2023
c4d75d4
Update mocks
hannahker Mar 13, 2023
a257836
Add review comment
hannahker Mar 13, 2023
662628e
Update mock baselines
hannahker Mar 13, 2023
e4b5f29
Adjust titles on mocks
hannahker Mar 13, 2023
3dcaab8
Update baselines
hannahker Mar 13, 2023
2a0011a
Add milti-line title
hannahker Mar 14, 2023
a8185f4
Update baselines
hannahker Mar 14, 2023
0c0e47e
Revert dy change
hannahker Mar 14, 2023
7f89c73
move smart default for title.y and title.yanchor to supply defaults
archmoj Mar 10, 2023
132c57b
Adjust dy to fix offset
hannahker Mar 15, 2023
7d4e352
baseline update
archmoj Mar 15, 2023
ad7c6f0
Add draftlog
hannahker Mar 15, 2023
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
Prev Previous commit
Next Next commit
Add reservedMargins logic
  • Loading branch information
@hannahker
hannahker committed Mar 7, 2023
commit 52b37c026dd65bac15ce29d26725bda2be71480b
18 changes: 8 additions & 10 deletions src/plot_api/subroutines.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,19 +445,19 @@ function setDflts(title) {
title.y = title.y === 0 ? title.y : 1;
if(title.yanchor === 'auto') {
if(title.y === 0) {
title.yanchor = 'top'
title.yanchor = 'top';
} else {
title.yanchor = 'bottom'
title.yanchor = 'bottom';
}
}
}
if(title.automargin && title.yref === 'container') {
title.y = title.y === 'auto' ? 1 : title.y;
if(title.yanchor === 'auto') {
if(title.y < 0.5) {
title.yanchor = 'bottom'
title.yanchor = 'bottom';
} else {
title.yanchor = 'top'
title.yanchor = 'top';
}
}
}
Expand Down Expand Up @@ -497,7 +497,7 @@ function applyTitleAutoMargin(gd, y) {
var position = gd._fullLayout.title.y > 0.5 ? 't' : 'b';
var push = {
x: title.x,
y: title.y > 0.5 ? 1 : 0, // TODO: Is it correct that this should either be 1 or 0?
y: title.y,
t: 0,
b: 0
};
Expand All @@ -508,17 +508,15 @@ function applyTitleAutoMargin(gd, y) {
title.pad.t +
title.pad.b
);
Plots.allowAutoMargin(gd, titleID);
Plots.autoMargin(gd, titleID, push);
} else if(title.yref === 'container') {
push[position] = (
gd._fullLayout._reservedMargin[position] = (
containerPushVal(position, title, gd._fullLayout.height, titleDepth(title)) +
title.pad.t +
title.pad.b
);
}


Plots.allowAutoMargin(gd, titleID);
Plots.autoMargin(gd, titleID, push);
}

function getMainTitleX(fullLayout, textAnchor) {
Expand Down
21 changes: 13 additions & 8 deletions src/plots/plots.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1863,6 +1863,7 @@ function initMargins(fullLayout) {
}
if(!fullLayout._pushmargin) fullLayout._pushmargin = {};
if(!fullLayout._pushmarginIds) fullLayout._pushmarginIds = {};
if(!fullLayout._reservedMargin) fullLayout._reservedMargin = {'t': 0, 'b': 0, 'l': 0, 'r': 0};
}

// non-negotiable - this is the smallest height we will allow users to specify via explicit margins
Expand Down Expand Up @@ -1980,6 +1981,7 @@ plots.doAutoMargin = function(gd) {

var gs = fullLayout._size;
var margin = fullLayout.margin;
var reservedMargins = fullLayout._reservedMargin;
var oldMargins = Lib.extendFlat({}, gs);

// adjust margins for outside components
Expand Down Expand Up @@ -2017,14 +2019,16 @@ plots.doAutoMargin = function(gd) {
var pl = pushleft.size;
var fb = pushbottom.val;
var pb = pushbottom.size;
var nonReservedWidth = width - reservedMargins.r - reservedMargins.h;
var nonReservedHeight = height - reservedMargins.t - reservedMargins.b;

for(var k2 in pushMargin) {
if(isNumeric(pl) && pushMargin[k2].r) {
var fr = pushMargin[k2].r.val;
var pr = pushMargin[k2].r.size;
if(fr > fl) {
var newL = (pl * fr + (pr - width) * fl) / (fr - fl);
var newR = (pr * (1 - fl) + (pl - width) * (1 - fr)) / (fr - fl);
var newL = (pl * fr + (pr - nonReservedWidth) * fl) / (fr - fl);
var newR = (pr * (1 - fl) + (pl - nonReservedWidth) * (1 - fr)) / (fr - fl);
if(newL + newR > ml + mr) {
ml = newL;
mr = newR;
Expand All @@ -2036,8 +2040,8 @@ plots.doAutoMargin = function(gd) {
var ft = pushMargin[k2].t.val;
var pt = pushMargin[k2].t.size;
if(ft > fb) {
var newB = (pb * ft + (pt - height) * fb) / (ft - fb);
var newT = (pt * (1 - fb) + (pb - height) * (1 - ft)) / (ft - fb);
var newB = (pb * ft + (pt - nonReservedHeight) * fb) / (ft - fb);
var newT = (pt * (1 - fb) + (pb - nonReservedHeight) * (1 - ft)) / (ft - fb);
if(newB + newT > mb + mt) {
mb = newB;
mt = newT;
Expand Down Expand Up @@ -2079,10 +2083,11 @@ plots.doAutoMargin = function(gd) {
}
}

gs.l = Math.round(ml);
gs.r = Math.round(mr);
gs.t = Math.round(mt);
gs.b = Math.round(mb);

gs.l = Math.round(ml) + reservedMargins.l;
gs.r = Math.round(mr) + reservedMargins.r;
gs.t = Math.round(mt) + reservedMargins.t;
gs.b = Math.round(mb) + reservedMargins.b;
gs.p = Math.round(margin.pad);
gs.w = Math.round(width) - gs.l - gs.r;
gs.h = Math.round(height) - gs.t - gs.b;
Expand Down
7 changes: 5 additions & 2 deletions test/image/mocks/zzz-automargin-title-container.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@
"height": 300,
"width": 400,
"margin": {"t":0, "b": 0, "l": 0, "r": 0},
"xaxis": {"automargin": true, "title": {"text": "x-axis title", "standoff": 0}},
Copy link
Contributor Author

@hannahker hannahker Mar 7, 2023

Choose a reason for hiding this comment

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

@alexcjohnson - The bottom margin ends up being set to the correct size to account for both the plot title and the axis, but the axis title then still moves to overlap with the plot title when given enough margin space. Manually setting the standoff to 0 fixes this, but I can't quite pinpoint why this is the case (as I don't see the margin size or plot height as inputs to the automatic standoff calculation in the code. Do you have any thoughts on where this might be coming from?

Copy link
Collaborator

@alexcjohnson alexcjohnson Mar 8, 2023

Choose a reason for hiding this comment

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

Looks to me like it's probably this block, which creates a default standoff that's not recorded anywhere or captured anywhere else, just fed into where the axis title is drawn:

plotly.js/src/plots/cartesian/axes.js

Lines 3973 to 3998 in 2b4584d

var isInside = insideTicklabelposition(ax);
if(ax.type === 'multicategory') {
titleStandoff = ax._depth;
} else {
var offsetBase = 1.5 * fontSize;
if(isInside) {
offsetBase = 0.5 * fontSize;
if(ax.ticks === 'outside') {
offsetBase += ax.ticklen;
}
}
titleStandoff = 10 + offsetBase + (ax.linewidth ? ax.linewidth - 1 : 0);
}
if(!isInside) {
if(axLetter === 'x') {
titleStandoff += ax.side === 'top' ?
fontSize * (ax.showticklabels ? 1 : 0) :
fontSize * (ax.showticklabels ? 1.5 : 0.5);
} else {
titleStandoff += ax.side === 'right' ?
fontSize * (ax.showticklabels ? 1 : 0.5) :
fontSize * (ax.showticklabels ? 0.5 : 0);
}
}

The ideal solution would be put all that logic into title.standoff in supplyDefaults, rather than at the draw step.

BTW @archmoj I notice standoff is incorrect for multicategory axes with side: 'top'... didn't look at left or right but I bet one of those is incorrect too.

Copy link
Contributor Author

@hannahker hannahker Mar 9, 2023

Choose a reason for hiding this comment

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

I was looking into this block as well but it's not clear to me how a change in margin size impacts the standoff value. By my reading, it's only the font size and axis properties (like tick length and line width) that we're looking at here. I could be missing something obvious since I've been staring at this block for a while.

Copy link
Contributor Author

@hannahker hannahker Mar 9, 2023

Choose a reason for hiding this comment

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

Look to scootTitle function in main Titles.draw

"title": {
"automargin": true,
"text": "Basic title",
"font": {"size": 24},
"pad": {"t": 10, "b": 10}
"pad": {"t": 0, "b": 0},
"y": 0,
"yref": "container"
}
}
}
}