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

Preserve double border line when using trackLabel offset and use smaller gap between snpcoverage and reads #2678

Merged
merged 8 commits into from
Mar 2, 2022

Conversation

cmdcolin
Copy link
Collaborator

This removes the border below the track label when using track labels: offset (the default is overlapping). It is a subtle change but i think it makes the offset mode more clear

It changes to use a div instead of a paper
this PR
Screenshot from 2022-01-27 07-03-05

main
Screenshot from 2022-01-27 07-03-18

Also removes a double line near the scalebar in the overlapping(default) mode

this PR
Screenshot from 2022-01-27 07-09-41

main
Screenshot from 2022-01-27 07-10-09

@github-actions github-actions bot added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Jan 27, 2022
@cmdcolin cmdcolin changed the title Use plain div with border-bottom instead of paper Avoid drawing a errant border line when using trackLabel offset Jan 27, 2022
@codecov
Copy link

codecov bot commented Jan 27, 2022

Codecov Report

Merging #2678 (f699255) into main (fd12bf9) will decrease coverage by 0.04%.
The diff coverage is 47.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2678      +/-   ##
==========================================
- Coverage   60.21%   60.17%   -0.05%     
==========================================
  Files         572      572              
  Lines       26611    26609       -2     
  Branches     6486     6484       -2     
==========================================
- Hits        16024    16011      -13     
- Misses      10262    10273      +11     
  Partials      325      325              
Impacted Files Coverage Δ
...ments/src/LinearAlignmentsDisplay/models/model.tsx 68.88% <ø> (ø)
...AlignmentsDisplay/components/AlignmentsDisplay.tsx 56.25% <46.15%> (+12.50%) ⬆️
...src/LinearGenomeView/components/TrackContainer.tsx 80.55% <50.00%> (-1.03%) ⬇️
...s/svg/src/SvgFeatureRenderer/components/Chevron.js 87.17% <0.00%> (-10.26%) ⬇️
packages/core/TextSearch/TextSearchManager.ts 96.55% <0.00%> (-3.45%) ⬇️
...inearGenomeView/components/RefNameAutocomplete.tsx 77.77% <0.00%> (-3.04%) ⬇️
packages/core/util/analytics.ts 89.79% <0.00%> (-2.05%) ⬇️
...gins/svg/src/SvgFeatureRenderer/components/util.ts 92.72% <0.00%> (-1.82%) ⬇️
...gFeatureRenderer/components/ProcessedTranscript.js 85.13% <0.00%> (-1.36%) ⬇️
... and 2 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 fd12bf9...f699255. Read the comment docs.

@cmdcolin
Copy link
Collaborator Author

@garrettjstevens is this ok? any reason to prefer the Paper component?

@cmdcolin cmdcolin added bug Something isn't working and removed needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) labels Jan 27, 2022
@garrettjstevens
Copy link
Collaborator

I prefer the look of using the Paper component. I think the top line gives a good visual cue of which track label mode you are using.

If we do change it, though, I think the color should be theme.palette.divider instead of lightgray so that it matches the theme, especially if we start using dark mode more.

@cmdcolin
Copy link
Collaborator Author

I guess my preference would be for minimizing the lines in the interface. The border lines look less like a border and more like a weird double line in the overlapping mode and in the offset mode, it kind of looks like an empty track with no features with a labelless track below it

@cmdcolin
Copy link
Collaborator Author

good catch on the theme.palette.divider, i changed it to that

@cmdcolin
Copy link
Collaborator Author

any updates on this one?

@cmdcolin
Copy link
Collaborator Author

Instead of using track borders at all now, the track border is removed, and instead, the resize handle between tracks is colored with theme.palette.divider (grey)

before
Screenshot from 2022-02-22 12-17-00

after
Screenshot from 2022-02-22 12-17-11

I considered also making the resize handle between pileup and snpcov in the alignments track visible with grey but did not, i did make the distance smaller though

@garrettjstevens
Copy link
Collaborator

I'm still seeing a border-bottom when testing this out. Also, I find the resize handle a lot harder to click with it being reduced to 2 pixels from 3.

@cmdcolin
Copy link
Collaborator Author

woops, I had an unpushed commit about the border. also debated the 2 vs 3 pixel height, pushed 3 here

@cmdcolin cmdcolin force-pushed the track_container_border branch 2 times, most recently from 587c467 to 20ff611 Compare February 28, 2022 19:31
@cmdcolin
Copy link
Collaborator Author

cmdcolin commented Mar 1, 2022

I restored the border after looking at the comparison for a bit. The double border line creates a bit of a visual queue that is helpful, and it can be little hard to distinguish features e.g. introns from the 3px resize handle if it is solid

This PR still contains some miscellaneous changes.

Including

  • The absolute positioning is removed and border-box because I could not see any effect.
  • The TrackLabel is put inside the Paper, so that the outline is not below the track label
  • Smaller area between the coverage and pileup on alignments tracks

@cmdcolin cmdcolin merged commit 26e5eee into main Mar 2, 2022
@cmdcolin cmdcolin changed the title Avoid drawing a errant border line when using trackLabel offset Preserve double border line when using trackLabel offset and smaller gap between snpcoverage and reads Mar 2, 2022
@cmdcolin cmdcolin deleted the track_container_border branch March 2, 2022 18:32
@cmdcolin cmdcolin changed the title Preserve double border line when using trackLabel offset and smaller gap between snpcoverage and reads Preserve double border line when using trackLabel offset and use smaller gap between snpcoverage and reads Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants