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

[Tabs] How to sync with react-router #18811

Closed
aress31 opened this issue Dec 12, 2019 · 15 comments · Fixed by #25827
Closed

[Tabs] How to sync with react-router #18811

aress31 opened this issue Dec 12, 2019 · 15 comments · Fixed by #25827
Labels
component: tabs This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@aress31
Copy link

aress31 commented Dec 12, 2019

My title is quite self-explanatory, I implemented a navigation bar using the <Tabs> which allows me to navigate through my website as one would expect. However, when/if I try to directly access an endpoint (e.g. /services or about), the <Tabs> are obviously out of sync and the <Tab> that should be set as active is not.

I am sure that this is a well-known issue with a "hacky" fix that should be included in the official doc, to avoid rookies like me posting this type of ticket. 😁

@oliviertassinari
Copy link
Member

The integration should be already detailed in https://material-ui.com/guides/composition/#routing-libraries.

@oliviertassinari
Copy link
Member

However, we might need to go into more details for the tabs.

@oliviertassinari oliviertassinari added component: tabs This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation labels Dec 12, 2019
@oliviertassinari
Copy link
Member

You can find prior coverage in the history of the project:

I would propose the following diff:

diff --git a/docs/src/pages/guides/composition/composition.md b/docs/src/pages/guides/composition/composition.md
index 05d30b193..87c0a721b 100644
--- a/docs/src/pages/guides/composition/composition.md
+++ b/docs/src/pages/guides/composition/composition.md
@@ -149,6 +149,10 @@ It covers the Button, Link, and List components, you should be able to apply the

 {{"demo": "pages/guides/composition/ListRouter.js"}}

+### Tabs
+
+{{"demo": "pages/guides/composition/TabsRouter.js", "defaultCodeOpen": false, "bg": true}}
+
 ## Caveat with refs

 This section covers caveats when using a custom component as `children` or for the

diff --git a/docs/src/pages/guides/composition/TabsRouter.tsxb/docs/src/pages/guides/composition/TabsRouter.tsx
index 05d30b193..87c0a721b 100644
+import React from 'react';
+import Tabs from '@material-ui/core/Tabs';
+import Tab, { TabProps } from '@material-ui/core/Tab';
+import Paper from '@material-ui/core/Paper';
+import Typography from '@material-ui/core/Typography';
+import { Route, MemoryRouter } from 'react-router';
+import { Link as RouterLink, LinkProps as RouterLinkProps, useLocation } from 'react-router-dom';
+import { Omit } from '@material-ui/types';
+
+function TabLink(props: TabProps) {
+  const { value } = props;

+  const renderLink = React.useMemo(
+    () =>
+      React.forwardRef<HTMLAnchorElement, Omit<RouterLinkProps, 'innerRef' | 'to'>>(
+        (itemProps, ref) => (
+          // With react-router-dom@^6.0.0 use `ref` instead of `innerRef`
+          // See https://github.com/ReactTraining/react-router/issues/6056
+          <RouterLink to={value} {...itemProps} innerRef={ref} />
+        ),
+      ),
+    [value],
+  );

+  return <Tab component={renderLink} {...props} />;
+}
+
+function MyTabs() {
+  const location = useLocation();

+  return (
+    <Paper square>
+      <Tabs aria-label="tabs router example" value={location.pathname}>
+        <TabLink label="Inbox" value="/inbox" />
+        <TabLink label="Drafts" value="/drafts" />
+        <TabLink label="Trash" value="/trash" />
+      </Tabs>
+    </Paper>
+  );
+}
+
+export default function TabsRouter() {
+  return (
+    <div>
+      <MemoryRouter initialEntries={['/drafts']} initialIndex={0}>
+        <Route>
+          {({ location }) => (
+            <Typography gutterBottom>Current route: {location.pathname}</Typography>
+          )}
+        </Route>
+        <MyTabs />
+      </MemoryRouter>
+    </div>
+  );
+}

However, I couldn't make the TypeScript definition run without error. @eps1lon I suspect something is wrong between the expectation of a div and an element.

@oliviertassinari oliviertassinari added the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label Dec 13, 2019
@RodyGL
Copy link

RodyGL commented Dec 26, 2019

I created a react custom hook to handle this problem and keep the TabLink and url path in sync. Notice that the "to" prop of react-router Link is inherited from "value" prop of MUI Tab component if it's not provided.

TabLink.tsx

import React from 'react';
import { LinkProps } from 'react-router-dom';
import { Tab } from '@material-ui/core';
import { TabProps } from '@material-ui/core/Tab';
import RouterLink from './RouterLink';

const TabLink: React.FC<Omit<LinkProps, 'to'> & TabProps & { to?: LinkProps['to'] }> = ({
  to,
  value,
  ...props
}) => {
  return <Tab component={RouterLink} to={to ?? value} value={value} {...props} />;
};

export default TabLink;

RouterLink.tsx

import React from 'react';
import { Link, LinkProps } from 'react-router-dom';

const RouterLink: React.FC<Omit<LinkProps, 'innerRef' | 'ref'>> = (props, ref) => (
  <Link innerRef={ref} {...props} />
);

export default React.forwardRef(RouterLink);

useTabsWithRouter.ts

import { useRouteMatch } from 'react-router-dom';

export const useTabsWithRouter = (routes: string | string[], defaultRoute: string): string => {
  const match = useRouteMatch(routes);

  return match?.path ?? defaultRoute;
};

Example.tsx

const Example: React.FC = () => {
  const tabValue = useTabsWithRouter(['users/add', 'users/edit', 'users'], 'users');

  return (
    <div>
      <Tabs value={tabValue}>
        <TabLink value="users" label="Users" />
        <TabLink value="users/add" label="Users New" />
        <TabLink value="users/edit" label="Users Edit" />
      </Tabs>
    </div>
  );
};

export default Example;

IMPORTANT If you provide an array of routes (which is the normal use case) then you need to provide them in descendant order. This means that if you have nested routes like users, users/new, users/edit, etc. Then the order would be ['users/add', 'users/edit', 'users'].

@oliviertassinari
Copy link
Member

What do you guys think about this version? In collaboration with @mbrevda: https://codesandbox.io/s/material-demo-ymnv8?file=/demo.js.

@mbrevda
Copy link

mbrevda commented May 3, 2020

FWIW, I don't love the solution myself. Specifically, I'd prefer the declarative nature of using the <Tab> component and would rather avoid a (separate) array of paths in order to tell <Tabs/> which one is the active tab.

Perhaps if <Tabs value/> took a predicate passing each tab's props and expecting true or false we could cut down on some duplication/imperative code?

@oliviertassinari
Copy link
Member

@mbrevda How would it look like?

@mbrevda
Copy link

mbrevda commented May 3, 2020

psudocode:

const value = ({to}: Tab) => useRouteMatch(to))

Basically each tab gets passed to the function until it returns two. A la Array.find

@sdg002
Copy link

sdg002 commented Dec 5, 2020

Sorry, for the late response. I had done a Proof of Concept solution using Bootstrap navbars. I had used the NavLink and the activeClassName to declaratively style the active link. This was dead simple.

https://reactrouter.com/web/api/NavLink

I was hoping Material Tabs would have been simple. But, I am lost.

@sdg002
Copy link

sdg002 commented Dec 5, 2020

@mbrevda and @oliviertassinari
Inspired by your code, I got this to work.

  • I am setting the value prop of the Tab object to the to property.
  • Using the path property of useLocation to set the value of the parent Tabs
const MaterialTabbedNavbar = () => {
    const  location = useLocation();

    return (
        <AppBar position="static" color="default">
            <Tabs
                variant="fullWidth"
                aria-label="scrollable auto tabs example"
                indicatorColor="secondary"
                textColor="primary"
                value={location.pathname}
            >
                <Tab to={'/'} component={Link} label={'Home'} value={'/'}></Tab>
                <Tab
                    to={'/create-customers'}
                    component={Link}
                    label={'Create customers'}
                    value={'/create-customers'}
                ></Tab>
                <Tab
                    to={'/customers-download'}
                    component={Link}
                    label={'Download Customers Info'}
                    value={'/customers-download'}
                ></Tab>
            </Tabs>
        </AppBar>
    );

The above works for now. But, I am quite shaken by the realization that there is significant coding involved when using Material UI. Simple chores end up being long hours of discovery on SFO/Git.

@oliviertassinari oliviertassinari added good first issue Great for first contributions. Enable to learn the contribution process. and removed ready to take Help wanted. Guidance available. There is a high chance the change will be accepted labels Dec 5, 2020
@oliviertassinari
Copy link
Member

@sdg002 Do you wish to add a live demo to the documentation? It seems to be a frequency problem we need to better cover :).

@mbrevda
Copy link

mbrevda commented Dec 6, 2020

It would indeed seem that this use case (navigation tabs) is popular enough to warrant first class support

@sdg002
Copy link

sdg002 commented Dec 6, 2020

@sdg002 Do you wish to add a live demo to the documentation? It seems to be a frequency problem we need to better cover :).

I have taken the liberty to fork the sanbox and add my changes.
https://codesandbox.io/s/material-demo-forked-il7p7?file=/demo.js

CC @mbrevda @oliviertassinari

@kodeschooler
Copy link

@sdg002 in your example, what if you had a tab that you didn't want to render based on a certain condition, like isUserLoggedIn? I've tried conditionally rendering the route and tab, but you'll still get an error when they enter the pathname in the address bar because the values need to match. I simply want to hide the tab, but we need our tabs to have urls so they can be navigated to. For now I just applied class to display: none - but that generates an error in the console from mui. Why is there no hidden prop for the tab child. just hides the content but not the tab

@sdg002
Copy link

sdg002 commented May 7, 2022

@kodeschooler Never tried that so I cannot answer.. I have bid adieu to React and moved to WebAssembly (C#).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: tabs This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants