Use absolute URLs to settings to avoid relative URL edge cases (#781)

* Create useAbsoluteFilePath hook

* Fix "report bug" link on Error page

* Replace relative URL to settings with absolute URL

* Replace other absolute file URLs to use common hook

* Use named const for default browser file name

* Fix UI tests that now rely on useRouteLoaderData()

Signed-off-by: Frank Noirot <frank@kittycad.io>

---------

Signed-off-by: Frank Noirot <frank@kittycad.io>

src/App.test.tsx

@@ -1,9 +1,15 @@
 import { render, screen } from '@testing-library/react'
 import { App } from './App'
 import { describe, test, vi } from 'vitest'
-import { BrowserRouter } from 'react-router-dom'
+import {
+  Route,
+  RouterProvider,
+  createMemoryRouter,
+  createRoutesFromElements,
+} from 'react-router-dom'
 import { GlobalStateProvider } from './components/GlobalStateProvider'
 import CommandBarProvider from 'components/CommandBar'
+import { BROWSER_FILE_NAME } from 'Router'
 
 let listener: ((rect: any) => void) | undefined = undefined
 ;(global as any).ResizeObserver = class ResizeObserver {
@@ -24,7 +30,7 @@ describe('App tests', () => {
       >
       return {
         ...actual,
-        useParams: () => ({ id: 'new' }),
+        useParams: () => ({ id: BROWSER_FILE_NAME }),
         useLoaderData: () => ({ code: null }),
       }
     })
@@ -41,12 +47,24 @@ describe('App tests', () => {
 })
 
 function TestWrap({ children }: { children: React.ReactNode }) {
-  // wrap in router and xState context
-  return (
-    <BrowserRouter>
+  // We have to use a memory router in the testing environment,
+  // and we have to use the createMemoryRouter function instead of <MemoryRouter /> as of react-router v6.4:
+  // https://reactrouter.com/en/6.16.0/routers/picking-a-router#using-v64-data-apis
+  const router = createMemoryRouter(
+    createRoutesFromElements(
+      <Route
+        path="/file/:id"
+        element={
           <CommandBarProvider>
             <GlobalStateProvider>{children}</GlobalStateProvider>
           </CommandBarProvider>
-    </BrowserRouter>
+        }
+      />
+    ),
+    {
+      initialEntries: ['/file/new'],
+      initialIndex: 0,
+    }
   )
+  return <RouterProvider router={router} />
 }

src/Router.tsx

@@ -94,6 +94,8 @@ export const paths = {
   ) as typeof onboardingPaths,
 }
 
+export const BROWSER_FILE_NAME = 'new'
+
 export type IndexLoaderData = {
   code: string | null
   project?: ProjectWithEntryPointMetadata
@@ -129,7 +131,9 @@ const router = createBrowserRouter(
     {
       path: paths.INDEX,
       loader: () =>
-        isTauri() ? redirect(paths.HOME) : redirect(paths.FILE + '/new'),
+        isTauri()
+          ? redirect(paths.HOME)
+          : redirect(paths.FILE + '/' + BROWSER_FILE_NAME),
       errorElement: <ErrorPage />,
     },
     {
@@ -167,7 +171,7 @@ const router = createBrowserRouter(
           )
         }
 
-        if (params.id && params.id !== 'new') {
+        if (params.id && params.id !== BROWSER_FILE_NAME) {
           // Note that PROJECT_ENTRYPOINT is hardcoded until we support multiple files
           const code = await readTextFile(params.id + '/' + PROJECT_ENTRYPOINT)
           const entrypoint_metadata = await metadata(
@@ -212,7 +216,7 @@ const router = createBrowserRouter(
       ),
       loader: async () => {
         if (!isTauri()) {
-          return redirect(paths.FILE + '/new')
+          return redirect(paths.FILE + '/' + BROWSER_FILE_NAME)
         }
         const fetchedStorage = localStorage?.getItem(SETTINGS_PERSIST_KEY)
         const persistedSettings = JSON.parse(fetchedStorage || '{}') as Partial<

src/components/ErrorPage.tsx

@@ -47,11 +47,9 @@ export const ErrorPage = () => {
             Clear storage
           </ActionButton>
           <ActionButton
-            Element="link"
+            Element="externalLink"
             icon={{ icon: faBug }}
-            target="_blank"
-            rel="noopener noreferrer"
-            to="https://discord.com/channels/915388055236509727/1138967922614743060"
+            to="https://github.com/KittyCAD/modeling-app/issues/new"
           >
             Report Bug
           </ActionButton>

src/components/UserSidebarMenu.test.tsx

@@ -1,6 +1,11 @@
 import { fireEvent, render, screen } from '@testing-library/react'
 import UserSidebarMenu from './UserSidebarMenu'
-import { BrowserRouter } from 'react-router-dom'
+import {
+  Route,
+  RouterProvider,
+  createMemoryRouter,
+  createRoutesFromElements,
+} from 'react-router-dom'
 import { Models } from '@kittycad/lib'
 import { GlobalStateProvider } from './GlobalStateProvider'
 import CommandBarProvider from './CommandBar'
@@ -93,11 +98,24 @@ describe('UserSidebarMenu tests', () => {
 
 function TestWrap({ children }: { children: React.ReactNode }) {
   // wrap in router and xState context
-  return (
-    <BrowserRouter>
+  // We have to use a memory router in the testing environment,
+  // and we have to use the createMemoryRouter function instead of <MemoryRouter /> as of react-router v6.4:
+  // https://reactrouter.com/en/6.16.0/routers/picking-a-router#using-v64-data-apis
+  const router = createMemoryRouter(
+    createRoutesFromElements(
+      <Route
+        path="/file/:id"
+        element={
           <CommandBarProvider>
             <GlobalStateProvider>{children}</GlobalStateProvider>
           </CommandBarProvider>
-    </BrowserRouter>
+        }
+      />
+    ),
+    {
+      initialEntries: ['/file/new'],
+      initialIndex: 0,
+    }
   )
+  return <RouterProvider router={router} />
 }

src/components/UserSidebarMenu.tsx

@@ -7,17 +7,17 @@ import {
   faSignOutAlt,
 } from '@fortawesome/free-solid-svg-icons'
 import { faGithub } from '@fortawesome/free-brands-svg-icons'
-import { useLocation, useNavigate } from 'react-router-dom'
+import { useNavigate } from 'react-router-dom'
 import { Fragment, useState } from 'react'
 import { paths } from '../Router'
-import makeUrlPathRelative from '../lib/makeUrlPathRelative'
 import { Models } from '@kittycad/lib'
 import { useGlobalStateContext } from 'hooks/useGlobalStateContext'
+import { useAbsoluteFilePath } from 'hooks/useAbsoluteFilePath'
 
 type User = Models['User_type']
 
 const UserSidebarMenu = ({ user }: { user?: User }) => {
-  const location = useLocation()
+  const filePath = useAbsoluteFilePath()
   const displayedName = getDisplayName(user)
   const [imageLoadFailed, setImageLoadFailed] = useState(false)
   const navigate = useNavigate()
@@ -132,11 +132,7 @@ const UserSidebarMenu = ({ user }: { user?: User }) => {
                     // since /settings is a nested route the sidebar doesn't close
                     // automatically when navigating to it
                     close()
-                    navigate(
-                      (location.pathname.endsWith('/')
-                        ? location.pathname.slice(0, -1)
-                        : location.pathname) + paths.SETTINGS
-                    )
+                    navigate(filePath + paths.SETTINGS)
                   }}
                 >
                   Settings

src/hooks/useAbsoluteFilePath.ts

@@ -0,0 +1,12 @@
+import { BROWSER_FILE_NAME, IndexLoaderData, paths } from 'Router'
+import { useRouteLoaderData } from 'react-router-dom'
+
+export function useAbsoluteFilePath() {
+  const routeData = useRouteLoaderData(paths.FILE) as IndexLoaderData
+
+  return (
+    paths.FILE +
+    '/' +
+    encodeURIComponent(routeData?.project?.path || BROWSER_FILE_NAME)
+  )
+}

src/routes/Onboarding/index.tsx

@@ -1,5 +1,5 @@
 import { useHotkeys } from 'react-hotkeys-hook'
-import { Outlet, useRouteLoaderData, useNavigate } from 'react-router-dom'
+import { Outlet, useNavigate } from 'react-router-dom'
 import Introduction from './Introduction'
 import Camera from './Camera'
 import Sketching from './Sketching'
@@ -15,7 +15,8 @@ import UserMenu from './UserMenu'
 import ProjectMenu from './ProjectMenu'
 import Export from './Export'
 import FutureWork from './FutureWork'
-import { IndexLoaderData, paths } from 'Router'
+import { paths } from 'Router'
+import { useAbsoluteFilePath } from 'hooks/useAbsoluteFilePath'
 
 export const onboardingPaths = {
   INDEX: '/',
@@ -86,29 +87,23 @@ export const onboardingRoutes = [
 ]
 
 export function useNextClick(newStatus: string) {
+  const filePath = useAbsoluteFilePath()
   const {
     settings: { send },
   } = useGlobalStateContext()
   const navigate = useNavigate()
-  const { project } = useRouteLoaderData(paths.FILE) as IndexLoaderData
 
   return useCallback(() => {
     send({
       type: 'Set Onboarding Status',
       data: { onboardingStatus: newStatus },
     })
-    navigate(
-      paths.FILE +
-        '/' +
-        encodeURIComponent(project?.path || 'new') +
-        paths.ONBOARDING.INDEX.slice(0, -1) +
-        newStatus
-    )
-  }, [project, newStatus, send, navigate])
+    navigate(filePath + paths.ONBOARDING.INDEX.slice(0, -1) + newStatus)
+  }, [filePath, newStatus, send, navigate])
 }
 
 export function useDismiss() {
-  const routeData = useRouteLoaderData(paths.FILE) as IndexLoaderData
+  const filePath = useAbsoluteFilePath()
   const {
     settings: { send },
   } = useGlobalStateContext()
@@ -119,10 +114,8 @@ export function useDismiss() {
       type: 'Set Onboarding Status',
       data: { onboardingStatus: 'dismissed' },
     })
-    navigate(
-      paths.FILE + '/' + encodeURIComponent(routeData?.project?.path || 'new')
-    )
-  }, [send, navigate, routeData])
+    navigate(filePath)
+  }, [send, navigate, filePath])
 }
 
 const Onboarding = () => {