WIP: Legend Scrolling

[mdtusz]

There's still a bug or two to work out, but it's in a working state.

When you toggle a trace for the first time, the width changes and hides the scrollbar until you scroll (for some reason the text-width is being changed when you toggle for the first time).

Otherwise, it will automatically enable the scroll if the height of the list of traces is greater than the height of the plot. I need to add a check whether the scrollbar will be inserted and rework the details on what sets the bounds, but the main logic is there.

[mdtusz]

This section should likely be moved to the draw step, but everything is highly coupled right now and the order of drawing/sizing/positioning is very scattered.

[etpinard]

@mdtusz Any reason for putting this section in repostionLegend in the first place?

[mdtusz]

Mainly for ease of development. The scroll handler requires the final height of the legend to calculate the viewbox offset and at the time, I was still trying to wrap my head around the control flow in here.

It will be moved out into draw before we merge though

[etpinard]

thanks for the info. 👍

[etpinard]

@mdtusz This PR is looking great.

To add some background info (cc @alexcjohnson @cldougl):

• Adding legend scroll brings in another nested <svg>, though it can be easily extracted in the to-image step if we like.
• The viewbox height will be computed via the plot's height and vertical margin. Down the road, we'll make that height configurable via an attribute, but not in this PR.
• Features like the scroll bar dimensions and colour could also be associated to attributes in future development.

[cldougl]

• The viewbox height will be computed via the plot's height and vertical margin. Down the road, we'll make that height configurable via an attribute, but not in this PR.

Pretty nice that this is dynamically computed now even cooler it will be configurable in the future 🎆

[alexcjohnson]

Can we get away from adding another nested svg, and do it with <g clip-path=...> instead? @jackparmer forwarded me another complaint about illustrator compatibility just last night. I started down that road with shapes (clipping to axes) and contours with missing data (which wouldn't have been possible at all otherwise as it's non-rectangular clipping). See drawing.setClipUrl. It probably wouldn't be very hard to axe ALL nested svgs right now, but we should at least not be adding new ones.

[mdtusz]

I'm not sure the best way how we could easily do this with a clip path - the reason it works now is because of the shifting viewBox. As far as I understand, with a clip-path, we'd need to have the clip-path coordinates shift, as well as an opposing shift of the element containing the legend items. I.e. legend items moves up while clip-path on it moves down to make the apparent legend box look stationary.

This could/would work, but I imagine the scrolling would appear janky because of the moving outer element - having the movements be perfectly in sync would likely be hard to achieve.

[alexcjohnson]

If you make a <g> with clipping and put another <g> inside it with a shifting transform, the clip path should not need to change. This is basically what happens to shapes when you drag an axis - though there I probably change its path rather than its transform, but the idea is the same.

[alexcjohnson]

having the movements be perfectly in sync would likely be hard to achieve.

Not an issue here because you can do it with a single transform... but as far as I've seen, browsers will not repaint during synchronous js execution. This is in fact the primary motivation behind syncOrAsync, so we can chain things that may or may not contain async parts without making every link in the chain async. That way when you are sync there are no intermediate flashes.

[mdtusz]

Yep, I played around and have gotten it to work using a <g> instead of the scrollBox <svg>, but if we want to change the pre-existing legendsvg to a <g> as well, we will need to rewrite a fair amount of positioning code - <g> elements don't have width, height, x, or y attributes to them unfortunately.

[alexcjohnson]

I mainly wanted to avoid making the nested svg problem worse... so I won't complain if you leave it at that. But I suspect it wouldn't be that hard: x and y --> transform=translate(x,y), and width and height --> clipPath width and height. Granted a lot of ugly math could probably be cleaned up with a more substantial rewrite...

Up to @jackparmer how much he wants Illustrator compatibility, people have been complaining about it forever but at a low level.

[etpinard]

This felt a little a crude on my laptop.

The mouse wheel scaling is very smooth though 👍

[mdtusz]

For archival reasons, I'll mention that this PR will fix #251.

[etpinard]

💃

[mdtusz]

FINALLY!

[jackparmer]

BOOM! 💥

On Mon, Feb 22, 2016 at 4:34 PM, Miklos Tusz notifications@github.com
wrote:

Merged #243 #243.

—
Reply to this email directly or view it on GitHub
#243 (comment).

[arikfr]

Best thing since open sourcing plotly.js :-) 🎉
Thanks!