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

Allow for custom channels on route change. #17

Merged
merged 4 commits into from Jan 4, 2016
Merged

Allow for custom channels on route change. #17

merged 4 commits into from Jan 4, 2016

Conversation

ghost
Copy link

@ghost ghost commented Jan 3, 2016

It is necessary on large single page apps to mutate the app state in some way when the route uri changes. This pull request adds route based callbacks for when the route uri changes. This addresses my concerns in #16 . All of the tests still pass.

twilson63 pushed a commit that referenced this pull request Jan 4, 2016
Allow for custom channels on route change.
@twilson63 twilson63 merged commit 08c35b2 into twilson63:master Jan 4, 2016
@nikuda
Copy link
Collaborator

nikuda commented Mar 31, 2016

I don't think this should have been merged. It seems to duplicate existing functionality and more importantly has no tests/use cases. @twilson63

In #16:

When writing large single page applications, it would be nice to be able to listen for changes on a route, and perform some ajax/other side effect work when the route state get's mutated.

var state = mercury.struct({
  route: Router()
});

route in this case is an observable, changes of which can be listened to:

state.route(function (change) {
  console.log(change)
})

And more importantly:

For example, if '/animals' changes to 'animals/:id', do some work to load that resource into the app state. Is there a built in way to do this, or do I have to modify the router to get it to work? I apologize if this is a stupid question, I'm still new to mercury.

This is the whole point of mercury-router, and route-view in particular:

routeView({
  '/': renderHome,
  '/animals': renderAnimals,
  '/animals/:id': renderAnimalItem
}, { route: state.route })

If you want to make requests you can either observe state.route or have specific channel events to trigger them.

@nikuda
Copy link
Collaborator

nikuda commented Mar 31, 2016

@coballast can you confirm that the above suggestions work for you?

@twilson63
Copy link
Owner

@nikuda

Thanks for your pull request, I made you a collaborator so that you can
make any adjustments, share your npm uid and I will give you publish rights.

Thanks

Tom

On Thu, Mar 31, 2016 at 6:58 PM, nikuda [email protected] wrote:

@coballast https://github.com/coballast can you confirm that the above
suggestions work for you?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#17 (comment)

Tom Wilson
Jack Russell Software, A Tabula Rasa Healthcare Division
494 Wando Park Blvd
Mount Pleasant, SC 29464
Phone: 843-606-6484
Mobile: 843-469-5856
Email: [email protected]
Web: http://www.jackhq.com
Calendar:
http://www.google.com/calendar/embed?src=tom%40jackrussellsoftware.com&ctz=America/New_York
http://www.jackhq.com/calendar

This e-mail may contain information that is confidential, privileged or
otherwise protected from disclosure by the Health Insurance Portability and
Accountability Act (HIPAA) and other state and federal laws. This
information is intended only for the individual names above. Any review,
use disclosure or dissemination of this material is strictly prohibited.
If you receive this information in error, please notify CareKinesis
immediately at 888-974-2763 and delete the original at once.

@ghost
Copy link
Author

ghost commented Apr 2, 2016

I don't mind if you guys remove it.

@nikuda
Copy link
Collaborator

nikuda commented Apr 20, 2016

Thanks @twilson63. My NPM uid is nikuda.

I will revert the changes made in this PR when I get a free moment. Documentation is probably what needs to be updated too, to make usage clearer.

nikuda added a commit to nikuda/mercury-router that referenced this pull request Apr 20, 2016
This reverts commit 08c35b2, reversing
changes made to 3698a92.
twilson63 pushed a commit that referenced this pull request Apr 20, 2016
Revert #17 and example/test/doc clean up
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants