Fix axis line width, length, and positioning for coupled subplots #1854
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,10 +41,16 @@ function overlappingDomain(xDomain, yDomain, domains) { | |
| } | ||
|
|
||
| exports.lsInner = function(gd) { | ||
| var fullLayout = gd._fullLayout; | ||
| var gs = fullLayout._size; | ||
| var pad = gs.p; | ||
| var axList = Plotly.Axes.list(gd); | ||
|
|
||
| // _has('cartesian') means SVG specifically, not GL2D - but GL2D | ||
| // can still get here because it makes some of the SVG structure | ||
| // for shared features like selections. | ||
| var hasSVGCartesian = fullLayout._has('cartesian'); | ||
| var i; | ||
|
|
||
| // clear axis line positions, to be set in the subplot loop below | ||
| for(i = 0; i < axList.length; i++) axList[i]._linepositions = {}; | ||
|
|
@@ -79,11 +85,9 @@ exports.lsInner = function(gd) { | |
| return; | ||
| } | ||
|
|
||
| var xDomain = plotinfo.xaxis.domain; | ||
|
There was a problem hiding this comment. Amazing. Note that references in |
||
| var yDomain = plotinfo.yaxis.domain; | ||
| var plotgroupBgData = []; | ||
|
|
||
| if(overlappingDomain(xDomain, yDomain, lowerDomains)) { | ||
| plotgroupBgData = [0]; | ||
|
|
@@ -124,22 +128,21 @@ exports.lsInner = function(gd) { | |
| fullLayout._plots[subplot].bg = d3.select(this); | ||
| }); | ||
|
|
||
| var freeFinished = {}; | ||
| subplotSelection.each(function(subplot) { | ||
| var plotinfo = fullLayout._plots[subplot]; | ||
| var xa = plotinfo.xaxis; | ||
| var ya = plotinfo.yaxis; | ||
|
|
||
| // reset scale in case the margins have changed | ||
| xa.setScale(); | ||
| ya.setScale(); | ||
|
|
||
| if(plotinfo.bg && hasSVGCartesian) { | ||
| plotinfo.bg | ||
| .call(Drawing.setRect, | ||
| xa._offset - pad, ya._offset - pad, | ||
| xa._length + 2 * pad, ya._length + 2 * pad) | ||
| .call(Color.fill, fullLayout.plot_bgcolor) | ||
| .style('stroke-width', 0); | ||
| } | ||
|
|
@@ -165,129 +168,157 @@ exports.lsInner = function(gd) { | |
| }); | ||
|
|
||
|
|
||
| plotinfo.plot | ||
| .call(Drawing.setTranslate, xa._offset, ya._offset) | ||
| .call(Drawing.setClipUrl, plotinfo.clipId); | ||
|
|
||
| var xIsFree = !xa._anchorAxis; | ||
| var showFreeX = xIsFree && !freeFinished[xa._id]; | ||
| var showBottom = shouldShowLine(xa, ya, 'bottom'); | ||
| var showTop = shouldShowLine(xa, ya, 'top'); | ||
|
|
||
| var yIsFree = !ya._anchorAxis; | ||
| 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); | ||
|
|
||
| /* | ||
| * 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(gd, xa, ylw, showLeft, 'left', axList); | ||
| var xLinesXRight = xa._length + pad + findCounterAxisLineWidth(gd, xa, ylw, showRight, 'right', axList); | ||
| 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(gd, ya, xlw, showBottom, 'bottom', axList) : | ||
| 0); | ||
| var yLinesYTop = -pad - (yIsFree ? | ||
| findCounterAxisLineWidth(gd, ya, xlw, showTop, 'top', axList) : | ||
| 0); | ||
| var yLinesXFree = gs.w * (ya.position || 0) + ((ylw / 2) % 1); | ||
| var yLinesXLeft = -pad - ylw / 2; | ||
| var yLinesXRight = xa._length + pad + ylw / 2; | ||
|
|
||
| function xLinePath(y, showThis) { | ||
| if(!showThis) return ''; | ||
| return 'M' + xLinesXLeft + ',' + y + 'H' + xLinesXRight; | ||
| } | ||
|
|
||
| function yLinePath(x, showThis) { | ||
| if(!showThis) return ''; | ||
| return 'M' + x + ',' + yLinesYTop + 'V' + yLinesYBottom; | ||
| } | ||
|
|
||
| // save axis line positions for ticks, draggers, etc to reference | ||
| // each subplot gets an entry: | ||
| // [left or bottom, right or top, free, main] | ||
| // main is the position at which to draw labels and draggers, if any | ||
| xa._linepositions[subplot] = [ | ||
| showBottom ? xLinesYBottom : undefined, | ||
| showTop ? xLinesYTop : undefined, | ||
| showFreeX ? xLinesYFree : undefined | ||
| ]; | ||
| if(xa._anchorAxis === ya) { | ||
| xa._linepositions[subplot][3] = xa.side === 'top' ? | ||
| xLinesYTop : xLinesYBottom; | ||
| } | ||
| else if(showFreeX) { | ||
| xa._linepositions[subplot][3] = xLinesYFree; | ||
| } | ||
|
|
||
| ya._linepositions[subplot] = [ | ||
| showLeft ? yLinesXLeft : undefined, | ||
| showRight ? yLinesXRight : undefined, | ||
| showFreeY ? yLinesXFree : undefined | ||
| ]; | ||
| if(ya._anchorAxis === xa) { | ||
| ya._linepositions[subplot][3] = ya.side === 'right' ? | ||
| yLinesXRight : yLinesXLeft; | ||
| } | ||
| else if(showFreeY) { | ||
| ya._linepositions[subplot][3] = yLinesXFree; | ||
| } | ||
|
|
||
| // translate all the extra stuff to have the | ||
| // same origin as the plot area or axes | ||
| var origin = 'translate(' + xa._offset + ',' + ya._offset + ')'; | ||
| var originX = origin; | ||
| var originY = origin; | ||
| if(showFreeX) { | ||
| originX = 'translate(' + xa._offset + ',' + gs.t + ')'; | ||
| xLinesYTop += ya._offset - gs.t; | ||
| xLinesYBottom += ya._offset - gs.t; | ||
| } | ||
| if(showFreeY) { | ||
| originY = 'translate(' + gs.l + ',' + ya._offset + ')'; | ||
| yLinesXLeft += xa._offset - gs.l; | ||
| yLinesXRight += xa._offset - gs.l; | ||
| } | ||
|
|
||
| if(hasSVGCartesian) { | ||
| plotinfo.xlines | ||
| .attr('transform', originX) | ||
| .attr('d', ( | ||
| xLinePath(xLinesYBottom, showBottom) + | ||
| xLinePath(xLinesYTop, showTop) + | ||
| xLinePath(xLinesYFree, showFreeX)) || | ||
| // so it doesn't barf with no lines shown | ||
| 'M0,0') | ||
| .style('stroke-width', xlw + 'px') | ||
| .call(Color.stroke, xa.showline ? | ||
| xa.linecolor : 'rgba(0,0,0,0)'); | ||
| plotinfo.ylines | ||
| .attr('transform', originY) | ||
| .attr('d', ( | ||
| yLinePath(yLinesXLeft, showLeft) + | ||
| yLinePath(yLinesXRight, showRight) + | ||
| yLinePath(yLinesXFree, showFreeY)) || | ||
| 'M0,0') | ||
| .style('stroke-width', ylw + 'px') | ||
|
There was a problem hiding this comment. Good catch. Is There was a problem hiding this comment. Yes, a lot of properties in SVG you can use either There was a problem hiding this comment. Ha, interesting. It's something we might to standardize in There was a problem hiding this comment. I think There was a problem hiding this comment. ... also, there's the thing that Microsoft browsers (IE, Edge) don't have full CSS support, so if the decision is to go with There was a problem hiding this comment. Right, I'm sympathetic to the idea of giving users more control, but in the end I think it would be a mistake to encourage overriding internals of the plot with CSS - we should push people to use the plot JSON instead - for three reasons:
There was a problem hiding this comment. Yes these points are all true. We don't currently go out of our way to dissuade scenegraph class based styling - we don't generate mangled or one-off class names or otherwise actively dissuade external styling. Also on the forum, SO and gh questions sometimes our answer is that a particular wish isn't supported but can be done with a Not sure if the emerging linter solution already covers it, I think this is the list of attributes (ie. all presentation attributes) that can go either way. Unfortunately it's quite Venn-diagrammey and common stuff like |
||
| .call(Color.stroke, ya.showline ? | ||
| ya.linecolor : 'rgba(0,0,0,0)'); | ||
| } | ||
|
|
||
| plotinfo.xaxislayer.attr('transform', originX); | ||
| plotinfo.yaxislayer.attr('transform', originY); | ||
| plotinfo.gridlayer.attr('transform', origin); | ||
| plotinfo.zerolinelayer.attr('transform', origin); | ||
| plotinfo.draglayer.attr('transform', origin); | ||
|
|
||
| // mark free axes as displayed, so we don't draw them again | ||
| if(showFreeX) freeFinished[xa._id] = 1; | ||
| if(showFreeY) freeFinished[ya._id] = 1; | ||
|
There was a problem hiding this comment. Smart. I usually set keys like this as |
||
| }); | ||
|
|
||
| Plotly.Axes.makeClipPaths(gd); | ||
|
|
@@ -297,6 +328,65 @@ exports.lsInner = function(gd) { | |
| return gd._promises.length && Promise.all(gd._promises); | ||
| }; | ||
|
|
||
| function shouldShowLine(ax, counterAx, side) { | ||
| return (ax._anchorAxis === counterAx && (ax.mirror || ax.side === side)) || | ||
| ax.mirror === 'all' || ax.mirror === 'allticks' || | ||
| (ax.mirrors && ax.mirrors[counterAx._id + side]); | ||
| } | ||
|
|
||
| function findCounterAxes(gd, ax, axList) { | ||
| var counterAxes = []; | ||
| var anchorAx = ax._anchorAxis; | ||
| if(anchorAx) { | ||
| var counterMain = anchorAx._mainAxis; | ||
| 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(gd, axes, side) { | ||
| for(var i = 0; i < axes.length; i++) { | ||
| var ax = axes[i]; | ||
| var anchorAx = ax._anchorAxis; | ||
| if(anchorAx && shouldShowLine(ax, anchorAx, side)) { | ||
| return Drawing.crispRound(gd, ax.linewidth); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function findCounterAxisLineWidth(gd, ax, subplotCounterLineWidth, | ||
| subplotCounterIsShown, side, axList) { | ||
| 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 = ax._mainAxis; | ||
| var counterAxes = findCounterAxes(gd, mainAxis, axList); | ||
|
|
||
| var lineWidth = findLineWidth(gd, counterAxes, side); | ||
| if(lineWidth) return lineWidth; | ||
|
|
||
| for(i = 0; i < axList.length; i++) { | ||
| if(axList[i].overlaying === mainAxis._id) { | ||
| counterAxes = findCounterAxes(gd, axList[i], axList); | ||
| lineWidth = findLineWidth(gd, counterAxes, side); | ||
| if(lineWidth) return lineWidth; | ||
| } | ||
| } | ||
| return 0; | ||
| } | ||
|
|
||
| exports.drawMainTitle = function(gd) { | ||
| var fullLayout = gd._fullLayout; | ||
|
|
||
|
|
||
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.
To note, this is needed as gl2d now uses cartesian (SVG layers) subplots to handle selections.
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.
Ah right - I figured out that that's the reason, which is why I added
SVGto the var name, but perhaps I should add the full explanation in a comment. Will become important when we start using SVG axes with GL2D plots @dfcreativeThere 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.
comment added 6d41a37