-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix positioning of scalebar tooltips, overview scalebar plotting, and refName label positioning when displaying many regions #1587
Conversation
To test, here is a share URL with lots of displayed regions http://localhost:3000/?config=test_data%2Fconfig_demo.json&session=share-BZ3qCPhTw6&password=lq7S7 |
…Bp since bpSoFar includes 'padding bp'
a7067b8
to
a4daa17
Compare
Codecov Report
@@ Coverage Diff @@
## master #1587 +/- ##
==========================================
- Coverage 59.40% 59.31% -0.10%
==========================================
Files 434 434
Lines 19612 19613 +1
Branches 4591 4595 +4
==========================================
- Hits 11651 11633 -18
- Misses 7667 7686 +19
Partials 294 294
Continue to review full report at Codecov.
|
…ront of inter-region-padding-block
f0723cc
to
4ccf16d
Compare
This one is maybe at a good place now. There is a fair amount of logic to make sure to carefully account for inter region padding blocks,etc. it's tricky but must be taken into account when zoomed out. I made some tests to check some of the expectations of this inter region padding block being taken into account when both zoomed out far and zoomed in close in various regions of hg38 which includes ellided blocks in the middle |
Standardize on the beforeFirst and afterLast interregionpaddingblocks |
…locks may not refName)
following discussion in meeting, I restored the "boundary" blocks to calculateDynamicBlocks (matching calculateStaticBlocks) and then make a special calculation to avoid them in dd76c3e |
dd76c3e
to
4ccf16d
Compare
I think this is probably good to go.. if anyone is interested in reviewing feel free but might merge soon |
With this change
#1556
#1532
Also see refname label positioning issue (X not flush with boundary) in #1556
I also make the minimum block width smaller to make it so realistically human sized chromosomes such as 20-22 of human are displayed even when zoomed out
The refname label positioning was fixed by updates to calculateStaticBlocks/calculateDynamicBlocks: only needs to add interregionpaddingwidth when it is a content block
This PR also adds ellided blocks to the overview bar so that we avoid rainbow-of-small-displayed regions, and changes it from paper with margins to exact width divs
Note that this is still not perfect: it is visible from the screenshot that the chr1 overview scale bar polygon does not match up perfectly with the end of the display and the chr1 displays some negative offsets in an actually visible region
However, these changes do improve the general situation