jasonmit / ember-onbeforeunload Goto Github PK

Hello!
Are there any current plans to get this upgraded to 3.6+? With the eventual deprecation of willTransition and didTransition. I may try taking a look at this when I get a chance. Hopefully just moving this into the init() and going off the new router events. https://deprecations-app-prod.herokuapp.com/deprecations/v3.x/#toc_transition-state

use controller as the source of truth for isDirty instead of currentModel

This is just a documentation fix: The custom message is now deprecated according to https://developer.mozilla.org/en-US/docs/Web/API/Window/beforeunload_event in all but IE.

These two terms are so close and it's a bit confusing as to what they do. This is how it works currently:

allowUnload

The return of this function says whether or not we should call canUnload(). Its default behavior is to not check the result of the canUnload method if transitioning to a child route of the route to which the ConfirmationMixin is applied.

canUnload

The return of this function says whether or not the page has dirty attributes that we should warn the user about via the onbeforeunload.

Proposal

As part of a breaking release, we change allowUnload to shouldCheckPageIsDirty and canUnload to pageIsDirty. IMO this is much clearer as to what each of the functions is doing.

Perhaps as part of this release we could also consider #10 since we'll already be breaking stuff.

@jasonmit thoughts?

We currently have two tests (yay!), but Travis isn't automatically running.

To be good Ember community members, we should discourage the use of Ember.Controller.

It would be a breaking change, but perhaps we should guide users to provide their own canUnload or have logic that utilizes this.modelFor(this.routeName) instead.

@jasonmit first thank you for this trivial looking yet complicated support for detecting when transitioning away from current view. On surface it looks easy but due to browser differences it is a bit pain.

When I navigate to a completely different route (not sub-route) and answer "yes" to the dialog onUnload is not called. I believe this is because to the browser it's not unloading anything. I was able to fix this by adding this.onUnload() after

I wanted to use your code - it works well, but there's no LICENSE file or mention in README. Could you add a license please?

The ember-onbeforeunload mixin will ONLY check for the dirty page attribute via isPageDirty when shouldCheckIsPageDirty returns false. This seems completely counter-intuitive. Why would we not check for a dirty page when shouldCheckIsPageDirty returns true?

The logic that does this is here. This constantly confuses me and my team because it seems completely backwards.

I am happy to open a PR fixing this @blimmer, would that be okay? Though, this could break any applications currently using this addon if their addon manager specification is general enough to catch the version.

This is going to require some stubbing of browser events, but it will help get our Ember Observer score up and more usage of this addon.