Skip to content

API change proposal for useTransition #393

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

Closed
yuchi opened this issue Dec 20, 2018 · 7 comments
Closed

API change proposal for useTransition #393

yuchi opened this issue Dec 20, 2018 · 7 comments
Labels
kind: request New feature or request that should be actioned

Comments

@yuchi
Copy link
Contributor

yuchi commented Dec 20, 2018

Description

Since configuration for a transition is usually static or a static composition of functions, it makes sense to define it outside of the component body:

const config = {
  trail: 100,
  from: { opacity: 0 },
  enter: { opacity: 1 },
  leave: { opacity: 0 }
};

const keys = item => item.id;

function MyAnimation({ items }) {
  const transitions = useTransition({ items, keys, ...config });
  // ....
}

The current approach of having a single configuration argument makes this slightly less usefull since you need to spread that static configuration over the object that includes items and keys too.

Proposal

I propose to change the API to this:

useTransition(items, keys, config);

This reduce the need for the creation of a pretty useless object at render time.

Additional note

This makes the API more coherent with useTrail:

useTrail(items.length, config);
useTransitions(items, keys, config);
@drcmda
Copy link
Member

drcmda commented Dec 21, 2018

Makes sense! The only thing i don't like is that keys is optional right now, here it would take a fixed spot with this approach. On the other hand, maybe we're better off enforcing keys, making users think about this, because default keys (props => props) can fail.

@yuchi
Copy link
Contributor Author

yuchi commented Dec 25, 2018

Willing to accept a PR for this or you want to do it yourself?
I'll just wait for your confirmation on the second keys argument.

@ruggedy
Copy link
Contributor

ruggedy commented Dec 26, 2018

@yuchi I think it's a good idea. feel free to create a PR for this of course. as this is going to be a breaking change. maybe wait for @drcmda to get back from the holidays.

@yuchi
Copy link
Contributor Author

yuchi commented Jan 7, 2019

Friendly reminder :) still interested in this API change?

@drcmda
Copy link
Member

drcmda commented Jan 8, 2019

@yuchi you want to make the PR? i think this is a good change - makes total sense to do it that way.

@drcmda
Copy link
Member

drcmda commented Jan 9, 2019

btw, some new ideas here: #425

@aleclarson aleclarson added kind: request New feature or request that should be actioned PR welcome labels Apr 1, 2019
@aleclarson
Copy link
Contributor

This was implemented in v8.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: request New feature or request that should be actioned
Projects
None yet
Development

No branches or pull requests

4 participants