Closed Frencil closed 8 years ago
UPDATE: This branch is now development complete.
This is a large refactor and is by no means finished, but in its current state it is stable and functional. As discussed today among LocusZoom devs we're better off freezing further feature development and refactoring on this branch in favor of review and merging to prevent the scope from growing much further.
So it doesn't look like there's actually a "serialize" method yet. Is the idea just to keep the "layout" property/object always current and have no state outside that object?
Lots of great stuff here. This will be immensely useful.
Re: the serialize method: see this comment for some thoughts about that.
In general I see power in keeping the layout object on the Plot/Instance as a serializable object that's kept current as the layout is changed (good example of this in action right now would be the UI to resize an instance which changes top-level layout width/height directly and redraws the plot from the values in the layout). But we need generic methods for merging partial layouts with a fully-defined layout (that fully defined layout could be the default layout, in the case of initialization, or whatever the current layout is, in the case of on-the-fly layout editing that end users could hook up to their own custom inputs).
In general the layout on the plot/instance should always be an accurate description of what that plot/instance is. There's an important distinction between that and the state
object, which describes the current query parameters for the data being displayed. The only other piece needed to essentially "clone" a LocusZoom are the data sources which presently are still built up as a separate object. This pattern is expressed explicitly in code as LocusZoom.populate
and LocusZoom.populateAll
take a selector for a target but then apply datasource
, layout
, and state
.
Because these three are different, but similar, and equally important to build a precise LocusZoom visualization we can probably expect each of them to undergo a fair bit of iteration going forward.
UPDATE: Most of our discussion today has been about directions to take layout patterns, API, and workflow beyond the scope of this branch. I've added a few issues to begin to document these needs for addressing on future branches (see #31, #32, and #33).
Tracking these discussions as other issues makes total sense. Unless you want to make any other changes first, i'd be happy to merge the PR now.
Yeah, I'd say go ahead and merge if you're comfortable. I'll continue to document more issues from our discussion and try to keep future refactors smaller in scope. Thanks!
The Problem
Now starting to look at how end users would integrate the LocusZoom plugin into projects it became apparent that the syntax for defining new "layouts" was unintuitive. Layouts were to be built as new Instance classes using methods and, while possible, it was cumbersome and did not lend itself to clarity or easy documentation.
Layouts as Objects
This branch introduces the concept of a
layout
object, a serializable object that can be built by any means and provided when an instance is created. This approach allows for easily breaking out attributes to cover any aspect of a layout we wish to be configurable, offering far greater flexibility.Instance
andPanel
classesWith this change there is no longer a need to have subclasses of Instances or Panels. Each base class is still vital to compartmentalize aspects of a rendered LocusZoom object, and now lend themselves even better to broad generalizations (e.g. "An Instance represents a single LocusZoom graph and all of its elements" or "A Panel controls the axes on a graph").
DefaultInstance
,PositionsPanel
, andGenesPanel
no longer exist.DefaultLayout
Where before
DefaultInstance
created the default layout by extending the base Instance class, nowDefaultLayout
does the same by defining a layout as an object:Recursive layout pattern
The classes
Instance
,Panel
, and the baseDataLayer
class all keep alayout
property that is their layout. Where before some of the above classes had aview
property for layout-related data, such as dimensions or dimension bounds, that's all now stored inlayout
. Any methods that change any of those values on the screen should now be using properties in their respective layout objects to track those values under the hood.Label functions
The
Panel
class now has a singleton calledLabelFunctions
for storing universal functions for axis labels that change based on state. This also has a robust suite of unit tests built out and ready to go.TODO
Lots!
Figure out what to do about Instance tests that are now failing without data sources (why did they work before?)Expand tests to include more custom-built layouts instead of just the default layoutUpdate plugin usage documentation to reflect new syntax and supported options (big!)