search

cerebral-legacy / cerebral-module-router

An opinionated URL change handler for Cerebral
http://cerebral-website.herokuapp.com/documentation/cerebral-module-router
19 stars 4 forks source link

When root route changes the page body should scroll to top #53

Closed garth closed 8 years ago

Guria commented 8 years ago

@garth what do you mean by root route changes?

garth commented 8 years ago

I mean that you may not want it for sub-routes, maybe it should be possible to opt-out.

Guria commented 8 years ago

@garth is there any ideas how we can distinguish routes from subroutes?

garth commented 8 years ago

perhaps something like this:

export default Router(controller, {
  '/': 'homeOpened',
  '/user/:user_id/': 'userOpened',
  '/user/:user_id/preferences/': { signal: 'userPreferencesOpened', noScroll: true },
  '*': 'notFoundPageOpened'
});
Guria commented 8 years ago

I not very happy with it. Also we should ask @christianalfoni.

I agree that scrollToTop is routing-related feature. But don't sure if it should be part of router. cerebral-router is great in that it has minimum impact to app. It could be disabled, but app would still able to work. From this point of view I think scrollToTop should be part of app itself.

Does it seem reasonable?

garth commented 8 years ago

I also had similar thoughts about keeping the router clean. I considered putting it into an action, but that also doesn't seem right, since actions should not really be UI related.

So we would then need a way to put the scrollToTop into the root application component.

garth commented 8 years ago

@Guria I think that we've got a solution: Use a ComponentSwitcher component that will swap out route related components and also do scrollToTop. The only question is where should it live?

garth commented 8 years ago

After some discussion, I think that this issue (whilst still not resolved) might be better handled elsewhere within the cerebral ecosystem.