statgen / locuszoom

A Javascript/d3 embeddable plugin for interactively visualizing statistical genetic data from customizable sources.
https://statgen.github.io/locuszoom/
MIT License
156 stars 29 forks source link

Make instance resizing automatic, manual, or disabled #41

Closed Frencil closed 8 years ago

Frencil commented 8 years ago

Presently by default all instances are manually resizable (with the resize_handle UI element).

At a minimum this should be a parameter that can be disabled, forcing an instance to keep a strictly defined size. Ideally a layout could also set this value to be something like "automatic" such that the instance always takes up 100% of its containing div.

Frencil commented 8 years ago

Thinking about this, the "automatic" behavior can definitely get tricky. For example, LocusZoom's existing resize logic allows for any arbitrary aspect ratio, so you could have a LocusZoom plot that's many thousands of pixels tall. As a page / containing dev resizes, if a LocusZoom plot is set to automatically stay at 100%, how should it handle changing its vertical size? Just maintain the present aspect ratio, perhaps?

@MrFlick / @benralexander thoughts?

benralexander commented 8 years ago

I agree that maintaining the aspect ratio would be a good solution.

On Tue, Mar 22, 2016 at 3:52 PM, Christopher Clark <notifications@github.com

wrote:

Thinking about this, the "automatic" behavior can definitely get tricky. For example, LocusZoom's existing resize logic allows for any arbitrary aspect ratio, so you could have a LocusZoom plot that's many thousands of pixels tall. As a page / containing dev resizes, if a LocusZoom plot is set to automatically stay at 100%, how should it handle changing its vertical size? Just maintain the present aspect ratio, perhaps?

@MrFlick https://github.com/MrFlick / @benralexander https://github.com/benralexander thoughts?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/statgen/locuszoom/issues/41#issuecomment-199988052

MrFlick commented 8 years ago

It seems like page resizing should just effect the width of the plot. Changing the height makes sense as a drag. Right now we have drag in the order to change both width and height. Maybe just allow it to change the height. Since we're not really making plots where the distance on the x and y axis are directly relatable, i'm not sure it makes sense to try to preserve aspect ratios. That's more important and x and y values are on the same scale.

benralexander commented 8 years ago

To be fully integrated with the rest of the portal there probably shouldn't be an independent drag event, and instead the graphic should drive its width from the page size. At small screen sizes I could imagine that a presentation with a preserved aspect ratio would be more readable than one that is tall but skinny. That said, I could also imagine someone intentionally making the screen very wide in order to better examine genomic position, and not necessarily wanting the graphic to become taller. So in the end I'm not sure in the end whether preserving the aspect ratio will be better or worse, but I imagine it's not a big coding switch either way (i.e. let's see how it feels once we get it in the portal). I do think, however, that we want to make the LZ's width (at least) dependent on browser page size, and not independently manipulable via a corner drag.

On Wed, Mar 23, 2016 at 4:26 PM, MrFlick notifications@github.com wrote:

It seems like page resizing should just effect the width of the plot. Changing the height makes sense as a drag. Right now we have drag in the order to change both width and height. Maybe just allow it to change the height. Since we're not really making plots where the distance on the x and y axis are directly relatable, i'm not sure it makes sense to try to preserve aspect ratios. That's more important and x and y values are on the same scale.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/statgen/locuszoom/issues/41#issuecomment-200530353

Frencil commented 8 years ago

Thanks for the discussion on this, folks. Here's an update...

A working prototype for responsive resizing is now up in #42 (working demo here). Things to note:

It sounds like, from the discussion here, that responsive resizing with a constant aspect ratio may only address some use cases of responsive resizing. Other use cases may involve keeping the height at a predefined constant and allowing the aspect ratio to flex, whereas other cases still may involve inferring a "pretty good" aspect ratio from the aspect ratio of the containing window.

How about, since we're already trying to get a responsive resizing proof of-concept in place for the March integration, that we stick with the simple fixed-aspect-ratio approach and then expand the layout syntax to support those other use cases in a subsequent branch? @benralexander on your end that would just require selecting a good catch-all aspect ratio and defining that along with the resizable parameter in your custom layout (see the source of demo_responsive.html on that branch for an example of how this is done).

Frencil commented 8 years ago

I just merged #42 which implements the basic three modes of resizing described here. @benralexander going forward for the March goals if an aspect ratio other than 16:9 would be preferable let me know as that was an arbitrary choice and it's trivial to change that at any point. I'm going to file follow-up issues for the other resizing modes described here, and we'll approach those at a later date.