statgen / fivex

Interactive eQTL visualizations
MIT License
13 stars 3 forks source link

Change resize_to_data to only resize to the height of the genes datalayer to avoid unceasing growth #32

Closed pjvandehaar closed 3 years ago

pjvandehaar commented 4 years ago

Currently, every time you click "Show All Genes" the genes panel gets taller.

This has been the case ever since Alan's change that made the variant vertical line's height update to its panel's height on each render.

Inside Alan's overridden LocusZoom.DataLayers.add('orthogonal_line_varpos', function(layout) {, I can change his line

panel[y_range][0] = panel.layout.height;

to

panel[y_range][0] = panel.layout.height - panel.layout.margin.top - panel.layout.margin.bottom;

in which case the growth slows to only 8px per click. Perhaps that's how it should work anyways.

Changing that line to

panel[y_range][0] = panel.layout.height - panel.layout.margin.top - panel.layout.margin.bottom - 8;

stops the growth. I don't know where those 8px are coming from.

Should the resize_to_data dashboard component allow a layout parameter watch_only_datalayer_id: "genes"?

abought commented 4 years ago

This is an interesting fix, and I'll try to review it a bit today to solve the mystery of those extra pixels.

It seems the root cause is the vertical line sizing issue: I'd like to dig into the original issue in LZ first, in case there is a way to fix that at the source instead of layering on multiple workarounds. I've slated it as a goal for the 0.10 milestone, which I'd hope to release officially in early december.

Here is the relevant tracking issue: https://github.com/statgen/locuszoom/issues/173

abought commented 4 years ago

I've added a partial solution to LZ.js core, which stops the height growth (by using layout.cliparea.height instead of panel.layout.height to determine line size). This allows the orthogonal line to respond to changes when the panel is resized, and not be tied only to the initial dimensions on first render.

The main weakness is that it doesn't auto-shrink the panel if there are few genes (because the size gets determined by the total size of items, and the line is the biggest thing). Thoughts?

https://github.com/statgen/locuszoom/commit/81f2e989edc5aeebe068f1532056c22d4f2c8a23#diff-5f7c570134e9f3aace4f2806098e3601R391

amkwong commented 4 years ago

If I could choose any solution, I would create a new variable that stores the initial rendering dimensions, and either add a new button or have the "Show All Genes" button change (after being clicked) to "Reset View", which resets the dimensions to the initial values (and apply the new state to the plot to update the view).

amkwong commented 4 years ago

Side issue: the current version works correctly in Chrome (also Safari and Microsoft Edge), but is still buggy in Firefox, i.e. hitting the "Show all genes" button in Firefox still leads to unceasing growth. This is more arcane and esoteric than I ever imagined, and beyond my abilities to debug...