react-fittext icon indicating copy to clipboard operation
react-fittext copied to clipboard

Successive renders reapply compressor

Open damiangreen opened this issue 6 years ago • 9 comments

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.)

damiangreen avatar Aug 28 '19 14:08 damiangreen

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:

  • Editor https://codesandbox.io/s/objective-pasteur-f8ryw
  • Browser https://f8ryw.csb.app

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

kennethormandy avatar Aug 28 '19 16:08 kennethormandy

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

damiangreen avatar Aug 28 '19 19:08 damiangreen

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.

kennethormandy avatar Aug 28 '19 19:08 kennethormandy

thanks @kennethormandy I'll have a play

damiangreen avatar Aug 28 '19 19:08 damiangreen

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

damiangreen avatar Aug 28 '19 20:08 damiangreen

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.

kennethormandy avatar Aug 28 '19 21:08 kennethormandy

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

damiangreen avatar Aug 29 '19 08:08 damiangreen

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 avatar Aug 29 '19 23:08 kennethormandy

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

damiangreen avatar Aug 30 '19 07:08 damiangreen