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

willLeave and willChangeContext events #89

Merged
merged 1 commit into from
Apr 22, 2014
Merged

Conversation

ef4
Copy link
Collaborator

@ef4 ef4 commented Apr 8, 2014

This supercedes emberjs/ember.js#4233

My intention is to expose a public API that's more fine-grained than willTransition. The problem with willTransition is that it fires on your route regardless of whether your route is actually going to be affected by the transition. The user is left crawling through private state on the transition object to figure out whether their route is really going away or changing context.

This change creates two new events: willLeave and willChangeContext. willLeave fires on routes that will no longer be active after the transition. willChangeContext fires on routes that will still be active but will re-resolve their models. Both of these hooks act like willTransition in the sense that they give you an opportunity to abort the transition before it happens. Common use cases include animating things away or prompting to user to deal with unsaved changes.

If this PR is merged, I'll make a (documentation-only) PR against ember itself. As far as I can tell, no code changes will be necessary there.

I left willTransition alone, so this should be backward compatible with stable Ember APIs.

ef4 added a commit to ef4/ember-animation-demo that referenced this pull request Apr 8, 2014
@ef4
Copy link
Collaborator Author

ef4 commented Apr 8, 2014

And here's an example of how this API simplifies my animation demo app and removes dependence on private properties of the transition object: ef4/ember-animation-demo@3f79d70

@machty
Copy link
Contributor

machty commented Apr 14, 2014

This looks good to me; the only hesitation I have is that we'd probably wanna feature flag this on the Ember side of things so that after we import this code change into Ember (and it goes into beta and then release), if we wanted to change the API in some way, we'd be breaking apps for the folk that looked into router.js and found these seemingly public API hooks. I'll bring this up w the other core folk to see what makes sense.

@ef4
Copy link
Collaborator Author

ef4 commented Apr 14, 2014

Right. It's not obvious how to apply Ember's feature flags to a feature like this in router.js. Possibly router.js needs to sniff for the existence of Ember and respect Ember's feature flags. Alternatively, Ember could actively detect and complain about willLeave, etc if it sees them on a user's route without the appropriate feature flag.

@machty
Copy link
Contributor

machty commented Apr 22, 2014

The other thing we could do is expose methods on Router prototype that send those 3 events and Ember can conditionally stub those out depending on a feature flag. How does that sound?

@machty
Copy link
Contributor

machty commented Apr 22, 2014

I'll do it :)

machty added a commit that referenced this pull request Apr 22, 2014
willLeave and willChangeContext events
@machty machty merged commit 520de07 into tildeio:master Apr 22, 2014
machty added a commit to machty/ember.js that referenced this pull request Apr 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants