Fix issue #384 (invisible legends when y is negative)

[n-riesco]

This a new solution to fix #384 that only touches Legend.draw()

[n-riesco]

I've commented out these two lines only to reproduce the baseline images, but I think they should be brought back.

[mdtusz]

This looks pretty good so far - could you elaborate on the commented out lx ly for the image baselines?

I'm going to continue testing to see if there's any issues but I haven't found anything yet.

[n-riesco]

The baseline images have been generated using lx and ly without rounding off.

When lx and ly are rounded off, many of the image tests (~75) fail with differences that are only 1 pixel.

Before introducing scrollbars, lx and ly were rounded off, and rounding off lx and ly makes it easier to reproduce plots in machines with different precisions.

[n-riesco]

• I noticed an issue in plots with many traces (~25). With these plots, when I drag the legend scrollbar, the mouse moves much faster than the bar. And the mouse wheel hardly scrolls the legend. With commit c96581c I've fixed both issues. This commit requires updating the wheel test.
• I've also noticed that this PR makes the margins of one of my mocks larger. I need to investigate this further.
• Another thing to look at is how the space for the scrollbars is allocated (is it hard-coded here?). But I want to do this last, just in case it affects any of the baseline images.

[n-riesco]

Commit 8baac47 adds a nice tweak to legends. Previously, the margins were expanded using the legend size, no matter whether the legend would fit or not. But since legends that don't fit are forced to fit into the plot height, there is no need to expand the vertical margin and make the plot box unnecessarily smaller.

Before this PR (i.e. using dist/plotly.js):

After (using build/plotly.js):

[n-riesco]

@mdtusz @etpinard I had to revert my last commit because of merge conflicts. I'd like to merge this PR before trying to rebase my last commit. Is that OK?

[mdtusz]

Looks great! Let's either remove these entirely or uncomment them (it seems like they may not be necessary?), then 💃 and we can merge it in!

[n-riesco]

@mdtusz Done.

I have a commit that splits Legend.draw() into smaller functions. It conflicts with master. Shall I try to rebase it?

While playing with the scrollbar I noticed that the top and bottom traces in the legend may overlap with the legend border. I've thought of two possible solutions:

1. move the clippath from legend to scrollbox (this implies updating the clippath position along with that of the scrollbox on drag and wheel events),
2. apply the clippath to a new <g> and make the scrollbox a child of this new <g>.

Option 2 involves less coding (but adds one more layer). Which option do you prefer?

[mdtusz]

I'd prefer to keep the layers as minimal as we can - can we just alter the clippath to be "padded" - i.e. reduce it's height by x pixels and translate down by x/2 pixels?

Or perhaps I'm misunderstanding the issue.

[n-riesco]

This is a screenshot showing the problem:

The clippath is applied to legend and legend contains both rect.bg and scrollbox. If we shrink the clippath, then both rect.bg and scrollbox will be affected.

[mdtusz]

Ah ok got it. I'd prefer if we went with option 1, but I'll leave it to your judgement.

[mdtusz]

Thanks Nico. This looks good to go.

💃