react-swipeable-views icon indicating copy to clipboard operation
react-swipeable-views copied to clipboard

Replay recent PR improving animatedHeight prop

Open pandaiolo opened this issue 5 years ago • 3 comments

I believe there is no link between this code and the recent issues. And it's a valuable improvement.

Replays the following PRs from the old master:

  • #536
  • #537
  • #540

pandaiolo avatar Feb 03 '20 14:02 pandaiolo

I suspect this one introduces a regression. I don't have time to look into it. Unless somebody is willing to significantly dedicate time to this component (more than +20h/month), I would be in favor of only handling changes regarding code infrastructure and to freeze any change that might impact the behavior of the component. The logic has been running fine for years now, check react-select, it's full of open issues and buggy edge cases and yet, it doesn't prevent developers from using it. We can go with the same approach here.

Changes that seem fine to push further:

  • #524
  • #539
  • #455
  • #570.

oliviertassinari avatar Feb 03 '20 14:02 oliviertassinari

Do you mind sharing your concern about the suspecte regression? It felt that this code was modifying only the animateHeight scope of this component. Anyway I guess more tests are needed... unfortunately I don't have more time to dedicate either :)

pandaiolo avatar Feb 03 '20 15:02 pandaiolo

I have noticed that the height animation demo was no longer running the animation on the master-off-track branch. I could be coming from these changes, modifying the default behavior.

oliviertassinari avatar Feb 05 '20 21:02 oliviertassinari