Routes and PageTabs improvements (#368)

* Rename file to match component name * De-duplicate paths in routes * Add catch-all for invalid competition tabs redirects to competition general info page * Make the url the source of truth for current tab * Remove unnecessary no-op render functions * fix eslint --------- Co-authored-by: FinnIckler <[email protected]>
thewca · Dec 4, 2023 · ed960ec · ed960ec
1 parent fe67980
commit ed960ec
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 32 deletions.
diff --git a/Frontend/src/index.dev.jsx b/Frontend/src/index.dev.jsx
@@ -4,7 +4,12 @@ import './global.scss'
 import '@thewca/wca-components/dist/index.esm.css'
 import React from 'react'
 import { createRoot } from 'react-dom/client'
-import { createBrowserRouter, Outlet, RouterProvider } from 'react-router-dom'
+import {
+  createBrowserRouter,
+  Navigate,
+  Outlet,
+  RouterProvider,
+} from 'react-router-dom'
 import { Container } from 'semantic-ui-react'
 import Events from './pages/events'
 import HomePage from './pages/home'
@@ -23,9 +28,9 @@ import CustomTab from './ui/CustomTab'
 import PageFooter from './ui/Footer'
 import PageHeader from './ui/Header'
 import FlashMessage from './ui/messages/flashMessage'
+import PageTabs from './ui/PageTabs'
 import PermissionsProvider from './ui/providers/PermissionsProvider'
 import UserProvider from './ui/providers/UserProvider'
-import PageTabs from './ui/Tabs'
 
 const router = createBrowserRouter([
   {
@@ -75,41 +80,45 @@ const router = createBrowserRouter([
         ),
         children: [
           {
-            path: `${BASE_ROUTE}/:competition_id`,
+            path: '',
             element: <HomePage />,
           },
           {
-            path: `${BASE_ROUTE}/:competition_id/events`,
+            path: 'events',
             element: <Events />,
           },
           {
-            path: `${BASE_ROUTE}/:competition_id/import`,
+            path: 'import',
             element: <Import />,
           },
           {
-            path: `${BASE_ROUTE}/:competition_id/schedule`,
+            path: 'schedule',
             element: <Schedule />,
           },
           {
-            path: `${BASE_ROUTE}/:competition_id/register`,
+            path: 'register',
             element: <Register />,
           },
           {
-            path: `${BASE_ROUTE}/:competition_id/tabs/:tab_id`,
+            path: 'tabs/:tab_id',
             element: <CustomTab />,
           },
           {
-            path: `${BASE_ROUTE}/:competition_id/registrations`,
+            path: 'registrations',
             element: <Registrations />,
           },
           {
-            path: `${BASE_ROUTE}/:competition_id/:user_id/edit`,
+            path: ':user_id/edit',
             element: <RegistrationEdit />,
           },
           {
-            path: `${BASE_ROUTE}/:competition_id/registrations/edit`,
+            path: 'registrations/edit',
             element: <RegistrationAdministration />,
           },
+          {
+            path: '*',
+            element: <Navigate to="" />,
+          },
         ],
       },
     ],

diff --git a/Frontend/src/routes.jsx b/Frontend/src/routes.jsx
@@ -1,5 +1,5 @@
 import React from 'react'
-import { Outlet } from 'react-router-dom'
+import { Navigate, Outlet } from 'react-router-dom'
 import { Container } from 'semantic-ui-react'
 import Events from './pages/events'
 import HomePage from './pages/home'
@@ -13,9 +13,9 @@ import App from './ui/App'
 import Competition from './ui/Competition'
 import CustomTab from './ui/CustomTab'
 import FlashMessage from './ui/messages/flashMessage'
+import PageTabs from './ui/PageTabs'
 import PermissionsProvider from './ui/providers/PermissionsProvider'
 import UserProvider from './ui/providers/UserProvider'
-import PageTabs from './ui/Tabs'
 
 export const BASE_ROUTE = '/competitions/v2'
 
@@ -32,7 +32,7 @@ const routes = [
     ),
     children: [
       {
-        path: `${BASE_ROUTE}/:competition_id`,
+        path: ':competition_id',
         element: (
           <Container>
             <Competition>
@@ -45,41 +45,45 @@ const routes = [
         ),
         children: [
           {
-            path: `${BASE_ROUTE}/:competition_id`,
+            path: '',
             element: <HomePage />,
           },
           {
-            path: `${BASE_ROUTE}/:competition_id/events`,
+            path: 'events',
             element: <Events />,
           },
           {
-            path: `${BASE_ROUTE}/:competition_id/schedule`,
+            path: 'schedule',
             element: <Schedule />,
           },
           {
-            path: `${BASE_ROUTE}/:competition_id/import`,
+            path: 'import',
             element: <Import />,
           },
           {
-            path: `${BASE_ROUTE}/:competition_id/register`,
+            path: 'register',
             element: <Register />,
           },
           {
-            path: `${BASE_ROUTE}/:competition_id/tabs/:tab_id`,
+            path: 'tabs/:tab_id',
             element: <CustomTab />,
           },
           {
-            path: `${BASE_ROUTE}/:competition_id/registrations`,
+            path: 'registrations',
             element: <Registrations />,
           },
           {
-            path: `${BASE_ROUTE}/:competition_id/:user_id/edit`,
+            path: ':user_id/edit',
             element: <RegistrationEdit />,
           },
           {
-            path: `${BASE_ROUTE}/:competition_id/registrations/edit`,
+            path: 'registrations/edit',
             element: <RegistrationAdministration />,
           },
+          {
+            path: '*',
+            element: <Navigate to="" />,
+          },
         ],
       },
     ],

diff --git a/Frontend/src/ui/Tabs.jsx → Frontend/src/ui/PageTabs.jsx b/Frontend/src/ui/Tabs.jsx → Frontend/src/ui/PageTabs.jsx
@@ -16,6 +16,7 @@ function pathMatch(name, pathname) {
   const eventsExpressions = /\/competitions\/v2\/[a-zA-Z0-9]+\/events/
   const scheduleExpressions = /\/competitions\/v2\/[a-zA-Z0-9]+\/schedule/
   const infoExpression = /\/competitions\/v2\/[a-zA-Z0-9]+$/
+
   switch (name) {
     case 'register':
       return registerExpression.test(pathname)
@@ -42,6 +43,7 @@ export default function PageTabs() {
   const { canAdminCompetition } = useContext(PermissionsContext)
   const navigate = useNavigate()
   const location = useLocation()
+
   const panes = useMemo(() => {
     const optionalTabs = []
     if (competitionInfo.use_wca_registration) {
@@ -59,7 +61,6 @@ export default function PageTabs() {
             Register
           </Menu.Item>
         ),
-        render: () => {},
       })
     }
     if (canAdminCompetition) {
@@ -77,7 +78,6 @@ export default function PageTabs() {
             Registrations
           </Menu.Item>
         ),
-        render: () => {},
       })
     }
     if (new Date(competitionInfo.registration_open) < Date.now()) {
@@ -95,7 +95,6 @@ export default function PageTabs() {
             Competitors
           </Menu.Item>
         ),
-        render: () => {},
       })
     }
     return [
@@ -111,7 +110,6 @@ export default function PageTabs() {
             General Info
           </Menu.Item>
         ),
-        render: () => {},
       },
       ...optionalTabs,
       {
@@ -133,7 +131,6 @@ export default function PageTabs() {
             Events
           </Menu.Item>
         ),
-        render: () => {},
       },
       {
         menuItem: (
@@ -149,7 +146,6 @@ export default function PageTabs() {
             Schedule
           </Menu.Item>
         ),
-        render: () => {},
       },
       ...competitionInfo.tabs.map((tab) => {
         return {
@@ -165,7 +161,6 @@ export default function PageTabs() {
               {tab.name}
             </Menu.Item>
           ),
-          render: () => {},
         }
       }),
     ]
@@ -186,8 +181,10 @@ export default function PageTabs() {
       panes={panes}
       renderActiveOnly={true}
       menu={{ secondary: true, pointing: true }}
-      // This is only relevant on refresh, why we don't need to use useEffect
-      defaultActiveIndex={
+      // clicking a tab will navigate to the new url,
+      // then location.pathname updates and this active index changes
+      // ie the pathname (url) is the single source of truth
+      activeIndex={
         panes.findIndex((pane) => {
           return pathMatch(pane.menuItem.props.name, location.pathname)
         }) ?? 0