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

Moving labels canvasfeatures #971

Merged
merged 15 commits into from Mar 24, 2018

Conversation

Projects
None yet
2 participants
@cmdcolin
Contributor

cmdcolin commented Feb 2, 2018

This PR adds support for moving the canvasfeatures labels while scrolling, referencing issue #390

One caveat is that there is a small moment where the "renderLabel" rendering and the "updateStaticElements" from this PR are both visible at the same time. Not sure if there is a good fix (maybe clearRect over region?)

Screenshot
screenshot-localhost-2018-02-01-23-39-52-502

@wafflebot wafflebot bot added the in progress label Feb 2, 2018

@nathandunn nathandunn changed the base branch from master to dev Feb 7, 2018

@rbuels rbuels added this to the 1.13.0 milestone Feb 27, 2018

@rbuels

This comment has been minimized.

Collaborator

rbuels commented Feb 27, 2018

a.) could you add a release-notes blurb about this?

b.) aren't the strand arrows just drawn as updatable static elements? probably the labels should be the same, don't you think? can we just disable the renderLabel rendering of the labels?

@cmdcolin

This comment has been minimized.

Contributor

cmdcolin commented Feb 28, 2018

I added a small change to help move towards option making it only an updateStaticFeatures thing. It works ok with single box features but with gene glyph the "bounds" of the fRect aren't expanded so the text overlaps feature on initial render (and, as noted above, the transcript labels overlap the transcripts during scrolling also, some question of whether this is desirable)

Screenshot, you can see that even when the label is not being "kept visible" the transcript labels overlap the EDEN feature. Normally the transcript labels are moved to the left of the feature.
screenshot-localhost-2018 02 27-21-10-08

@rbuels

This comment has been minimized.

Collaborator

rbuels commented Mar 8, 2018

does the gene glyph need to be fixed to make that work then?

@rbuels rbuels modified the milestones: 1.13.0, 1.13.1 Mar 14, 2018

@rbuels rbuels merged commit 9b75846 into dev Mar 24, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@cmdcolin

This comment has been minimized.

Contributor

cmdcolin commented Mar 27, 2018

this is a little weird but it seems like maybe a little tinier clearrect boundary might help, otherwise surrounding feature labels seem to get clipped

diff --git a/src/JBrowse/View/FeatureGlyph/Box.js b/src/JBrowse/View/FeatureGlyph/Box.js
index 3c1682e..4ca2021 100644
--- a/src/JBrowse/View/FeatureGlyph/Box.js
+++ b/src/JBrowse/View/FeatureGlyph/Box.js
@@ -313,7 +313,7 @@ return declare([ FeatureGlyph, FeatureLabelMixin], {
                 context.font = fLabelRecord.font
                 context.fillStyle = fLabelRecord.fill
                 context.textBaseline = fLabelRecord.baseline
-                context.clearRect(labelLeft,labelTop,fLabelRecord.w,fLabelRecord.h)
+                context.clearRect(labelLeft,labelTop,fLabelRecord.w,fLabelRecord.h*0.7)
                 context.fillText(
                     fLabelRecord.text,
                     labelLeft,

@cmdcolin

This comment has been minimized.

Contributor

cmdcolin commented Mar 27, 2018

also the feature labels kind of go a little crazy when you click zoom out

@rbuels

This comment has been minimized.

Collaborator

rbuels commented Mar 27, 2018

@cmdcolin thanks for pointing those out, I fixed those two issues in dev

@cmdcolin

This comment has been minimized.

Contributor

cmdcolin commented Mar 27, 2018

Awesome:))!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment