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

Successive renders reapply compressor #4

Open
damiangreen opened this issue Aug 28, 2019 · 9 comments
Open

Successive renders reapply compressor #4

damiangreen opened this issue Aug 28, 2019 · 9 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@damiangreen
Copy link

If i resize the window with this component in with a compressor value of <0 the text keeps shrinking and shrinking until its almost invisible

The same for text that is >1.
Resizes cause it to grow and grow.

This issue didn't happen with react-fittext. (I'm trying to do a direct swap of your component with theirs.)

@kennethormandy kennethormandy added the question Further information is requested label Aug 28, 2019
@kennethormandy
Copy link
Owner

Hey, thanks for the issue report.

Setting the compressor to 0 or less isn’t a valid value, and does put a warning in the console if it’s 0 but I can also make it warn for less than 0. I’m open to other suggestions here too.

For the other part, I’m not able to reproduce it, or maybe I’m misunderstanding. Here’s a comparison against the other react-fittext component with a few different compressor values:

Would you mind modifying it or making a new demo that demonstrates the problem? Thanks again.

@damiangreen
Copy link
Author

Hi @kennethormandy thanks for your response

I tried to change the css to closer match my real world app from your example

https://codesandbox.io/s/jovial-sara-wrshs?fontsize=14

if you refresh this browser, and then take the browser out of full screen mode and resize the brwoser a few times the text just gets bigger and bigger

I'm not sure if this is due to the flex box container

@kennethormandy
Copy link
Owner

Thanks, I see what you mean.

How about this? https://codesandbox.io/s/eager-resonance-sdjru

if you refresh this browser, and then take the browser out of full screen mode and resize the brwoser a few times the text just gets bigger and bigger

I'm not sure if this is due to the flex box container

Possibly, yes. I see what you mean, but as soon as I changed that span to a div, that behaviour disappeared.

It’s also because the other react-fittext expects a single React node, and then adds the new fontSize to that node. So if you have one h1, it will add the font size directly to that. This component provides the wrapper div for you, so you can do things like this: http://react-fittext.kennethormandy.com/?selectedKind=FitText&selectedStory=with%20children%20in%20fixed%20sizes&full=0&addons=0&stories=1&panelRight=0

So that’s why the h1 is a different size between the two versions: the parent font size is being set to ex. 220px and then the h1 defaults to 2em, so the size increases from there. You could change it to:

<h1 className="text">
  <FitText compressor={0.5}>The quick brown fox</FitText>
</h1>

Or something like:

<FitText compressor={0.5}>
  <h1 className="text" style={{ fontSize: '1em' }}>The quick brown fox</h1>
</FitText>

Let me know how that works out for you.

@damiangreen
Copy link
Author

thanks @kennethormandy I'll have a play

@damiangreen
Copy link
Author

damiangreen commented Aug 28, 2019

2019-08-28_22-11-13
ok that got me a little futher but I also noticed another difference between this one and react-fittext. react-fittext appears to detect when the container has changed in size to recalculate the font size.

I have this inside a resizable widget and it appears that because the props to FitText are not changing it doesn't redraw?

I appreciate I may be hijacking the original issue but it would be nice if i coudl start using your lib since it supports the latest react.

Here's a screen-grab of our app, we have re-sizable widgets on a dashboard using react-grid-layout

image

I've compared the other library and the only obvious difference is the use of window.requestAnimationFrame.

When i get more time i can try and create another codesandbox with a button on that changes the width of a containing div (not just the window) to see if the ReactFitText updates

@kennethormandy
Copy link
Owner

kennethormandy commented Aug 28, 2019

Just to clarify, are you looking for the font size to scale to the full size of the container? If so, there are some other options discussed in the README that will solve your problem, that’s not really what FitText is best at. The other React FitText and the original jQuery library doesn’t aim to do this either.

As far as I know, the other React FitText library always scales based on the window/body as well, not the parent container, but I can see how that would be useful.

I would be open to a Pull Request that builds on the parent prop (still kind of rough) that accepts a DOM node (probably via React ref) to use in place of the body. It’s started, but right now it only works along with the vertical prop for vertical scaling which I had a specific use case for. http://react-fittext.kennethormandy.com/?selectedKind=FitText&selectedStory=with%20scaling%20based%20on%20vertical%20of%20different%20parentNode&full=0&addons=0&stories=1&panelRight=0

I might also take that opportunity to look at something like react-size-aware to manage this.

@damiangreen
Copy link
Author

Hi thanks again for your swift response.
In answer to your question I was hoping for a 1:1 swap out from react-fittext to your library.

So I've made a codesandbox that hopefully demonstrates it: https://codesandbox.io/s/confident-snow-iwspo?fontsize=14

If you click the increase /decrease width buttons a few times with the browser un-maximised and then resize the browser you see that this library only updates the content on browser resize whereas react-fittext updates every time a button is clicked (when the div containing FitText width changes).
I'ts more noticeable when decreasing the width.
regards

@kennethormandy
Copy link
Owner

Thanks very much for the test case, that helps a lot, and I see what you mean.

I think it would either make sense to:

  • Re-implement that detail of react-fittext, or
  • Improve use of the parent prop and using the react-size-aware hook I mentioned in my previous comment, so the sizing is can always be triggered based on the parent, rather than the body

If the other react-fittext does this well enough for you already, that might be your best bet for now. I’ll keep this issue open to track this, and I do like the idea of supporting this use case, but full disclosure I might not get to it in the immediate term.

@kennethormandy kennethormandy added the enhancement New feature or request label Aug 29, 2019
@damiangreen
Copy link
Author

No worries, maybe if I get some time I can explore creating a PR.
regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants