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

@vx/axis ticklabelprops dy not working #267

Closed
maganuk opened this issue Apr 9, 2018 · 6 comments
Closed

@vx/axis ticklabelprops dy not working #267

maganuk opened this issue Apr 9, 2018 · 6 comments

Comments

@maganuk
Copy link

maganuk commented Apr 9, 2018

After update to 0.0.159 or 0.0.160 from 0.0.158, setting the dy in ticklabelprops on the left or right axis doesnt seem to work.

tickLabelProps={() => ({ dy: '-0.4em', textAnchor: 'start' })}

@hshoff hshoff added the 🐛 bug label Apr 9, 2018
@hshoff
Copy link
Member

hshoff commented Apr 9, 2018

Thanks for the bug report @maganuk! We replaced axis tick labels with @vx/text in v0.0.159. Looks like we need to update@vx/text to support em/rem values for dx/dy

For now, the workaround is to override your Axis tickComponent:

<AxisLeft
   // ...stuff
   tickLabelProps={() => ({
     dy: '-0.4em',
     textAnchor: 'start'
   })}
   tickComponent={({ formattedValue, ...tickProps }) => (
      <text {...tickProps}>{formattedValue}</text>
   )}
/>

@sto3psl
Copy link
Contributor

sto3psl commented Apr 10, 2018

I would be happy to come up with a PR for this. The calculation for em to px should be pretty straightforward with the font size of the <Text /> element. But for rem I'm not quite sure. Would getComputedStyle(document.documentElement).fontSize be the correct way? It returns the font size in px which should equal 1rem.

@techniq
Copy link
Collaborator

techniq commented Apr 10, 2018

@sto3psl based on a quick search that should work. Might be better to do this in userland and pass the pixel value to <Text dy={...} />?

screen shot 2018-04-10 at 8 16 10 am

@sto3psl
Copy link
Contributor

sto3psl commented Apr 10, 2018

@techniq I agree. I think supporting em is fine and if one needs rem it's not hard to do in userland. There can even be a hint in the docs how to work with rem.

@hshoff
Copy link
Member

hshoff commented Apr 10, 2018

This plan sounds good to me

@hshoff
Copy link
Member

hshoff commented Apr 28, 2018

fixed by #271

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

No branches or pull requests

4 participants