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

Add a "CSS reset" to jbrowse-react-linear-genome-view to prevent parent styles from outside the component leaking in #1763

Merged
merged 2 commits into from
Mar 4, 2021

Conversation

cmdcolin
Copy link
Collaborator

@cmdcolin cmdcolin commented Mar 3, 2021

Fixes #1678

Uses a "CSS reset" on the div containing the jbrowse-react-linear-genome-view to avoid parent styles leaking into this component

Ref
https://css-tricks.com/almanac/properties/a/all/

@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 3, 2021
@cmdcolin cmdcolin changed the title Fix text-align:center causing the LGV polygon to be badly positioned Add a "CSS reset" to the jbrowse-react-linear-genome-view, plus storybook example showing that things like parent text-align and font don't affect inside the LGV div Mar 3, 2021
@cmdcolin
Copy link
Collaborator Author

cmdcolin commented Mar 3, 2021

I updated this PR to use CSS called {all:initial} on the div containing the jbrowse-react-linear-genome-view

This avoids parent styles leaking into our LGV which, while normally CSS is used for cascading and would be "good" for helping style our LGV, I think our little component is a little "sacrosanct" and not to be fiddled with by parent styles

@cmdcolin
Copy link
Collaborator Author

cmdcolin commented Mar 3, 2021

My initial comment noted also that examples of our embedded code like https://elliothershberg.shinyapps.io/sars-cov-2-spike-mutations/ can be seen to have somewhat weird styles presumably by parent styles leaking into the embedded lgv

@codecov
Copy link

codecov bot commented Mar 4, 2021

Codecov Report

Merging #1763 (d95444b) into master (6f27b40) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1763      +/-   ##
==========================================
- Coverage   58.74%   58.73%   -0.01%     
==========================================
  Files         455      455              
  Lines       21033    21033              
  Branches     4979     4979              
==========================================
- Hits        12355    12353       -2     
- Misses       8368     8370       +2     
  Partials      310      310              
Impacted Files Coverage Δ
...BrowseLinearGenomeView/JBrowseLinearGenomeView.tsx 87.50% <ø> (ø)
...inearGenomeView/components/RefNameAutocomplete.tsx 89.36% <0.00%> (-4.26%) ⬇️
packages/core/util/index.ts 82.20% <0.00%> (ø)

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 6f27b40...d95444b. Read the comment docs.

@cmdcolin cmdcolin added internal enhancement New feature or request and removed needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) internal labels Mar 4, 2021
Copy link
Member

@elliothershberg elliothershberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Simple change, but probably useful. Do you see this having any impact on some of the CSS weirdness in the Nextstrain demo? Might be more related to some hardcoded numbers in their layout, but stuff got pushed around there.

On a related note, it might be good to add a prop to the component that controls the paper depth of the parent div of the component. I can see instances (like Nextstrain) where people might not want the default depth/shadow effect.

@rbuels
Copy link
Contributor

rbuels commented Mar 4, 2021

@elliothershberg yes I think this will probably fix a lot of the weirdness in the nextstrain demo

@rbuels rbuels merged commit cb93ddb into master Mar 4, 2021
@rbuels rbuels deleted the fix_textalign_polygon branch March 4, 2021 20:39
@cmdcolin cmdcolin changed the title Add a "CSS reset" to the jbrowse-react-linear-genome-view, plus storybook example showing that things like parent text-align and font don't affect inside the LGV div Add a "CSS reset" to the jbrowse-react-linear-genome-view to prevent styles from outside the embedded component leaking in Mar 4, 2021
@cmdcolin cmdcolin changed the title Add a "CSS reset" to the jbrowse-react-linear-genome-view to prevent styles from outside the embedded component leaking in Add a "CSS reset" to jbrowse-react-linear-genome-view to prevent parent styles from outside the component leaking in Mar 4, 2021
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.

Embedded mode can incorrectly position scalebar polygon if parent node has some certain stylings
3 participants