Introduce new option, closeOn, to override $locationChangeSuccess.

[elliottregan]

I discovered that all modals will close on a the $locationChangeSuccess event. I've made a small alteration to generalize the event. Instead of always using $locationChangeSuccess, you can now pass in an event as an (optional) option, or false to turn the feature off entirely.

This is NOT a breaking change for anyone, but makes the service more flexible for other environments. I feel like a better default is to not close the modal on any event, but allow developer to pass in whatever event they wish the user.

I'd like to update the Changelog for this commit, but I'm not sure how you'd like me to do that. It looks like it hasn't been updated in a few versions, and I'm not sure what went into those. I guess I could put a note in there about the gap and make a new entry with my update. 0.12.2?

[coveralls]

Coverage increased (+0.5%) to 85.057% when pulling 30bf89e on elliottregan:$locationChangeSuccess-optional into 126c6ec on dwmkerr:master.

[coveralls]

Coverage decreased (-84.5%) to 0.0% when pulling 30bf89e on elliottregan:$locationChangeSuccess-optional into 126c6ec on dwmkerr:master.

[joeprisk]

I would suggest that the clean up doesn't need changing, and just adds to the cyclomatic complexity.

This would have the same effect

!!rootScopeOnClose && rootScopeOnClose();

As this

// remove event watcher
if (options.closeOn) {
    rootScopeOnClose && rootScopeOnClose();
}

There is no point checking if it should close it then checking if it has something to close then closing, may aswell skip the fist part.

The creation of the modal doesn't have the functionality that you described.

if (options.closeOn) {
    rootScopeOnClose = $rootScope.$on(options.closeOn, cleanUpClose);
}

Maybe try change this to

!options.hasOwnProperty('closeOn') && (options.closeOn = '$locationChangeSuccess');

if(options.closeOn !== false) {
    rootScopeOnClose = $rootScope.$on(options.onClose, cleanUpClose);
}

Then if nothing is specified it sticks to the standard way of working, unless you specifically tell it to change, and false turns it off completely.

This is all untested and just written in here, but should work...

[coveralls]

Coverage increased (+0.5%) to 85.057% when pulling 56d0594 on elliottregan:$locationChangeSuccess-optional into 126c6ec on dwmkerr:master.

[elliottregan]

@kernowjoe Codeclimate wasn't a fan of those suggestions, but they work.

I changed the functionality in this commit to use $locationChangeSuccess as the default onClose event, which can be overridden by any value. If false is passed, nothing happens. I added that to the README.

[elliottregan]

@dwmkerr What do you think the schedule would be to get this PR merged in? If there is anything I can do to help the process, just let me know.

[dwmkerr]

Hi @elliottregan I'll look at it this week, really sorry about the delay am super crunched at work!!

[elliottregan]

@dwmkerr Thanks!

[lorenzodianni]

There are some news about this PR?

[dwmkerr]

Sorry I'd let this one go stale, it looks like there's some conflicts in the PR now which'd have to be addressed first