jasonmit / ember-onbeforeunload

invoke logic when transitioning between routes or closing window
MIT License
30 stars 6 forks source link

canUnload and allowUnload is confusing #12

Closed blimmer closed 8 years ago

blimmer commented 8 years ago

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?

jasonmit commented 8 years ago

Completely agree, and I am also ago with breaking the API for a saner future.

blimmer commented 8 years ago

Sounds good - I've created an 0.4.0 milestone that will (at least at first) contain all the open issues. If we're going to rework this, we might as well tackle all the breaking changes and add tests as we go.