faceyspacey / redux-first-router-navigation

MIT License
5 stars 0 forks source link

StackRouter nested in Tabs #1

Open MrLoh opened 7 years ago

MrLoh commented 7 years ago

I came across an issue, with setting up react navigation StackRouters within a custom tab router.

I have the following routes:

/settings/
/showtimes/
/showtimes/movie/:id

Both settings and showtimes are separate tabs and inside the showtimes tab there is a movie stack. I controll the tabs myself with a trivial tab router, that get's the active tab with this selector: state => state.location.pathname.split('/')[1].

Now the problem appears when I do the following:

  1. starting in the showtimes tab
  2. I go to a movie screen, adding to the stack
  3. I go to the settings tab
  4. I go back to the showtimes tab

Until 2 everything works fine. But when I go back to the showtimes tab, I want to activate the route /showtimes/movie/:id without adding to the stack. How do I do that easily without needing to know a ton about my navigation state.

I found that I can actually simply put a /showtimes/ route inside my routesMap and then call the corresponding action, which will activate the showtimes tab and even display the top of the stack. But now the path in my location is /showtimes/ even though it actually displays /showtimes/movie/1.

Shouldn't there be a standard way for redux-first-router to handle this?

MrLoh commented 7 years ago

My current workaround is the following code, that simply overrides the path in a custom reduxer. All the redux-first stuff is registered in the state under router.

import createMemoryHistory from 'history/createMemoryHistory';
import { connectRoutes } from 'redux-first-router';
import reduxNavigation from 'redux-first-router-navigation';

import { ShowtimesStack } from '../views/ShowtimesView';

const history = createMemoryHistory({ initialEntries: ['/showtimes/'] });

// connect stack navigators from react-navigation
const navigators = reduxNavigation({ showtimes: ShowtimesStack });

const routesMap = {
    'OPEN_SHOWTIMES': { path: '/showtimes/_'},
    'OPEN_SETTINGS': { path: '/settings'},
    'OPEN_MOVIE_IN_SHOWTIMES': { path: '/showtimes/movie/:id', navKey: 'showtimes' },
    'OPEN_SHOWTIMES_HOME': { path: '/showtimes/', navKey: 'showtimes' },
};

const {
    reducer: routesMapReducer,
    middleware: routerMiddleware,
    enhancer: routerEnhancer,
} = connectRoutes(history, routesMap, { location: state => state.router.location, navigators });

const showtimesStackReducer = (state, action) =>
    ShowtimesStack.router.getStateForAction(action, state) || state;

const routerReducer = (state = {}, action) => {
    const newState = {
        ...state,
        location: routesMapReducer(state.location, action),
        showtimes: showtimesStackReducer(state.showtimes, action),
    };
    if (action.type === 'OPEN_SHOWTIMES') {
        const actualPath = state.location.history.entries.reduce(
            (curr, { pathname }) => (pathname.split('/')[1] === 'showtimes' ? pathname : curr),
            '/showtimes/'
        );
        newState.location.pathname = actualPath;
        newState.location.history.entries[newState.location.history.index].pathname = actualPath;
    }
    return newState;
};

export { routerReducer, routerMiddleware, routerEnhancer };

export const selectMainRoute = state => state.router.location.pathname.split('/')[1];
faceyspacey commented 7 years ago

I don't suppose a repro is possible?

I'm having a bit of a hard time understanding. Basically, I think what you want to do is go to a tab and not add to the stack, and rather show what's already there.

To do that you have to dispatch the action that corresponds to it, and if hasn't changed, it will show. If you dispatch a different action with the same navKey, it will add to the stack. But if it's the same, it should be smart enough to not add to the stack.

You have to use the React Navigations action type of RESET.

There's basically no way around it--because in one way or another you need to convey whether you want a RESET or a PUSH. In this case, PUSH is default, and RESET requires a bit more work.

There are other ways though. You can customize the reducers after all. So you can dispatch a custom action that only switches tabs. But you do really want it to be URL-based and as close to the standard as possible.

Another way: make the action triggered on the tab itself smart enough to get the current tab's action and then just re-dispatch it.


I havent been in the React Navigation demo for so long, but if you give me a repro it will force to get back on it, and solve your specific problems. Providing solutions for cases like this is exactly what RFR is supposed to bring to the table.

faceyspacey commented 7 years ago

Actually, I think I just realized your problem: I made PRs to react-navigation for various things and have been assuming you have them. I made a bunch of PRs long ago (and they only merged like 1 super small one):

https://github.com/react-community/react-navigation/pulls/faceyspacey

Apply this code internally to React Navigation within node_modules and see if it solves your problem:

https://github.com/react-community/react-navigation/pull/1515

Basically I wrote code to do automatic REPLACE if the action is the same as what's already displaying: https://github.com/react-community/react-navigation/pull/1515/files

Here's the exact change:

https://github.com/faceyspacey/react-navigation/blob/4641bfc7ccbc8e75a1f8053e5c1db36ac7322fe2/src/routers/StackRouter.js#L188

basically a replace will be done if the ROUTE + PARAMS are the same.

So that should solve your case, because in one way or another you need to be dispatching what's already shown or of course it will push on to the stack without a RESET action.

Also FYI, the React Navigation people basically recommended to just externalize a bunch of their in order to customize. I plan to pull out StackNavigator all together and beat it into submission, which they clearly have no time for since they bit off far more than they can chew with all the unnecessary non-redux features, and unnecessary Drawer/TabNavigators as we discussed the other day. Supporting all that will mean the Redux-based StackNavigator with proper animations control and all these minor bugs fixed will never arrive. They also seem to never respond to any comments. Also if you look at the first 2 pages of commits:

https://github.com/react-community/react-navigation/commits/master

one of the creators, skevy was active for about a week (mid July) in the past 2 months. They blew this thing up in popularity but dont seem to have much time for it. Definitely not as much time that is necessary in relation to how many bugs they have.

That's not to knock them, but just to indicate how imperative it is that we remove StackNavigator from that repo. I wouldn't count on it serving your needs any time soon. Too many problems, and they're spread too thin, focused primarily on needs that dont affect their initial Redux base.

MrLoh commented 7 years ago

First of all. It'd be amazing to have a package that just contains a declarative version of the StackRouter without the factory stuff and so on. Unless these factories are really essential, I am not sure why they went that route. I'd switch to a working standalone package today, if it only had your pull request mentioned above merged and a proper maintainer who is more responsive.

faceyspacey commented 7 years ago

exactly. who knows what the heck they're doing. probably working high-salary jobs that require most of their time and they thought they would get to focus on this. but it's also important to know that the code there is like 2-3 years worth of work. It's in the lineage line of like 3 or 4 navigators that came before it, all built by the same people. So we can count on them finishing their bugs--they're navigator-obsessed, but it might be a year from now.

I however think they've made the core problem (of being state-driven) so much easier. I think it would be very easy to rip out StackNavigator. I'm pretty sure the community would rejoice. After all, all the most senior React coders would agree that they just want StackNavigator--so that means we'd pull away their best contributors to make sure StackNavigator works. They really made their lives hard by making something that appeals to everyone (i.e. doesn't require redux, i.e. has all 3 navigators pretty much built-in). now they have a million issues from junior developers across the planet.

MrLoh commented 7 years ago

To my problem. Your fix to react-navigation should definitely solve an important part of the problem. The other thing is that the use case is just hard to handle. When I switch between tabs, I want it to be easy meaning just firing a simple action that contains the tab name. But then what I want to happen in the store is quite complex because I need to push a different route based on the previous state.

In my code above I tried to hack this in, by routing to a dummy route /showtimes/_ and then replacing that in the if block of the routerReducer to the last route from the corresponding stack. I'm only doing this dummy thing to avoid having to do all the history updates and so on myself. Which are the real overhead of that operation.

With your fix, I should at least be able to simply dispatch the correct action and avoid any such strange overrides.

faceyspacey commented 7 years ago

im on chat btw

MrLoh commented 7 years ago

Still this is quite complex, only to keep the route in sync, which is just an overhead. I would be really nice if there would be some helpers to do something like switch to a tab without knowing which exact URL is active in that tab.

I think a URL architexture, like tab/stackComponent/:id where /tab/ is always the Home component of the stack is quite common. So a helper to switch to whatever is active in /tab/* would be really handy. Similarly it would be nice to easily start an app in the state where a tab router is in something like ['/tab1/', 'tab1/stackComponent/1']. Basically I can't think of a setup, where a Stack Router doesn't have a Home Componente that should always be the base of the stack. But it is not very easy at all to enforce this at the moment.

Just some thoughts on the challenges of issues that stacks nested in tabs come with.

MrLoh commented 7 years ago

Btw. I'd also really like a way to avoid reference to the react-navigation actions if possible at all. But I guess that would really need to factor out the StackNavigator from react-navigation.

faceyspacey commented 7 years ago

I like the idea of going to a tab, and keeping the current route as long as it matches a base path, but if it doesn't, pushing.

faceyspacey commented 7 years ago

i also hate those damn react-navigation actions.

..the truth is the StackRouter reducer I've mastered. The state shape is simple. Just arrays basically. We dont need their reducers or actions. All we need is the state driven Navigator which responds to the obvious state shape it already consumes.

So we can easily make changes here.

The hard part and stuff i'm really looking forward to is customizing animations and styles. And it's probably not hard, I just dont know it yet. But basically u should be able to trigger a custom animation per PUSH, not just one stack-wide animation.

As for styles, they chose an approach that doesn't jive with what "power users" would want. Basically they chose this approach where u can customize at a very high level, OR, they suggest you just rip it out and make your own (which you can do with the Top/BottomBar, or the whole Navigator, which they suggest to do as well--they seem to anticipate people ripping code from their codebase).

Really, we need an in-the-middle approach to customizing. Who wants crappy standard iOS/Android styles anyway? I really couldn't believe they worked so hard to support that stuff. Nobody that builds real apps wants that stuff. So they basically gave all their most serious users no choice but to just fork them.