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

getStringWidth leaves an empty svg in the document body. #356

Closed
justnick21 opened this issue Sep 18, 2018 · 7 comments · Fixed by #358
Closed

getStringWidth leaves an empty svg in the document body. #356

justnick21 opened this issue Sep 18, 2018 · 7 comments · Fixed by #358

Comments

@justnick21
Copy link

justnick21 commented Sep 18, 2018

When using Text from @vx/text. The getStringWith function seems to be leaving an empty svg on the body of my document which messes up the document height:

<svg style="width: 0px; height: 0px;"><text id="__react_svg_text_measurement_id">CM</text></svg>
@mmartinsky
Copy link
Contributor

I believe the SVG is required, had the same issue myself. Setting the following CSS to target svg's that are direct children of body solved my issues:

body > svg {
    display: none
}

Your actual charts and svg's are almost certainly wrapped in some sort of div, so they won't be affected by this rule.

@hshoff
Copy link
Member

hshoff commented Sep 18, 2018

Hi @justnick21, as @mmartinsky said the svg is required, here's the reasoning for this: #218 (comment)

@techniq think we should display: none the svg by default?

@techniq
Copy link
Collaborator

techniq commented Sep 18, 2018

@hshoff display: none should be fine. I thought it being width and height of 0px would be the same result, but if @justnick21 is having issues from this, I don't see any reason why we couldn't do this as well (and just be safe).

@hshoff
Copy link
Member

hshoff commented Sep 18, 2018

fixed in #357 will be released in v0.0.175

@hshoff hshoff closed this as completed Sep 18, 2018
@justnick21
Copy link
Author

@hshoff Setting the css to display: none seemed to mess up the text for me. It looked like it broke the measurement util. I have solved this for now by setting the position to absolute instead of display: none. It might be worth double checking that the measurement still works with update #357.

@hshoff
Copy link
Member

hshoff commented Sep 18, 2018

@justnick21 yup will verify before releasing

@loopmode
Copy link

For measuring, I usually place elements in negative offscreen using position: fixed and e.g. top: -1000px, never had issues with that.
Also, visibility: hidden is better than display: none in such cases because indeed, browser layout algorithms tend to be disabled by display: none altogether, leading to all kinds of quirks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants