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

Question about scaleToFit behavior #1360

Closed
kyythane opened this issue Oct 21, 2021 · 7 comments
Closed

Question about scaleToFit behavior #1360

kyythane opened this issue Oct 21, 2021 · 7 comments

Comments

@kyythane
Copy link
Contributor

Hello! I was hoping to use the scaleToFit option on Text to shrink the font-size of a number if it was longer than the space available to display it. Unfortunately, it appears that the behavior of scaleToFit scales the size of the text up as well.

Is there an existing workaround to do what I need via Text (only shrink the size if it is too long)?
If not, would you be open to a PR that adds this option?

Though, I am not certain the best way to add it. Adding a second prop seems somewhat clunky. Updating the existing prop to have shrink-only (or something similar) would be more ergonomic, but might be awkward to do in a backward-compatible way.

@williaster
Copy link
Collaborator

Hey @kyythane 👋 thanks for checking out visx. I can't think of a workaround for this at the moment, it's not exposed in Text but you might be able to leverage the useText hook directly. It might still be a little bit tricky since this is implemented with a transform, so a PR might be the best way to add it cleanly.

I'm definitely happy to review a PR! modifying shrinkToFit: boolean to something like shrinkToFit: boolean | 'shrink-only' seems like it could be a good approach to me 👍 I agree keeping a single prop sounds good and I don't think it'd be breaking 💥 since it would still accept true/false.

@kyythane
Copy link
Contributor Author

Thanks for the quick reply! Sounds good. I'll get a PR together tomorrowish.

@kyythane
Copy link
Contributor Author

@williaster Will I need to do anything to update the docs or are those generated?

@williaster
Copy link
Collaborator

sweet! 🙏 I'll try to take a look asap, either today or tomorrow. you shouldn't need to update the docs if the typings are updated (looks like they are). the only other possible addition would be to add a demo of the functionality to the visx-demo package but def not a requirement.

@kyythane
Copy link
Contributor Author

If I get time, I'll make an update to the demo later today. I guess the easiest change would be replacing the checkbox with a dropdown.

@kyythane
Copy link
Contributor Author

@williaster I made the change, unfortunately, I can't get the demo to run locally. yarn dev:demo errors with

error - ./src/pages/_app.tsx:5:39
Syntax error: Unexpected token, expected ","

  3 | import 'prismjs/themes/prism.css';
  4 | 
> 5 | function MyApp({ Component, pageProps }: AppProps) {
    |                                        ^
  6 |   return <Component {...pageProps} />;
  7 | }
  8 | 

I assume I skipped a step somewhere, but I don't see anything in the local development instructions?

@williaster
Copy link
Collaborator

closed by #1362

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

2 participants