Skip to content
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

Make LinearAlignmentsDisplay fully configurable in the UI #1795

Merged
merged 4 commits into from
Apr 1, 2021

Conversation

garrettjstevens
Copy link
Collaborator

@garrettjstevens garrettjstevens commented Mar 10, 2021

This makes the pileup display config be based off of the linear basic display instead of the base linear display, restoring mouseover that went missing after #1666.

Editing the mouseover callback for an AlignmentsDisplay in the UI doesn't work, though, since you can only edit the top-level config and not the nested PileupDisplay and SNPCoverageDisplay configs. Maybe we need to expose those in the UI somehow? No longer applies, see next comment.

@garrettjstevens garrettjstevens self-assigned this Mar 10, 2021
@github-actions github-actions bot added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Mar 10, 2021
@codecov
Copy link

codecov bot commented Mar 12, 2021

Codecov Report

Merging #1795 (0532e77) into main (17e0ed1) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1795   +/-   ##
=======================================
  Coverage   59.19%   59.20%           
=======================================
  Files         459      459           
  Lines       21351    21349    -2     
  Branches     5008     5008           
=======================================
  Hits        12639    12639           
+ Misses       8405     8403    -2     
  Partials      307      307           
Impacted Files Coverage Δ
...alignments/src/LinearPileupDisplay/configSchema.ts 100.00% <ø> (ø)
...src/LinearAlignmentsDisplay/models/configSchema.ts 100.00% <100.00%> (ø)
...nments/src/LinearAlignmentsDisplay/models/model.ts 67.14% <100.00%> (-0.47%) ⬇️
packages/core/util/index.ts 80.99% <0.00%> (ø)
packages/core/configuration/configurationSchema.ts 78.43% <0.00%> (+0.98%) ⬆️
...BaseLinearDisplay/components/BaseLinearDisplay.tsx 87.23% <0.00%> (+2.12%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 17e0ed1...0532e77. Read the comment docs.

@garrettjstevens
Copy link
Collaborator Author

This now contains a full LinearPileupDisplay config and a full LinearSNPCoverageDisplay in the LinearAlignmentsDisplay config, and gets rid of the handful of top-level configs in LinearAlignmentsDisplay config that were mostly duplicates of entries in the LinearSNPCoverageDisplay config. Now all config settings are available in the config editor UI.

Base automatically changed from master to main March 23, 2021 17:21
@garrettjstevens garrettjstevens marked this pull request as ready for review March 30, 2021 18:10
@cmdcolin
Copy link
Collaborator

Random thoughts:

One thing we could also consider is using "less" mouseover tooltips. They can sometimes (maybe even often) get in the way when you are trying to gesture with the mouse, etc.

Another thing to consider is allowing per-base information to be shown by the tooltip. This is done in igv, and can be useful

@garrettjstevens garrettjstevens changed the title Restore Pileup mouseover Make LinearAlignmentsDisplay fully configurable in the UI Mar 31, 2021
@garrettjstevens
Copy link
Collaborator Author

Changed the title of this PR since it now is really more about having a fully UI-configurable LinearAlignmentsDisplay than about mouseovers.

@rbuels rbuels added internal and removed needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) labels Apr 1, 2021
@rbuels rbuels merged commit c125341 into main Apr 1, 2021
@rbuels rbuels deleted the restore_pileup_mouseover branch April 1, 2021 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants