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

Bug while separating route declarations into several modules #9

Open
bompi88 opened this issue Jul 26, 2016 · 6 comments
Open

Bug while separating route declarations into several modules #9

bompi88 opened this issue Jul 26, 2016 · 6 comments

Comments

@bompi88
Copy link

bompi88 commented Jul 26, 2016

I have a main layout that renders a navigation bar and the content related to each route. The navigation bar is animated when rendered for the first time (Using Animate.css). When the user first goes to /, the index page is rendered and the navigation bar should animate down from the top of the window.

This works perfectly if I declare all my routes inside a single routes.js, but it seems not to work if I declare some routes in one module and the rest in another module. It seems that switching between routes that are not declared in the same file, re-renders the whole main layout (including the top bar). This behaviour results in the navigation bar animating "up" and down in some cases. I though maybe this behaviour comes from how mantra-core (or FlowRouter) handles things behind the scenes, correct me if I'm wrong.

Example illustrating the behaviour is seen below (The image is a GIF, try clicking on it if it is behaving like a static image). The route linked to the GrandView button is declared within an index module and the rest are declared within a core module.

5e7a9dbe-b713-4220-9cd7-38f126cc90f9-11065-00006ace3b0f2775

@bompi88
Copy link
Author

bompi88 commented Jul 26, 2016

Immediately after posting this issue, I understood why I get this behaviour. It's because of this particular code-snippet:

export default function (injectDeps, {FlowRouter}) {
  const MainLayoutCtx = injectDeps(MainLayout);

I'm doing this for each of my routes.js files. For this to work as I want, the MainLayoutCtx must be shared between both files, and declared in an upper scope. Is there a way of doing this with the current implementation? Let's say you want to share the main layout context between modules. How would I do this? Does this contradict with the mantra concept?

@HaareeKrishna
Copy link

HaareeKrishna commented Aug 5, 2016

@bompi88 As I understood the problem, You might need to have root level component acting as parent for all modules and do injectDeps to root parent component. It doesn't contradict mantra concept.
const RootComponentCtx = injectDeps(RootComponent); <RootComponentCtx> <Module1 /> <Module2 /> </RootComponent>

@macrozone
Copy link

run into the same issue, switching from one route of module A to route of module B results in a flicker, because the dom-tree gets emptied in the transition from one route of the other.

@bompi88 could you solve this issue?

I also do not understand your solution @HaareeKrishna. We have to call injectDeps in every route-file and the problem is, that we cannot share the resulting component between the routes.

@HaareeKrishna
Copy link

@macrozone We should not have multiple route files, routes are app specific. So only routes.js at root level and call injectDeps in that. We should not have module level route files, thus we will not encounter the problem.

@bompi88
Copy link
Author

bompi88 commented Sep 4, 2016

@HaareeKrishna Adding all routes to the core module would fix this, but the mantra specs states that routes could be module specific. I think each module should be isolated as much as possible, so you could possibly drop modules from one app to another.

@englishtom
Copy link

I agree with @bompi88 it would be better to keep the modules decoupled as much as possible. I just ran into this issue today and as far as I can see the only workable solution at the moment is to move all routes into core.

When thinking about a long term solution to this problem it's worth noting this also affects FlowRouter group definitions. To use groups defined in a different routes file you have to redeclare them.

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

No branches or pull requests

4 participants