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

isActive should not be on history #2606

Closed
taion opened this issue Nov 28, 2015 · 7 comments
Closed

isActive should not be on history #2606

taion opened this issue Nov 28, 2015 · 7 comments
Labels

Comments

@taion
Copy link
Contributor

taion commented Nov 28, 2015

It'd be a cleaner API - lets us keep the API to history stateless, and "active" state does belong naturally to the location object.

@taion
Copy link
Contributor Author

taion commented Nov 28, 2015

If we have a concept of a "routing location" that supports isActive, we can also put routes (and params, but keep the current injection) on that location object.

@mjackson
Copy link
Member

I'd like to keep location objects as simple as possible (i.e. not add a bunch of methods that make them impossible to serialize). Also, the current design of location objects is analogous to window.location which doesn't have an isActive method.

@taion taion changed the title isActive should be on location, not history isActive should not be on history Nov 29, 2015
@taion
Copy link
Contributor Author

taion commented Nov 29, 2015

Fair enough there - the issue is more that it's on history, which is otherwise stateless and doesn't provide any way to read information about the current location other than by listening.

@taion taion mentioned this issue Nov 29, 2015
@mjackson
Copy link
Member

I agree it's a bit messy. isActive should basically be just a utility method like isActive(route, currentState). Then it has everything it needs in order to say whether or not a route is active.

@taion
Copy link
Contributor Author

taion commented Nov 30, 2015

I'm re-opening this, then.

@taion taion reopened this Nov 30, 2015
@taion
Copy link
Contributor Author

taion commented Nov 30, 2015

Route matching is a property of the history object, though - so perhaps this should be history.isActive(locationSpec, currentLocation).

@taion
Copy link
Contributor Author

taion commented Dec 2, 2015

Fold this into #2614 as well.

@taion taion closed this as completed Dec 2, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants