automatic margins for long labels #2243

nicolaskruchten · 2018-01-10T20:37:24Z

Proof of concept for addressing #296 and #1504.

The Plots.autoMargin and calcBoundingBox machinery is all already in place so I've wired them together such that margins can only grow, otherwise it's easier to trigger interactions between various axes such that the layout machinery loops without converging. Unfortunately this means that it's possible to get into situations where e.g. the left-hand margin is big enough to accommodate some long Y-axis labels which are then hidden because some long X-axis labels caused the plot to become shorter.

Known issues with this PR:

the axis autoMargin calls don't mesh very well with the legend autoMargin calls, so it's still possible for the legend to sit on top of tick labels. Resolving that seems like it would involve modifying Plots.doAutoMargin to sum the requested _pushmargins somehow. This is a known issue: Add legend 'auto' x|y and/or 'container' (x|y)ref #1199
this behaviour should have some controls associated with it to turn it on/off

nicolaskruchten · 2018-01-10T20:38:09Z

Also note that I didn't test this with subplots, I wanted to kick off some discussion first :)

nicolaskruchten · 2018-01-10T20:39:31Z

And here's what the example looks like, with a legend, two Y axes and some big axis titles:

etpinard · 2018-01-11T16:48:49Z

This looks like a very solid first step. 💪

this behaviour should have some controls associated with it to turn it on/off

I think so. I'd vote for something like layout.automargins which would be false by default in v1.x and true starting in v2.

This new attribute will require some fancyeditType and relayout logic similar to autorange to work correctly on updates. Moreover, we should make sure that labels automargins are only computed when needed (e.g. when axis labels or the plot domain change). Plots.doAutoMargins can slow things down.

We should also test various combinations of things that can push margins e.g. colorbars, legend, range sliders, update menus and layout sliders (anything else?).

Working this patch in Axes.doTicks should make this new logic work automatically with ternary and polar (coming soon), but we'll need to patch pie/plot.js to make sure pie labels get the same treatment - that could be in a different PR though 😉

One final comment, should anchor: 'free' axes also be allowed to bump the margins? I think so, but that might be debatable and/or pretty tricky to get right in situations like mock 20.

nicolaskruchten · 2018-01-11T17:18:54Z

Re the testing with other things that push margins, it behaves just as detailed in #1199 ... no improvement but no worse either. The margins move such that there is enough room for the labels, and then the legend still sits on top of the labels. So I would consider that a separate issue that we can tackle as part of solving #1199 etc?

etpinard · 2018-01-29T21:08:29Z

src/plots/cartesian/tick_label_defaults.js

@@ -50,6 +50,8 @@ module.exports = function handleTickLabelDefaults(containerIn, containerOut, coe
 }

 if(axType !== 'category' && !options.noHover) coerce('hoverformat');
+
+ coerce('ticklabelsautomargin');


@nicolaskruchten you'll have to add an option to opt-out of coercing ticklabelautomargin in handleTickLabelDefaults so that other callees

don't cough on it.

nicolaskruchten · 2018-01-30T17:02:15Z

Thanks for the hint @etpinard ! Now I can't figure out why this PR is breaking all the gl3d_* image tests... it seems to be dropping all ticklabels from those image tests but I don't understand the mechanism.

etpinard

@nicolaskruchten the gl3d image tests are now passing on CI after clearing the cache (i.e. rm node_modules/). The image tests are still failing as they are looking for the corresponding baseline PNG for your long_axis_labels.json mock.

You might want to try generating that baseline image locally. The instructions are here. Let me know if you need help.

etpinard · 2018-01-31T15:57:17Z

src/plots/cartesian/tick_label_defaults.js

@@ -50,6 +50,8 @@ module.exports = function handleTickLabelDefaults(containerIn, containerOut, coe
 }

 if(axType !== 'category' && !options.noHover) coerce('hoverformat');
+
+ if(axType === 'cartesian') coerce('ticklabelsautomargin');


Oh this won't work. axType is either 'linear', 'date', 'log' or 'category'.

I think the easiest way would to add an options.automargin flag set to true when called from plots/cartesian/axis_defaults.js.

Oh and does coerce('ticklabelsautomargin') have an effect when showticklabels is false? More precisely, do the axis title font settings and/or ticklen affect the auto-margin computations?

I'm not sure what coerce actually does, to be honest, so I'm not sure if it will have an impact. What exactly does it do? Is it necessary here?

To answer your question more generally, the auto-margin calculation explicitly takes the axis title font size into account, and for the rest relies on the pre-existing bounding-box calculation.

I'm not sure what coerce actually does,

In brief, coerce fills in fullLayout with a sanitise version of its user layout value. Almost everywhere downstream of the Plots.supplyDefaults uses fullLayout and fullData value (i.e. not gd.layout and gd.data) including your new logic in axes.js.

So, yes it is necessary 😄

the auto-margin calculation explicitly takes the axis title font size into account,

Ok great. So, auto-margin doesn't just depend on tick settings like the attribute name ticklabelsautomargin is suggesting. We'll need to think about how to name this new attribute some more cc #2243 (comment)

for the rest relies on the pre-existing bounding-box calculation.

Ok, so this should depend on ticklen (we should double-check though). Knowing which attributes impact the auto-margin computation is important to make sure relayout calls like Plotly.relayout(gd, 'xaxis.ticklen', /**/) update the (auto) margin when ticklabelsautomargin is turned on.

etpinard · 2018-01-31T16:07:14Z

src/plots/cartesian/layout_attributes.js

@@ -344,6 +344,16 @@ module.exports = {
 editType: 'ticks',
 description: 'Determines whether or not the tick labels are drawn.'
 },
+ ticklabelsautomargin: {


This might be too verbose. Would (x|y)axis.automargin suffice? Or maybe tickautomargin? Or perhaps we're looking at this the wrong way, something like:

var layout = { automargin: true || false, automarginmode: 'all' || 'ticks' || 'title' || 'xaxis.title' || 'legend' || ... // or any combination of these using '+', // a 'flaglist' attribute like scatter 'mode' }

might scale better.

@alexcjohnson @cldougl @chriddyp any thoughts on this?

+1 for automargin / automarginmode or at least (x|y)axis.automargin - this is more clear if attributes other than ticks/tick labels contributes to this (now or in the future).

nicolaskruchten · 2018-02-01T22:07:31Z

@etpinard which files should I look into for managing the dependencies among attributes for restyle/relayouts? I.e. the fact that my new option depends on showticklabels etc: how do I encode that?

nicolaskruchten · 2018-03-02T17:17:12Z

Ready for re-review.

alexcjohnson · 2018-03-02T17:45:22Z

src/plot_api/edit_types.js

@@ -47,6 +47,7 @@ var layoutOpts = {
 '*plot* calls `Plotly.plot` but without first clearing `gd.calcdata`.',
 '*legend* only redraws the legend.',
 '*ticks* only redraws axis ticks, labels, and gridlines.',
+ '*margins* recomputes ticklabel automargins.',


Ah yes, that's a much better solution 🎉

alexcjohnson · 2018-03-02T17:47:28Z

src/plots/polar/polar.js

+ _counteraxis: true,
+
+ // don't use automargins routine for labels
+ automargin: false


might be nice to let polar and ternary contribute to automargin in the future - but since this is an axis-level attribute I guess there's no problem ignoring that part for now.

alexcjohnson · 2018-03-02T17:48:36Z

Nice job @nicolaskruchten - nothing more from me, looks great! 💃

etpinard · 2018-03-02T20:11:19Z

All right! Time to merge this thing 💃

yniknafs · 2018-04-19T14:53:07Z

@etpinard Hey Étienne. Can't thank you enough for all your work on plotly. Whats the timeframe for merging these fresh PR merges into the distributed plotly package. Curious if I should pull the repo and make my own local copy or wait for it to be in the distribution.

etpinard · 2018-04-19T15:13:20Z

This PR has been released in 1.35.0 back on March 7 .

yniknafs · 2018-04-19T15:40:02Z

@etpinard

Gotcha, awesome. I just saw there was a new release yesterday, thanks for that.

I'm puling my hair out trying to figure out why my automargin isn't working. Not sure if you can help me at all. both the legend + axis label collide with the axis labels.

Here's my code for the layout:

const layout = (scale = 'linear', layoutData, name) => {
  let orientation = 'h';
  let subtitle = '';
  if (layoutData !== undefined) {
    orientation = (layoutData.legendOrientation === 'h') ? 'h' : 'v';
    subtitle = layoutData.subtitle;
  }

  const layoutObject = {
    title: (name) ? `<b>${name} (Expression)</b> <br> ${subtitle}` : '<b>No Gene Selected</b>',
    height: (layoutData) ? layoutData.height : 600,
    width: (layoutData) ? layoutData.width : '',
    autosize: true,
    showlegend: (layoutData) ? layoutData.showLegend : true,
    legend: {
      orientation,
    },
    font: {
      family: 'Helvetica',
    },
    yaxis: {
      title: (!name) ? '<b>TPM</b>' : `<b> ${name} (TPM) </b>`,
      automargin: true,
      showline: false,
      ticks: 'outside',
      showticklabels: true,
      rangemode: 'tozero',
      type: scale,
    },
    xaxis: {
      title: (layoutData) ? layoutData.xaxis : '',
      automargin: true,
      // showgrid: false,
      // zeroline: false,
      // showline: scale === 'log',
      // ticks: 'outside',
      // showticklabels: true,
      // type: 'category',
      // categoryorder: 'array',
      // categoryarray: [],
    },
  };

The data is a little complex, but I'll try to make a codepen to emulate the exact plot I'm doing if you don't have any ideas initially.

etpinard · 2018-04-19T15:42:45Z

That's part of the known issues in #2243 (comment)

the axis autoMargin calls don't mesh very well with the legend autoMargin calls, so it's still possible for the legend to sit on top of tick labels.

yniknafs · 2018-04-19T16:02:18Z

Awesome. thanks for the info. I think I got a bit confused as I was expecting the axis label to be below the longest label, but it seems like the behavior now is to be below the label that is directly above it?

nicolaskruchten · 2018-04-19T17:02:44Z

@yniknafs can you please create a new issue for this and I'll investigate?

nicolaskruchten · 2018-04-19T17:07:09Z

One thought on this is that the automargins code bails out if the new margins are bigger than half the plot dimension, which is likely what's happening in your screenshot above.

alexcjohnson · 2018-04-19T17:12:58Z

Aside from the legend overlap, this question about where the axis title goes has come up before - though I'm not aware of an issue discussing it. @yniknafs as you've deduced, we currently have the title avoid the bounding box of each tick label independently, so if there's a long label near the edge somewhere, that won't cause the title to shift outward (until you pan the axis to put that long label in conflict with the title, then it'll move). But you're right, in certain cases it would be nice to have a mode where the title stays out beyond the longest label even if they wouldn't actually cover each other.

nicolaskruchten · 2018-04-19T17:21:53Z

@alexcjohnson actually automargins is that mode, or it's intended to be, but as I said in some cases the pushmargins stuff bails out if it would result in a too-small plotting area.

alexcjohnson · 2018-04-19T17:45:19Z

actually automargins is that mode

Ah I see, didn't realize those two were coupled but you're right, that does generally work and seems like a good standard behavior. I still think it would be nice to be able to decouple them, ie by default it works like you have it but you could use another attribute to force either separate-label avoidance with automargins or all-label avoidance without.

yniknafs · 2018-04-20T13:03:54Z

@nicolaskruchten @alexcjohnson
Will great a new issue with this. FYI, I tried to make the plot bigger to test the 1/2size bailout, and it seems that no matter how large I make the plot, I still see the behavior.

johnbenac · 2019-02-17T07:43:00Z

Please add this to the 3D scene. It wont work in scatter3D. I'm getting the following error:

`ValueError: Invalid property specified for object of type plotly.graph_objs.layout.scene.XAxis: 'automargin'

Valid properties:
    autorange
        Determines whether or not the range of this axis is
        computed in relation to the input data. See `rangemode`
        for more info. If `range` is provided, then `autorange`
        is set to False.
    backgroundcolor
        Sets the background color of this axis' wall.
    calendar
        Sets the calendar system to use for `range` and `tick0`
        if this is a date axis. This does not set the calendar
        for interpreting data on this axis, that's specified in
        the trace or via the global `layout.calendar`
    categoryarray
        Sets the order in which categories on this axis appear.
        Only has an effect if `categoryorder` is set to
        "array". Used with `categoryorder`.
    categoryarraysrc
        Sets the source reference on plot.ly for  categoryarray
        .

`

because the automargins property belongs to "plotly.graph_objs.layout.XAxis:" and not "plotly.graph_objs.layout.scene.XAxis:"

For my particular scatter3d plots, I need to use the scene.

first cut at automatic margins for long labels

6797c9d

🔪 test code

f83fb55

etpinard added feature something new status: discussion needed labels Jan 11, 2018

alexcjohnson mentioned this pull request Jan 15, 2018

Add legend 'auto' x|y and/or 'container' (x|y)ref #1199

Closed

etpinard mentioned this pull request Jan 16, 2018

Tick text not in correct position in scattergl #1874

Closed

nicolaskruchten added 2 commits January 22, 2018 11:14

proper fix for top/right axes

eda678d

subplot-compatible generalization

e86cfea

jackparmer requested review from mdtusz and removed request for mdtusz January 25, 2018 00:59

alexcjohnson mentioned this pull request Jan 28, 2018

Implement notched box plots, closes #2286 krassowski/plotly.js#1

Open

nicolaskruchten added 2 commits January 29, 2018 14:04

Merge branch 'master' into label_automargin

8a5de93

creating default-false parameter

0ef6009

etpinard reviewed Jan 29, 2018

View reviewed changes

nicolaskruchten added 3 commits January 29, 2018 17:08

option guard

412941c

removing package-lock.json

bc0cf06

Merge branch 'master' into label_automargin

b2d49d5

etpinard reviewed Jan 31, 2018

View reviewed changes

nicolaskruchten added 4 commits January 31, 2018 13:50

adding baseline, fixing coerce check

25a36db

moving coercion around

d89f23e

Merge branch 'master' into label_automargin

8d0a5eb

ticklabelsautomargin -> automargin

67b04e9

nicolaskruchten added 2 commits February 27, 2018 15:27

Merge branch 'master' into label_automargin

05b909a

first relayout test

a05bd81

add editType to clear automargins from pushmargins

1df34bd

alexcjohnson reviewed Mar 2, 2018

View reviewed changes

etpinard added this to the v1.35.0 milestone Mar 2, 2018

nicolaskruchten merged commit 12e69c8 into master Mar 2, 2018

nicolaskruchten deleted the label_automargin branch March 2, 2018 20:21

This was referenced Mar 2, 2018

Axis titles overlapping axis tick labels #1504

Closed

Make auto-margin step dependent on axis label span #296

Closed

etpinard mentioned this pull request Mar 2, 2018

wishlist for potential breaking changes since v1 #420

Closed

15 tasks

deecay mentioned this pull request Mar 2, 2018

Revisit Plotly integration getredash/redash#2117

Closed

hms1 mentioned this pull request Mar 29, 2018

ggplotly - axis titles still overlapping with labels when converting faceted plot plotly/plotly.R#1224

Closed

yniknafs mentioned this pull request Apr 20, 2018

axis automargin moves axis label outside of individual tick labels, not the longest tick label #2575

Closed

Weiqi97 mentioned this pull request Jul 27, 2018

Manipulating plotly dendrograms WheatonCS/Lexos#789

Closed

anders-kiaer mentioned this pull request Sep 9, 2018

Fix tornado plot label position bug equinor/webviz-archived#79

Merged

YashRE42 mentioned this pull request Nov 29, 2021

Fix cut-off labels in /stats zulip/zulip#18912

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

automatic margins for long labels #2243

automatic margins for long labels #2243

nicolaskruchten commented Jan 10, 2018 •

edited

Loading

nicolaskruchten commented Jan 10, 2018

nicolaskruchten commented Jan 10, 2018

etpinard commented Jan 11, 2018 •

edited

Loading

nicolaskruchten commented Jan 11, 2018

etpinard Jan 29, 2018

nicolaskruchten commented Jan 30, 2018

etpinard left a comment

etpinard Jan 31, 2018

etpinard Jan 31, 2018 •

edited

Loading

nicolaskruchten Jan 31, 2018

etpinard Jan 31, 2018 •

edited

Loading

etpinard Jan 31, 2018

cldougl Jan 31, 2018

nicolaskruchten commented Feb 1, 2018

nicolaskruchten commented Mar 2, 2018

alexcjohnson Mar 2, 2018

alexcjohnson Mar 2, 2018

alexcjohnson commented Mar 2, 2018

etpinard commented Mar 2, 2018

yniknafs commented Apr 19, 2018

etpinard commented Apr 19, 2018

yniknafs commented Apr 19, 2018

etpinard commented Apr 19, 2018 •

edited

Loading

yniknafs commented Apr 19, 2018

nicolaskruchten commented Apr 19, 2018

nicolaskruchten commented Apr 19, 2018

alexcjohnson commented Apr 19, 2018

nicolaskruchten commented Apr 19, 2018

alexcjohnson commented Apr 19, 2018

yniknafs commented Apr 20, 2018

johnbenac commented Feb 17, 2019

automatic margins for long labels #2243

automatic margins for long labels #2243

Conversation

nicolaskruchten commented Jan 10, 2018 • edited Loading

nicolaskruchten commented Jan 10, 2018

nicolaskruchten commented Jan 10, 2018

etpinard commented Jan 11, 2018 • edited Loading

nicolaskruchten commented Jan 11, 2018

etpinard Jan 29, 2018

Choose a reason for hiding this comment

nicolaskruchten commented Jan 30, 2018

etpinard left a comment

Choose a reason for hiding this comment

etpinard Jan 31, 2018

Choose a reason for hiding this comment

etpinard Jan 31, 2018 • edited Loading

Choose a reason for hiding this comment

nicolaskruchten Jan 31, 2018

Choose a reason for hiding this comment

etpinard Jan 31, 2018 • edited Loading

Choose a reason for hiding this comment

etpinard Jan 31, 2018

Choose a reason for hiding this comment

cldougl Jan 31, 2018

Choose a reason for hiding this comment

nicolaskruchten commented Feb 1, 2018

nicolaskruchten commented Mar 2, 2018

alexcjohnson Mar 2, 2018

Choose a reason for hiding this comment

alexcjohnson Mar 2, 2018

Choose a reason for hiding this comment

alexcjohnson commented Mar 2, 2018

etpinard commented Mar 2, 2018

yniknafs commented Apr 19, 2018

etpinard commented Apr 19, 2018

yniknafs commented Apr 19, 2018

etpinard commented Apr 19, 2018 • edited Loading

yniknafs commented Apr 19, 2018

nicolaskruchten commented Apr 19, 2018

nicolaskruchten commented Apr 19, 2018

alexcjohnson commented Apr 19, 2018

nicolaskruchten commented Apr 19, 2018

alexcjohnson commented Apr 19, 2018

yniknafs commented Apr 20, 2018

johnbenac commented Feb 17, 2019

nicolaskruchten commented Jan 10, 2018 •

edited

Loading

etpinard commented Jan 11, 2018 •

edited

Loading

etpinard Jan 31, 2018 •

edited

Loading

etpinard Jan 31, 2018 •

edited

Loading

etpinard commented Apr 19, 2018 •

edited

Loading