React-spring: height: 'auto' calculates incorrect height for some elements

[taylorlapeyre]

Hi again! Looks like the height calculation in fixAuto is not ending up with the right height for some elements.

I've noticed that it happens with elements who have text that wraps and makes the height larger than it would otherwise be. Increasing the width of those elements to the point where the text no longer wraps results in the row being the correct height.

I have a suspicion that this is due to using clientHeight, which has bitten us a fair amount as well. We usually use getBoundingClientRect().height these days - might be worth a look!

Example:

[drcmda]

@taylorlapeyre Could you try it? You don't have to fork, make a local copy of this file: https://github.com/drcmda/react-spring/blob/master/src/animated/targets/react-dom/fix-auto.js

Replace import AnimatedValue from '../../AnimatedValue' with import { AnimatedValue } from 'react-spring' + your changes to the measuring strategy

And then do this:

import MyOwnFix from './local-fix-auto'

<Spring inject={MyOwnFix} ... />

If it works we can upstream it.

It could also happen if the object doesn't have bounds, because it is projected to the document root without any container that restricts it. If if doesn't already at least a width, it can cause wrong measurement. This can be fixed in userland by measuring out the cell width of your grid and applying it to the objects styles.

The reason i don't render it directly where it is supposed to be is that i fear it would cause flicker. The object would appear and cause a paint, then we measure bounds and make it disappear again, causing another paint. It would potentially push away other elements for two frames that way.

[drcmda]

PS. You can remove the portal simply by omitting "createPortal" and return the <div ...> inside as is, then it will render inside your grid. Maybe the browser does it so fast that there isn't any flicker - who knows.

function fixAuto(spring, props) {
  const { native, children, from, to } = props

  // Dry-route props back if nothing's using 'auto' in there
  if (![...getValues(from), ...getValues(to)].some(check)) return

  const forward = spring.getForwardProps(props)
  const allProps = Object.entries({ ...from, ...to })

  // Collect to-state props
  const componentProps = native ? allProps.reduce(convert, forward) : { ...from, ...to, ...forward }

  // Render to-state vdom to portal
  return (
    <div
      style={{ visibility: 'hidden' }}
      ref={ref => {
        if (ref) {
          // Once it's rendered out, fetch bounds
          const height = ref.clientHeight
          const width = ref.clientWidth

          // Defer to next frame, or else the springs updateToken is canceled
          requestAnimationFrame(() =>
            spring.updateProps(
              {
                ...props,
                from: Object.entries(from).reduce(overwrite(width, height), from),
                to: Object.entries(to).reduce(overwrite(width, height), to)
              },
              true
            )
          )
        }
      }}>
      {children(componentProps)}
    </div>
  )
}

[taylorlapeyre]

@drcmda thanks! While trying this out I ran into a small issue - we are using Transition here, not Spring directly. Is there any way to inject my new version of fixAuto with this setup?

[drcmda]

Should be the same, Transitions should be able to take in injects as well.

[a-type]

I've also encountered this problem. I tried patching with getBoundingClientRect(), but it resolved the same height as clientHeight. I did notice when logging the rect that the width was massively different from the actual element bounds; close to the total width of the page instead of the smaller area my collapsible element was contained in.

Removing the portal fixed this, now it behaves as I'd expect. Makes sense.

I didn't understand @drcmda at first, but yeah, I guess you said that would happen 😄

It could also happen if the object doesn't have bounds, because it is projected to the document root without any container that restricts it. If if doesn't already at least a width, it can cause wrong measurement. This can be fixed in userland by measuring out the cell width of your grid and applying it to the objects styles.

I'm not totally satisfied by the userland solution though. I'd love to be able to apply springs to any arbitrary elements without having to think about measuring bounds or adhering to a strict grid.

I wonder if there's another possible hack... like rendering it with overflow-y: auto; height: 0; and measuring the client width and scroll height?

Update: just tried it myself, seems to work acceptably to me. There may be some browser quirks with scrollHeight I'm not aware of though.

import React from 'react';
import ReactDOM from 'react-dom';
import { AnimatedValue } from 'react-spring';

const getValues = object => Object.keys(object).map(k => object[k]);
const check = value => value === 'auto';
const convert = (acc, [name, value]) => ({
  ...acc,
  [name]: new AnimatedValue(value),
});
const overwrite = (width, height) => (acc, [name, value]) => ({
  ...acc,
  [name]: value === 'auto' ? (name === 'height' ? height : width) : value,
});

export default function fixAuto(spring, props) {
  const { native, children, from, to } = props;

  // Dry-route props back if nothing's using 'auto' in there
  if (![...getValues(from), ...getValues(to)].some(check)) return;

  const forward = spring.getForwardProps(props);
  const allProps = Object.entries({ ...from, ...to });

  // Collect to-state props
  const componentProps = native
    ? allProps.reduce(convert, forward)
    : { ...from, ...to, ...forward };

  return (
    <div
      style={{ overflowY: 'auto', height: 0 }}
      ref={ref => {
        if (ref) {
          // Once it's rendered out, fetch bounds
          const height = ref.scrollHeight;
          const width = ref.clientWidth;
          // Defer to next frame, or else the springs updateToken is canceled
          requestAnimationFrame(() =>
            spring.updateProps(
              {
                ...props,
                from: Object.entries(from).reduce(
                  overwrite(width, height),
                  from,
                ),
                to: Object.entries(to).reduce(overwrite(width, height), to),
              },
              true,
            ),
          );
        }
      }}
    >
      {children(componentProps)}
    </div>
  );
}

[drcmda]

@a-type The first solution, just leaving out the portal, worked? Or did you run into another issue with that. I don't quite follow with the scrollheight stuff. And did you notice any flicker? It's weird, there should be, but i haven't seen any - but anyway, the inject stuff is the right way, userland shouldn't be too complex i agree fully. We only need to hack out a good strategy for it and i'll update.

[a-type]

@drcmda The portal was my problem. clientHeight was no different from getClientBounds, so that was a red herring. But removing the portal fixed the size.

I didn't see any flicker when I removed the portal, but just to be sure, I suggested another solution which I posted the code for above. Basically, you render the element in place with height: 0; overflow-y: auto. Then, you have an accurate width. Instead of taking clientHeight, you take scrollHeight.

While this hack works for me, I'm not sure it would work for everyone.

[drcmda]

Now I get it, that‘s really clever! Is that a common thing or did you just unearth it? Could you click the PR button on your fork, it’s only missing width and we could merge.

[a-type]

Just occurred to me as I was tinkering with it. I'll open a PR soon.

[drcmda]

@a-type Sorry for the wait, i merged, and will publish soon.

[nathanmarks]

@drcmda Do you know when you might release this? Deciding whether to add the fix to my codebase or wait it out

[drcmda]

@nathanmarks It's out under @5.1.3, i actually thought i already did publish hence the close - must've been confused. 😵

[drcmda]

@a-type @taylorlapeyre ran into some crazy issues recently with the previous approaches, the flicker is real - on mobile i could see it, and in some cases the calculation was off depending on box-sizing: border-box/content-box. I've made a couple of revisions cherry picking from everything we've discussed so far - it doesn't get clientHeight any longer but calculates using getComputedStyles, pretty much like jquery does, also checking for the box model. Also tried position: absolute.

Would be glad if you could try it again, if it still works in your projects.

[a-type]

@drcmda I didn't have luck with position: absolute originally, but would be willing to test your changes. Box model improvements sound welcome.

[ncordin]

Hi there :)

I'm having the exact same issue with react-spring 5.3.8.

Here is my code sandbox: https://codesandbox.io/s/2o9pxo895y

Any browser width > 300px will cause the problem. If you fix the content width (see comment at bottom), then it always works fine.

[drcmda]

@dindonus

You can help it out a little by providing more context:

        <div style={{ position: 'relative'}}>
          <h1>
            <button onClick={toggle}>Toggle</button>
          </h1>
          <Spring from={{ height: 0 }} to={{ height: on ? "auto" : 0 }}>
            {styles => (
              <div style={{ ...styles, overflow: 'hidden' }}>

Now the lower div container is part of the parent-div instead of the app/root-div (in css absolute/relative is always relative to the previous container that's absolute/relative, even if it's not the direct parent). When it's set to 'absolute' by react-spring for measurement it'll retain bounds. I know this stuff can be confusing and i wish there'd be an easier way, but it's css after all.

More on that here: https://github.com/drcmda/react-spring/blob/master/API-OVERVIEW.md#animating-auto