Stanko / react-animate-height

Lightweight React component for animating height using CSS transitions. Slide up/down the element, and animate it to any specific height.
https://muffinman.io/react-animate-height
MIT License
758 stars 53 forks source link

Typescript errror: Type '(props: { newHeight: number; }) => void' is not assignable to type '(event: AnimationEvent<HTMLDivElement>) => void #88

Closed remijean closed 4 years ago

remijean commented 4 years ago

Code example

<AnimateHeight
    duration={300}
    height={childrenHeight}
    onAnimationStart={(props: { newHeight: number }) =>
        handleAnimationStart(props.newHeight)
    }
>

Expected behavior Typescript error Type '(props: { newHeight: number; }) => void' is not assignable to type '(event: AnimationEvent<HTMLDivElement>) => void:

Capture d’écran de 2019-12-23 15-55-52

Possible Solution

Remove & HTMLAttributes<HTMLDivElement> ?

export type AnimateHeightProps = {
  animateOpacity?: boolean;
  animationStateClasses?: AnimationStateClasses;
  applyInlineTransitions?: boolean;
  children: ReactNode | ReactNode[];
  className?: string;
  contentClassName?: string;
  delay?: number;
  duration?: number;
  easing?: "ease" | "linear" | "ease-in" | "ease-out" | "ease-in-out" | string;
  height?: string | number;
  onAnimationEnd?(props: { newHeight: number }): void;
  onAnimationStart?(props: { newHeight: number }): void;
  style?: CSSProperties;
} & HTMLAttributes<HTMLDivElement>
Stanko commented 4 years ago

Hey @remijean, I think the problem here are native animation callbacks (onanimationstart for example).

HTMLAttributes<HTMLDivElement> have to be there, so users can easily add other attributes (aria-* for example).

I think changing the order should fix it

export type AnimateHeightProps = HTMLAttributes<HTMLDivElement> & { 
  /* props go here */ 
};

I'll publish a new version soon.

Stanko commented 4 years ago

Published in 2.0.18, can you please try it?

remijean commented 4 years ago

@Stanko always the same error :cry:

Capture d’écran de 2020-01-16 16-57-11

Stanko commented 4 years ago

Oh, I think I know what it is. It seems it is clashing with native attributes: https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers/onanimationstart https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers/onanimationend

I'll try to fix it this weekend.

I could rename callback props, but I would like to avoid it as it makes a breaking change.

Stanko commented 4 years ago

I just tested it with version 2.0.18 and react create app (with typescript), and everything works as expected. This is the exact code I'm using:

import React, { useState } from 'react';
import AnimateHeight from 'react-animate-height';

const Home: React.FC = () => {
  const [height, setHeight] = useState<string|number>(0);

  return (
    <div>
      <button onClick={() => setHeight(height === 0 ? 'auto' : 0)}>Toggle</button>
      <AnimateHeight
        height={height}
        onAnimationStart={() => console.log('start')}
        onAnimationEnd={() => console.log('end')}
      >
        Vestibulum eget purus in risus elementum bibendum. 
        Nunc et sapien mattis, tincidunt mi ut, sollicitudin mi. Morbi vel nunc enim. 
        Nunc vehicula ligula magna, non mattis neque suscipit in.
      </AnimateHeight>
    </div>
  );
}

export default Home;

Please make sure you updated to 2.0.18

remijean commented 4 years ago

@Stanko Yes that's right, you can try to use Omit /Pick / Exclude to overwrite the two attributes: https://stackoverflow.com/a/56916686 https://stackoverflow.com/a/48216010

remijean commented 4 years ago

@Stanko I'll check again :+1:

Stanko commented 4 years ago

Thank you! I think the order change I did:

 export type AnimateHeightProps = HTMLAttributes<HTMLDivElement> & AnimateHeightProps;

should do the trick. As far as I understand it AnimateHeightProps' onAnimationStart/End should overwrite the HTMLAttributes ones.

remijean commented 4 years ago

@Stanko results of my new test:

This code works:

import React, { FC } from 'react'
import AnimateHeight from 'react-animate-height'

export const Test: FC = () => {
  return (
    <AnimateHeight onAnimationStart={() => console.log(true)}>
      Hello world!
    </AnimateHeight>
  )
}

export default Test

This code doesn't work:

import React, { FC } from 'react'
import AnimateHeight from 'react-animate-height'

export const Test: FC = () => {
  return (
    <AnimateHeight onAnimationStart={(props: { newHeight: number }) => console.log(props.newHeight)}>
      Hello world!
    </AnimateHeight>
  )
}

export default Test

Screenshot:

Capture d’écran de 2020-01-17 12-06-17

My packages.json:

Capture d’écran de 2020-01-17 12-05-37

Stanko commented 4 years ago

Dang! My bad again 🙈 Omit it is! I'll try to get the new version out asap.

Stanko commented 4 years ago

https://github.com/Stanko/react-animate-height/commit/b6ed68677f91ea33ba74ec7e964bd99337464a04#diff-ad4b8db0d72d668877ca1a852f2d2079

There it goes, please try version 2.0.19

And thank you for bearing with me :)

Stanko commented 4 years ago

It is not my day today, I added onAnimationStart twice :/ and forgot onAnimationEnd 2.0.20 coming up

Stanko commented 4 years ago

Published 2.0.20 🎉 I made this way more complicated than it should be :)

remijean commented 4 years ago

Ahah, thank you it works perfectly! :fire: