Fix rangeslider titles with stacked y axes #2451
Conversation
we were sorting too late, so if 'y2' was referenced before 'y' in gd.data, it would become the default anchor for x axes.
src/components/rangeslider/draw.js
Outdated
| y: y + opts._height + opts._offsetShift + 10 + 1.5 * axisOpts.titlefont.size, | ||
| 'text-anchor': 'middle' | ||
| } | ||
| }); |
There was a problem hiding this comment.
Where to put this Titles.draw call totally confused me. First I put it after Plots.automargin below... that failed, calling Titles.draw twice: first with the adjusted margins and THEN with the un-adjusted margins - the reason being that Plots.autoMargin invokes a whole new Plotly.plot call, and in that one we get the adjusted margin, but then after that we come back here and draw the title with the original margin. Instead, I had to put it here, so that the draw with the original margin happens before we adjust the margins, then the draw with the adjusted margin happens inside Plots.autoMargin. Or I suppose I could have added some MathJax so Plots.autoMargin would be async, that would have fixed it too 🙄
@etpinard this is what got me thinking about altering our plotting pipeline with some intermediate step that's just concerned with arranging all the components but doesn't draw anything other than prerendering and stashing all the text we need sizes for.
Also, I started trying to pull out the 10 + 1.5 * axisOpts.titlefont.size that's repeated from Axes.doTicks into some sort of Titles.offset function (that's why I started changing the structure in titles/index), but gave up for now as there's too much logic concerning side and other axis attributes in doTicks.
| // title goes next to range slider instead of tick labels, so | ||
| // just take it over and draw it from here | ||
| if(axisOpts.side === 'bottom') { | ||
| Titles.draw(gd, axisOpts._id + 'title', { |
| placeholder: fullLayout._dfltTitle.x, | ||
| attributes: { | ||
| x: axisOpts._offset + axisOpts._length / 2, | ||
| y: y + opts._height + opts._offsetShift + 10 + 1.5 * axisOpts.titlefont.size, |
There was a problem hiding this comment.
Here, that 10 px offset comes from eyeballing it?
There was a problem hiding this comment.
... or is there a similar offset for range-slider-less axes?
There was a problem hiding this comment.
copied from doTicks
plotly.js/src/plots/cartesian/axes.js
Line 2031 in dfe13b5
That's what made me start to try and abstract this out, before deciding the logic was a little too convoluted to want to tackle it right now.
| @@ -1390,6 +1385,11 @@ plots.supplyLayoutModuleDefaults = function(layoutIn, layoutOut, fullData, trans | |||
| Cartesian.finalizeSubplots(layoutIn, layoutOut); | |||
| } | |||
|
|
|||
| // sort subplot lists | |||
| for(var subplotType in layoutOut._subplots) { | |||
| layoutOut._subplots[subplotType].sort(Lib.subplotSort); | |||
There was a problem hiding this comment.
Yes, I think so, at least that was the main part of it.
| "rangeslider": {} | ||
| "rangeslider": {}, | ||
| "title": "X", | ||
| "side": "top" |
| "layout": { | ||
| "xaxis": {"rangeslider": {}, "title": "x"}, | ||
| "yaxis": {"title": "y", "domain": [0, 0.25]}, | ||
| "yaxis2": {"title": "y2", "domain": [ 0.3, 1]} |
There was a problem hiding this comment.
Referencing #2172 and more specifically #2172 (comment) - so if I understand correctly, the baseline for this mock is wrong as the desired behavior would be replicate the stacked y-axes in the range slider?
| var tickHeight = (axisOpts._boundingBox || {}).height || 0; | ||
|
|
||
| var oppBottom = Infinity; | ||
| var subplotData = Axes.getSubplots(gd, axisOpts); |
There was a problem hiding this comment.
... I hope these patched lines won't conflict with #2364
|
Great. Looks like all my questions have been answered 💃 (or maybe wait until #2364 is merged - your call) |
Fixes #2443 - this was actually two issues in one:
y2showed up in the data beforey, theny2became the default.I also noticed while working on this that titles with
xaxis.side = 'top'was mangled. I simplified the whole business by having the rangeslider itself draw the (bottom) title and bailing out of title drawing inAxes.doTicks.cc @etpinard