search

venantius / accountant

ClojureScript navigation for single-page applications, made simple.
Eclipse Public License 1.0
250 stars 39 forks source link

Add unconfigure-navigation! function #58

Closed tomjkidd closed 6 years ago

tomjkidd commented 6 years ago

Attempts to address the feature request from https://github.com/venantius/accountant/issues/57.

After digging into the code, I realized that there are two event listeners that are created as part of configure-navigation!, and I capture the listener keys for them. unconfigure-navigation! uses these keys to allow unlistenByKey to clean them up.

venantius commented 6 years ago

Thanks. This mostly looks good to me; the only thing I don't love is the term unconfigure-navigation!. It doesn't feel like the right term to reflect what's happening, and makes me sort of uncomfortable with the original term configure-navigation!.

I feel like we should re-name these to something more like start-event-listeners! and stop-event-listeners! (not wedded to those specific terms, but they're a good starting point for something that's more reflective of what's actually going on). @iku000888, what do you think?

tomjkidd commented 6 years ago

I'm all for whatever changes y'all want to make, just didn't want to change the public API without that being your desire. I tend to use setup/teardown, but again am open to whatever you prefer. Also, there are some aspects of the configure-navigation! that I did not seek to undo (the setUseFragment, setPathPrefix, and setEnabled calls) because I wasn't quite confident of the best approach or if it was necessary.

venantius commented 6 years ago

I'm going to merge this as is for now. When and if we decide to rename these function calls we'll want to do a major version bump and I suspect what we'll want to do first is run with this for a few minor versions while people let us know if they want to tweak setUseFragement etc, per your comment.