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

New SVG gene glyph with directional arrows #2775

Merged
merged 19 commits into from
Mar 18, 2022
Merged

New SVG gene glyph with directional arrows #2775

merged 19 commits into from
Mar 18, 2022

Conversation

cmdcolin
Copy link
Collaborator

This removes the chevron code for directional indications (xref #1909)

The arrows are added on top level features (e.g. the subfeature Box glyphs do not draw the arrowheads) and it determines this using a topLevel prop

Fun technical detail but this also uses a line+polygon elements instead of a single polyline to draw the polygon. Also note there is code drawing arrows like this on main, but it is hidden behind the chevron.

Reason for polygon+line is because it is sharper this way, see screenshots

polygon triangle
Screenshot from 2022-02-27 19-07-00

polyline triangle, has a little "foot" on the arrow when the polyline presumably doesn't come to a perfect point
Screenshot from 2022-02-27 19-07-14

also converted the svg features to typescript in the process. can separate into multiple PR if helpful

also uses an outline config variable for boxes, instead of only displaying the outline when selected (current main does this) and not use color2 but instead a new config variable that is nothing by default (color2 is the intron color) (xref #2449)

@github-actions github-actions bot added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Feb 28, 2022
@codecov
Copy link

codecov bot commented Feb 28, 2022

Codecov Report

Merging #2775 (410ab7c) into main (9bbe58a) will decrease coverage by 0.00%.
The diff coverage is 80.86%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2775      +/-   ##
==========================================
- Coverage   59.97%   59.97%   -0.01%     
==========================================
  Files         584      584              
  Lines       26651    26576      -75     
  Branches     6447     6444       -3     
==========================================
- Hits        15985    15938      -47     
+ Misses      10338    10324      -14     
+ Partials      328      314      -14     
Impacted Files Coverage Δ
packages/core/util/simpleFeature.ts 84.90% <ø> (ø)
packages/core/util/index.ts 85.18% <50.00%> (-0.50%) ⬇️
...FeatureRenderer/components/SvgFeatureRendering.tsx 71.87% <71.87%> (ø)
...gins/svg/src/SvgFeatureRenderer/components/util.ts 91.83% <76.47%> (-0.90%) ⬇️
.../src/SvgFeatureRenderer/components/Subfeatures.tsx 85.71% <83.33%> (ø)
...svg/src/SvgFeatureRenderer/components/Segments.tsx 85.71% <85.71%> (ø)
...BaseLinearDisplay/components/BaseLinearDisplay.tsx 82.60% <87.50%> (-0.98%) ⬇️
...FeatureRenderer/components/ProcessedTranscript.tsx 87.32% <90.90%> (ø)
...src/SvgFeatureRenderer/components/FeatureGlyph.tsx 94.11% <94.11%> (ø)
packages/core/configuration/util.ts 91.30% <100.00%> (-0.19%) ⬇️
... and 6 more

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 9bbe58a...410ab7c. Read the comment docs.

@cmdcolin cmdcolin added enhancement New feature or request and removed needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) labels Feb 28, 2022
@cmdcolin cmdcolin marked this pull request as draft March 9, 2022 19:21
@cmdcolin cmdcolin marked this pull request as ready for review March 10, 2022 00:26
@cmdcolin cmdcolin force-pushed the new_svg_gene_gylph branch 2 times, most recently from 2bf2d40 to ba08013 Compare March 10, 2022 21:45
@cmdcolin
Copy link
Collaborator Author

I think the agreement from the meeting was this PR should be ok to go, at least conceptually. any concerns?

one note was that the Chevron glyph is being removed, and we wondered if any third party libs directly used this glyph cc @bbimber

@bbimber
Copy link
Contributor

bbimber commented Mar 11, 2022

@hextraza Sebastian, could you give this a quick look? I dont think we need chevron; however, we might need to do some tweaks on our plugin to adapt to this.

@cmdcolin
Copy link
Collaborator Author

I don't believe chevron is re-exported so should not cause breakage in third party code

@cmdcolin
Copy link
Collaborator Author

maybe can merge optimistically :)

@cmdcolin cmdcolin merged commit 1d6e373 into main Mar 18, 2022
@cmdcolin cmdcolin deleted the new_svg_gene_gylph branch March 18, 2022 17:24
cmdcolin added a commit that referenced this pull request Mar 18, 2022
…chevrons on subfeatures, and convert SVG features to typescript (#2775)

* New style of arrowhead on gene glyph

* Updates

* Misc

* T1

* Fix some tsc

* Fix tsc

* Misc

* Add Arrow component

* Add arrow

* Fix tests

* Less use of any

* More typescript

* Updates

* Add outline variable

* Fix flip on arrow

* Minor refactor to avoid ClientRect

* Update snaps

* No arrow if no strand

* Arrow before box so box covers arrow
@cmdcolin cmdcolin restored the new_svg_gene_gylph branch March 24, 2022 15:57
@cmdcolin cmdcolin deleted the new_svg_gene_gylph branch March 24, 2022 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants