jbrowse-components
jbrowse-components copied to clipboard
Adds option to auto-adjust the track height to show all features
Adds new menu option to auto-adjust the track height to expand and shrink based on the features shown.
The functionality is as follows:
‘static’ === the track height will assume the height the user has adjusted the height to, or the configured height
‘dynamic’ === the track height will expand and shrink based on the height of the features shown
Resolves https://github.com/GMOD/jbrowse-components/issues/534
Adds a slot for this in the config:
/**
* #slot configuration.LinearGenomeViewPlugin.adjustTrackLayoutHeight
*/
adjustTrackLayoutHeight: {
type: 'string',
defaultValue: 'static',
model: types.enumeration('adjustTrackLayoutHeightOptions', [
'static',
'dynamic'
]),
},
@cmdcolin couple of questions:
- Where do you think it makes most sense to keep the setting for this? In the commit above it's in the display menu options (because I thought it made sense next to 'set max height'?), but an argument could be made for it in the view settings (because it's more 'universal') and we also discussed in the JBrowse settings. Some visuals:
Option 1: JBrowse settings
Option 2: View settings
Options 3: Display settings
-
Can you define what you mean/want out of 'on first render'?
-
Can you comment on the implementation of the setting -- I mirrored the setting for track labels, but then noticed you had some scaffolding on this branch for using a disposer on the web session model...track labels operates similarly (semi-universal setting on the tracks), but its placement is kind of odd like that (a track-level setting, in the view menu, that applies to the whole app until changed)
Codecov Report
Attention: Patch coverage is 59.64912% with 23 lines in your changes missing coverage. Please review.
Project coverage is 62.62%. Comparing base (
a097560) to head (49214ed). Report is 115 commits behind head on main.
:exclamation: Current head 49214ed differs from pull request most recent head 6ff2f10
Please upload reports for the commit 6ff2f10 to get more accurate results.
Additional details and impacted files
@@ Coverage Diff @@
## main #4252 +/- ##
==========================================
+ Coverage 62.61% 62.62% +0.01%
==========================================
Files 1088 1086 -2
Lines 31495 31442 -53
Branches 7531 7507 -24
==========================================
- Hits 19721 19692 -29
+ Misses 11602 11578 -24
Partials 172 172
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
- I dont quite know the best but the "View menu" might be a happy medium between per-track and global. Can make it a localstorage setting so once you set it on one view, it is "sticky" to other future instances of other views and sessions.
- given that it is a block based thing, perhaps it would mean "max height after all blocks in view are rendered". hard to maybe do that codewise so it could be "first block" but ideal i think is taking max layout height of all visible blocks after the first render of all blocks.
- the session model code that was there was an errant commit that was for unrelated thing, i didn't mean to include it in my scaffolding (can remove it)
can you post a couple session links that demonstrate the usage of this feature?
@cmdcolin
- Session with two views and tracks open,
auto-adjust to show all features on the trackis enabled on both views, and you can observe the track height is different for the two views, adjusted to show all features on the track: https://s3.amazonaws.com/jbrowse.org/code/jb2/layout_max_height_calc/index.html?session=share-Pd1tUMeUnd&password=Vp8DR - Session with two views and tracks open,
auto-adjust to show height of initial featuresis enabled on the top view, andauto-adjust to show all features on the trackis enabled on the bottom view. Scroll to the left and you'll notice the track height does not adjust to show all features like the bottom view shows. https://s3.amazonaws.com/jbrowse.org/code/jb2/layout_max_height_calc/index.html?session=share-mxG8gyfp70&password=oxCfG
This is great to see, it will be a nice option to have. Some things I noticed while trying out this branch:
- This doesn't seem to work with alignments displays
- "auto-adjust to show all features on the track" will expand the track to show more features, but not shrink the track when there are fewer features
- Wasn't sure if this was intended behavior or not
- If we make it shrink as well as expand, we can disable the track resizing drag handle when in this mode
- Using the track resize drag handle to resize the track causes unexpected behavior
- If you use the drag handle resize the track with "auto-adjust to show all features on the track" on, the auto-adjusting stops working
- If you use the drag handle to resize the track in any mode, switching between track height modes no longer does anything, you have to turn the track off and on again to reset it
- Even with auto resizing, there is always a scroll bar on the SVG tracks unless you manually expand the track. I think this is due to the padding that SVG tracks use to ensure there is somewhere to click so you can de-select a feature: https://github.com/GMOD/jbrowse-components/blob/8355237c871d4d87a082c2f24f14ac01cefe70ff/plugins/svg/src/SvgFeatureRenderer/components/SvgFeatureRendering.tsx#L24-L26 It might be good to take this padding into account when sizing the track.
@garrettjstevens
This doesn't seem to work with alignments displays
Fixed, should work with alignments displays now
"auto-adjust to show all features on the track" will expand the track to show more features, but not shrink the track when there are fewer features Wasn't sure if this was intended behavior or not If we make it shrink as well as expand, we can disable the track resizing drag handle when in this mode
Going to leave as-is for now, because the layout height "only increases"
Using the track resize drag handle to resize the track causes unexpected behavior If you use the drag handle resize the track with "auto-adjust to show all features on the track" on, the auto-adjusting stops working If you use the drag handle to resize the track in any mode, switching between track height modes no longer does anything, you have to turn the track off and on again to reset it
Now, if the user resizes their track while one of the auto-adjust settings are activated, the setting will change to "off" with a notification to the user. The user can then change the setting back if they wish. I think this is a good solution because it will always do what the user is asking (that is, maintain the track height they have set it to).
Let me know what you think.
Even with auto resizing, there is always a scroll bar on the SVG tracks unless you manually expand the track. I think this is due to the padding that SVG tracks use to ensure there is somewhere to click so you can de-select a feature [...]
The +100 magic number has been added to the layout height calc so the scroll bar will no longer appear when the user is making use of these settings.
Some comments from Lincoln:
- Add an option such that the track height is expandable but bounded by a value specified by the user
- Shrinking and expanding with the user's navigation is a must
- Add a hotkey (like a double click, or a shift click) that expands to max height or retracts a track (like double clicking on a browser window expands it)
- The "first render" option might see limited use
Demo: https://s3.amazonaws.com/jbrowse.org/code/jb2/layout_max_height_calc/index.html?session=share-cTpZQPAt3Z&password=8epM2
Clear your lgv-adjustTrackLayoutHeight localStorage setting so the old terms don't mess it up.
The following changes have been made:
- Settings have been changed to be 'static', 'dynamic', and 'bound' (see the PR desc. for definitions)
- 'dynamic' now shrinks and expands with the height of the currently viewable blocks
Let me know how it feels; from a usability standpoint, does this make sense as a universal setting, like the track labels setting?
Something I am uncertain about is setting to 'dynamic', then resizing a single track will apply 'static' to all the tracks...is that what the user wants? I like the visibility in the view menu, but is this perhaps something a user would want to apply per-track?
Also wondering about the "bound" setting being based on the track height configuration...is that accessible to a user who would be interested in that setting?
nice work...random test, I tried changing to "compact" features here but the height did not adjust to the new shorter height of the track after applying compact
https://jbrowse.org/code/jb2/layout_max_height_calc/?config=%2Fgenomes%2FGRCh38%2F1000genomes%2Fconfig_1000genomes.json&session=share-aZWHI-8%2B%2Bu&password=uqy0x
i originally visited https://jbrowse.org/code/jb2/v2.10.3/?config=%2Fgenomes%2FGRCh38%2F1000genomes%2Fconfig_1000genomes.json&session=share-SUK-mntGyB&password=eQF0F
and then swapped this branch name in the url https://jbrowse.org/code/jb2/layout_max_height_calc/?config=%2Fgenomes%2FGRCh38%2F1000genomes%2Fconfig_1000genomes.json&session=share-SUK-mntGyB&password=eQF0F
also changing to "dynamic" increases the size of the snpcoverage subtrack
Possibly off-topic, but does this PR allow this track height setting to be configured in a config.json?
@nathanweeks configuring the track height in config.json is already allowed in released versions, and it should remain allowed even after this PR :)
an example of config.json with height preconfigured looks like this, it is a "display" config
{
"type": "AlignmentsTrack",
"trackId": "mytrack",
"name": "my track",
"assemblyNames": ["volvox"],
"adapter": {
"type": "CramAdapter",
"cramLocation": {"uri": "volvox-sorted-altname.cram"},
"craiLocation": {"uri": "volvox-sorted-altname.cram.crai"},
"sequenceAdapter": {
"type": "TwoBitAdapter",
"twoBitLocation": {
"uri": "volvox.2bit",
"locationType": "UriLocation"
}
}
},
"displays": [
{
"type": "LinearPileupDisplay",
"displayId": "mytrack_pileup",
"height": 400
}
]
},
Thanks @cmdcolin! I now realize the distinction between track "height" and "max height" --- the latter was what I meant.
Adapting your track height example, the following seemd to work for "max height":
{
"type": "FeatureTrack",
"trackId": "mytrack",
"name": "my track",
"adapter": {
"type": "Gff3TabixAdapter",
"gffGzLocation": {
"uri": "https://example.org/my.gff3.gz",
"locationType": "UriLocation"
},
"index": {
"location": {
"uri": "https://example.org/my.gff3.gz.tbi",
"locationType": "UriLocation"
},
"indexType": "TBI"
}
},
"category": ["Genes"],
"assemblyNames": ["myassembly"],
"displays": [
{
"displayId": "mytrack_displayId",
"renderer": {
"maxHeight": 3000
}
}
]
},
A more-relevant follow-on: will the default value for the "auto-adjust track height" feature proposed in this PR be configurable for a given track in config.json, so users wouldn't necessarily have to discover it in the menu?
Thanks @cmdcolin! I now realize the distinction between track "height" and "max height" --- the latter was what I meant.
Adapting your track height example, the following seemed to work for "max height":
{
"type": "FeatureTrack",
"trackId": "mytrack",
"name": "my track",
"adapter": {
"type": "Gff3TabixAdapter",
"gffGzLocation": {
"uri": "https://example.org/my.gff3.gz",
"locationType": "UriLocation"
},
"index": {
"location": {
"uri": "https://example.org/my.gff3.gz.tbi",
"locationType": "UriLocation"
},
"indexType": "TBI"
}
},
"category": ["Genes"],
"assemblyNames": ["myassembly"],
"displays": [
{
"displayId": "mytrack_displayId",
"renderer": {
"maxHeight": 3000
}
}
]
},
A more-relevant follow-on question: will the default value for the "auto-adjust track height" feature proposed in this PR be configurable for a given track in config.json, so users wouldn't necessarily have to discover it in the menu?
will the default value for the "auto-adjust track height" feature proposed in this PR be configurable for a given track in config.json
currently that is not part of this PR. I think this is worth discussing though.
example of where per-track setting might be relevant
a) situation like @nathanweeks refers to where admin might want it pre-configured b) situation where currently in this PR, clicking and dragging a particular tracks height changes the "dynamic" to "static" for all tracks. this seems somewhat unexpected in a way
possible way to help address this in the UI: the vertical resize handle on each track has a small "indicator" (not sure how it would look) that shows whether it is dynamic or static, and can be toggled on and off with a click.
i think also the height calculation on the blocks might still have a race condition. from quickly moving around example screenshot below
will the default value for the "auto-adjust track height" feature proposed in this PR be configurable for a given track in config.json
It is configurable on a per-session basis as part of this PR -- like track labels orientation -- I would say we should consider either making it configurable per-session, or configurable per-track. Thoughts, @cmdcolin ?
it could be worth experimenting with per track to see how it feels
Demo of per-track settings https://s3.amazonaws.com/jbrowse.org/code/jb2/layout_max_height_calc/index.html?session=share-fFEMRjtavP&password=NXDQb
close for now...will probably want to be revisited eventually