Skip to content
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

expose onTransitionEnd event for react components #2187

Closed
shilpan opened this issue Sep 13, 2014 · 13 comments
Closed

expose onTransitionEnd event for react components #2187

shilpan opened this issue Sep 13, 2014 · 13 comments

Comments

@shilpan
Copy link

shilpan commented Sep 13, 2014

An onTransitionEnd event for components which is normalized across all browsers supporting transitions (and fake it for those that don't using setTimeout). Currently react uses ReactTransitionEvents internally for the TransitionGroup. However this is not useful in cases where you are not adding/removing a view.

@jeffchan
Copy link
Contributor

👍 to this. Even as a plugin would be helpful.

@idris
Copy link

idris commented Dec 11, 2014

👍 currently using this workaround:

componentDidUpdate: function() {
  this.getDOMNode().addEventListener('transitionend', this.onTransitionEnd, false);
}

Would prefer to do:

<MyComponent onTransitionEnd={this.onTransitionEnd} />

@Janekk
Copy link

Janekk commented Jan 20, 2015

+1

@idris
Copy link

idris commented Jan 20, 2015

FYI, "faking it" as @shilpan mentioned may be harder than it seems. Apparently the native transitionEnd event does not fire if the transition ends off-screen (in my case, the element is display:none). So it may be better to have React just handle the native behavior for supported browsers, and let us handle any custom code for old browsers.

@srph
Copy link
Contributor

srph commented Jan 28, 2015

Any update regarding this feature?

@akre54
Copy link
Contributor

akre54 commented Feb 25, 2015

+1

@f0rmat1k
Copy link

👍

1 similar comment
@idoros
Copy link

idoros commented Jun 11, 2015

👍

@lucasmotta
Copy link

👍 that would amazing ;)

@chris-lock
Copy link

👍

1 similar comment
@milesj
Copy link
Contributor

milesj commented Dec 29, 2015

+1

@supasympa
Copy link

+1

@zpao
Copy link
Member

zpao commented Mar 29, 2016

This was fixed by #6005 & be in v15.

@zpao zpao closed this as completed Mar 29, 2016
josephsavona added a commit that referenced this issue May 15, 2024
See context from #2187 for background about control dependencies. 

Our current `PruneNonReactiveIdentifiers` pass runs on ReactiveFunction, after 
scope construction, and removes scope dependencies that aren't reactive. It 
works by first building up a set of reactive identifiers in 
`InferReactiveIdentifiers`, then walking the ReactiveFunction and pruning any 
scope dependencies that aren't in that set. 

The challenge is control variables, as demonstrated by the test cases in #2184. 
`InferReactiveIdentifiers` runs against ReactiveFunction, and when we initially 
wrote it we didn't consider control variables. To handle control variables we 
really need to use precise control- & data-flow analysis, which is much easier 
with HIR. 

This PR adds the start of `InferReactivePlaces`, which annotates each `Place` 
with whether it is reactive or not. This allows the annotation to survive 
LeaveSSA, which swaps out the identifiers of places but leaves other properties 
as-is. This version does _not_ yet handle control variables, but it's already 
more precise than our existing inference. In our current inference, if `x` is 
ever assigned a reactive value, then all `x`s are marked reactive. In our new 
inference, each instance of `x` (each Place) gets a separate flag based on 
whether x can actually be reactive at that point in the program. 

There are two main next steps (in follow-up PRs): 

* Update the mechanism by which we prune non-reactive dependencies from scopes. 

* Handle control variables. I think we may be able to use dominator trees to 
figure out the set of basic blocks whose reachability is gated by the control 
variables. This should clearly work for if/else and switch, as for loops i'm not 
sure but intuitively it seems right.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests