Fix axis line width, length, and positioning for coupled subplots #1854
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -174,29 +174,116 @@ exports.lsInner = function(gd) { | |
| (ax.mirrors && ax.mirrors[counterAx._id + side]); | ||
| } | ||
|
|
||
| var xIsFree = xa.anchor === 'free'; | ||
| var showFreeX = xIsFree && !freeFinished[xa._id]; | ||
| var showBottom = shouldShowLine(xa, ya, 'bottom'); | ||
| var showTop = shouldShowLine(xa, ya, 'top'); | ||
|
|
||
| var yIsFree = ya.anchor === 'free'; | ||
| var showFreeY = yIsFree && !freeFinished[ya._id]; | ||
| var showLeft = shouldShowLine(ya, xa, 'left'); | ||
| var showRight = shouldShowLine(ya, xa, 'right'); | ||
|
|
||
| var xlw = Drawing.crispRound(gd, xa.linewidth, 1); | ||
| var ylw = Drawing.crispRound(gd, ya.linewidth, 1); | ||
|
|
||
| function findMainAxis(ax) { | ||
| return ax.overlaying ? Plotly.Axes.getFromId(gd, ax.overlaying) : ax; | ||
| } | ||
|
|
||
| function findCounterAxes(ax) { | ||
|
There was a problem hiding this comment. Would it be too much to ask to move those new There was a problem hiding this comment. ah, I actually meant to un-nest these, thanks for reminding me. b31e60d |
||
| var counterAxes = []; | ||
| var anchorAx = Plotly.Axes.getFromId(gd, ax.anchor); | ||
| if(anchorAx) { | ||
| var counterMain = findMainAxis(anchorAx); | ||
| if(counterAxes.indexOf(counterMain) === -1) { | ||
| counterAxes.push(counterMain); | ||
| } | ||
| for(var i = 0; i < axList.length; i++) { | ||
| if(axList[i].overlaying === counterMain._id && | ||
| counterAxes.indexOf(axList[i]) === -1 | ||
| ) { | ||
| counterAxes.push(axList[i]); | ||
| } | ||
| } | ||
| } | ||
| return counterAxes; | ||
| } | ||
|
|
||
| function findLineWidth(axes, side) { | ||
| for(var i = 0; i < axes.length; i++) { | ||
| var ax = axes[i]; | ||
| if(ax.anchor !== 'free' && shouldShowLine(ax, {_id: ax.anchor}, side)) { | ||
| return Drawing.crispRound(gd, ax.linewidth); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function findCounterAxisLineWidth(ax, subplotCounterLineWidth, | ||
| subplotCounterIsShown, side) { | ||
| if(subplotCounterIsShown) return subplotCounterLineWidth; | ||
|
|
||
| var i; | ||
|
|
||
| // find all counteraxes for this one, then of these, find the | ||
| // first one that has a visible line on this side | ||
| var mainAxis = findMainAxis(ax); | ||
| var counterAxes = findCounterAxes(mainAxis); | ||
|
|
||
| var lineWidth = findLineWidth(counterAxes, side); | ||
| if(lineWidth) return lineWidth; | ||
|
|
||
| for(i = 0; i < axList.length; i++) { | ||
| if(axList[i].overlaying === mainAxis._id) { | ||
| counterAxes = findCounterAxes(axList[i]); | ||
| lineWidth = findLineWidth(counterAxes, side); | ||
| if(lineWidth) return lineWidth; | ||
| } | ||
| } | ||
| return 0; | ||
| } | ||
|
|
||
| /* | ||
| * x lines get longer where they meet y lines, to make a crisp corner | ||
| * free x lines are not excluded - they don't necessarily *meet* the | ||
| * y lines, but it still looks good if the x line reaches to the ends | ||
| * of the y lines, especially in the case of a free axis parallel to | ||
| * an anchored axis, like this: | ||
|
There was a problem hiding this comment. I agree 100%. Thanks for documenting. |
||
| * | ||
| * | | ||
| * | | ||
| * +----- | ||
| * x1 | ||
| * ------ | ||
| * ^ x2 | ||
| */ | ||
| var xLinesXLeft = -pad - findCounterAxisLineWidth(xa, ylw, showLeft, 'left'); | ||
| var xLinesXRight = xa._length + pad + findCounterAxisLineWidth(xa, ylw, showRight, 'right'); | ||
| var xLinesYFree = gs.h * (1 - (xa.position || 0)) + ((xlw / 2) % 1); | ||
| var xLinesYBottom = ya._length + pad + xlw / 2; | ||
| var xLinesYTop = -pad - xlw / 2; | ||
|
|
||
| /* | ||
| * y lines do not get longer when they meet x axes, because the | ||
| * x axis already filled that space and we don't want to double-fill. | ||
| * BUT they get longer if they're free axes, for the same reason as | ||
| * we do not exclude x axes: | ||
|
|
||
| * | ||
| * | | | ||
| * y2| y1| | ||
| * | | | ||
| * >| +----- | ||
| * | ||
| * arguably if the free y axis is over top of the anchored x axis, | ||
| * we don't want to do this... but that's a weird edge case, doesn't | ||
| * seem worth adding a lot of complexity for. | ||
| */ | ||
| var yLinesYBottom = ya._length + pad + (yIsFree ? | ||
| findCounterAxisLineWidth(ya, xlw, showBottom, 'bottom') : | ||
| 0); | ||
| var yLinesYTop = -pad - (yIsFree ? | ||
| findCounterAxisLineWidth(ya, xlw, showTop, 'top') : | ||
| 0); | ||
| var yLinesXFree = gs.w * (ya.position || 0) + ((ylw / 2) % 1); | ||
| var yLinesXLeft = -pad - ylw / 2; | ||
| var yLinesXRight = xa._length + pad + ylw / 2; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1684,8 +1684,7 @@ axes.doTicks = function(gd, axid, skipTitle) { | |
| gcls = axid + 'grid', | ||
| zcls = axid + 'zl', | ||
| pad = (ax.linewidth || 1) / 2, | ||
| labelStandoff = (ax.ticks === 'outside' ? ax.ticklen : 0), | ||
| labelShift = 0, | ||
| gridWidth = Drawing.crispRound(gd, ax.gridwidth, 1), | ||
| zeroLineWidth = Drawing.crispRound(gd, ax.zerolinewidth, gridWidth), | ||
|
|
@@ -1695,10 +1694,14 @@ axes.doTicks = function(gd, axid, skipTitle) { | |
|
|
||
| if(ax._counterangle && ax.ticks === 'outside') { | ||
| var caRad = ax._counterangle * Math.PI / 180; | ||
| labelStandoff = ax.ticklen * Math.cos(caRad) + 1; | ||
| labelShift = ax.ticklen * Math.sin(caRad); | ||
| } | ||
|
|
||
| if(ax.ticks === 'outside' || ax.showline) { | ||
| labelStandoff += 0.2 * ax.tickfont.size; | ||
| } | ||
|
|
||
| // positioning arguments for x vs y axes | ||
| if(axLetter === 'x') { | ||
| sides = ['bottom', 'top']; | ||
|
|
@@ -1781,7 +1784,7 @@ axes.doTicks = function(gd, axid, skipTitle) { | |
| labelpos0 = position + (labelStandoff + pad) * flipit; | ||
| labely = function(d) { | ||
| return d.dy + labelpos0 + d.fontSize * | ||
| ((axside === 'bottom') ? 1 : -0.2); | ||
| }; | ||
| labelanchor = function(angle) { | ||
| if(!isNumeric(angle) || angle === 0 || angle === 180) { | ||
|
|
@@ -1792,7 +1795,9 @@ axes.doTicks = function(gd, axid, skipTitle) { | |
| } | ||
| else { | ||
| flipit = (axside === 'right') ? 1 : -1; | ||
| labely = function(d) { | ||
| return d.dy + d.fontSize * 0.35 - labelShift * flipit; | ||
|
There was a problem hiding this comment. Non- ⛔ (might be better suited for #1434 - the part where we proposed making it configurable) but this There was a problem hiding this comment. hah, I found the 0.35 value empirically, but polar had the same one, and it turns out there's a rational basis for exactly this value. 16bae9c There was a problem hiding this comment. indeed! just out of curiosity, would There was a problem hiding this comment.
Quite possibly! I recall some baseline properties that I tried to use in the past not having sufficient cross-browser support, so we would need to test it. Also I'm not sure how this would work with pseudo-html (subscript/superscript, changing font sizes, multiple lines...) so I'm not going to do it now but we could investigate later. There was a problem hiding this comment. If you run into such a thing I'm interested as the upcoming |
||
| }; | ||
| labelx = function(d) { | ||
| return d.dx + position + (labelStandoff + pad + | ||
| ((Math.abs(ax.tickangle) === 90) ? d.fontSize / 2 : 0)) * flipit; | ||
|
|
@@ -2019,16 +2024,23 @@ axes.doTicks = function(gd, axid, skipTitle) { | |
| avoid.offsetTop = translation.y; | ||
| } | ||
|
|
||
| var titleStandoff = 10 + fontSize * offsetBase + | ||
| (ax.linewidth ? ax.linewidth - 1 : 0); | ||
|
|
||
| if(axLetter === 'x') { | ||
| counterAxis = (ax.anchor === 'free') ? | ||
| {_offset: gs.t + (1 - (ax.position || 0)) * gs.h, _length: 0} : | ||
| axisIds.getFromId(gd, ax.anchor); | ||
|
|
||
| x = ax._offset + ax._length / 2; | ||
| if(ax.side === 'top') { | ||
| y = -titleStandoff - fontSize * (ax.showticklabels ? 1 : 0); | ||
| } | ||
| else { | ||
| y = counterAxis._length + titleStandoff + | ||
| fontSize * (ax.showticklabels ? 1.5 : 0.5); | ||
| } | ||
| y += counterAxis._offset; | ||
|
|
||
| if(ax.rangeslider && ax.rangeslider.visible && ax._boundingBox) { | ||
| y += (fullLayout.height - fullLayout.margin.b - fullLayout.margin.t) * | ||
|
|
@@ -2043,10 +2055,14 @@ axes.doTicks = function(gd, axid, skipTitle) { | |
| axisIds.getFromId(gd, ax.anchor); | ||
|
|
||
| y = ax._offset + ax._length / 2; | ||
| if(ax.side === 'right') { | ||
| x = counterAxis._length + titleStandoff + | ||
| fontSize * (ax.showticklabels ? 1 : 0.5); | ||
| } | ||
| else { | ||
| x = -titleStandoff - fontSize * (ax.showticklabels ? 0.5 : 0); | ||
| } | ||
| x += counterAxis._offset; | ||
|
|
||
| transform = {rotate: '-90', offset: 0}; | ||
| if(!avoid.side) avoid.side = 'left'; | ||
|
|
||
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Non- ⛔ : might be nice to stash references to overlaying and anchor axes in
plotinfoduringmakeSubplotDataand 🔪 thoseAxes.getFromIdThere 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.
good idea. Actually
makeSubplotDatadidn't really seem the place for this, it duplicated a lot of effort that should have just been done once per axis. I put this all intoplots.linkSubplots. 274526a