-
Notifications
You must be signed in to change notification settings - Fork 763
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
Fix support for disappear transitions #743
base: master
Are you sure you want to change the base?
Conversation
The current setupTransitions() implementation never looks at any transitions defined on the previous LayoutState when creating AnimationBindings. This can be an issue for disappear transitions because the new LayoutState will no longer contain any disappearing items. This change adds logic in the TransitionManager to first create AnimationBindings for the previous LayoutState, then create AnimationBindings for the new LayoutState and finally merge the two.
A null check is added to the TransitionManager to cover the case where the current LayoutState does not contain any transitions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adityasharat has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Hi @adityasharat I hope you are well! Just wanted to check whether there are any thoughts / feedback on this PR. Thanks! |
Hey @nicous, |
Hey @chggr can you point to a specific case when this would be needed? |
Hi @nicous, thanks for looking into this PR! This is a transitions issue that we unearthed a while back. If you set a disappearTo value on a Component and then update the ComponentTree so that the Component is gone, the UI shows the Component disappear immediately. |
Hey @chggr just tried this and it works, maybe there's some other condition. This is the code I used to test this. @LayoutSpec
public class PlaygroundComponentSpec {
@OnCreateLayout
static Component onCreateLayout(ComponentContext c, @State boolean noShow) {
return Column.create(c)
.backgroundColor(Color.WHITE)
.clickHandler(PlaygroundComponent.onClick(c))
.child(
!noShow
? Text.create(c).textSizeSp(20).text("Playground sample").transitionKey("KEY")
: null)
.build();
}
@OnEvent(ClickEvent.class)
static void onClick(ComponentContext c, @FromEvent View view) {
PlaygroundComponent.onUpdateState(c);
}
@OnUpdateState
static void onUpdateState(StateValue<Boolean> noShow) {
noShow.set(!noShow.get());
}
@OnCreateTransition
static Transition onCreateTransition(ComponentContext c) {
return Transition.create("KEY").animate(AnimatedProperties.ALPHA).disappearTo(0);
}
} |
Summary
The current setupTransitions() implementation never looks at any
transitions defined on the previous LayoutState when creating
AnimationBindings. This can be an issue for disappear transitions
because the new LayoutState will no longer contain any disappearing
items.
This change adds logic in the TransitionManager to first create
AnimationBindings for the previous LayoutState, then create
AnimationBindings for the new LayoutState and finally merge the two.
Changelog
Fixes support for disappear transitions in TransitionManager
Test Plan
We have manually verified disappear transitions are working as expected
after this change.