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

feat(components, app, labware-library): migrate to react-router-dom v6.24.1 #15699

Merged
merged 12 commits into from
Jul 22, 2024
2 changes: 1 addition & 1 deletion app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
"react-intersection-observer": "^8.33.1",
"react-markdown": "9.0.1",
"react-redux": "8.1.2",
"react-router-dom": "5.3.4",
"react-router-dom": "6.24.1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not introducing v6.25.0/v6.25.1 because too new?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh wait i think v6.25.0 came out just yesterday, that is why i did not use it lol. Shouldn't we stick with 6.24.1 to match PD though? hmm. @shlokamin thoughts?

Copy link
Contributor

@koji koji Jul 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6.25.0 2days ago and 6.25.1 yesterday.
https://github.com/remix-run/react-router/releases

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't matter to me as long as all of our frontend projects share the same version, so we can either update PD or keep these at 6.24.1

"react-select": "5.4.0",
"react-simple-keyboard": "^3.7.0",
"react-viewport-list": "6.3.0",
Expand Down
60 changes: 28 additions & 32 deletions app/src/App/DesktopApp.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react'
import { Redirect, Route, Switch, useRouteMatch } from 'react-router-dom'
import { Navigate, Route, Routes, useMatch } from 'react-router-dom'
import { ErrorBoundary } from 'react-error-boundary'
import { I18nextProvider } from 'react-i18next'

Expand Down Expand Up @@ -48,20 +48,17 @@ export const DesktopApp = (): JSX.Element => {
const desktopRoutes: RouteProps[] = [
{
Component: ProtocolsLanding,
exact: true,
name: 'Protocols',
navLinkTo: '/protocols',
path: '/protocols',
},
{
Component: ProtocolDetails,
exact: true,
name: 'Protocol Details',
path: '/protocols/:protocolKey',
},
{
Component: ProtocolTimeline,
exact: true,
name: 'Protocol Timeline',
path: '/protocols/:protocolKey/timeline',
},
Expand All @@ -73,26 +70,22 @@ export const DesktopApp = (): JSX.Element => {
},
{
Component: DevicesLanding,
exact: true,
name: 'Devices',
navLinkTo: '/devices',
path: '/devices',
},
{
Component: DeviceDetails,
exact: true,
name: 'Device',
path: '/devices/:robotName',
},
{
Component: RobotSettings,
exact: true,
name: 'Robot Settings',
path: '/devices/:robotName/robot-settings/:robotSettingsTab?',
},
{
Component: CalibrationDashboard,
exact: true,
name: 'Calibration Dashboard',
path: '/devices/:robotName/robot-settings/calibration/dashboard',
},
Expand All @@ -103,7 +96,6 @@ export const DesktopApp = (): JSX.Element => {
},
{
Component: AppSettings,
exact: true,
name: 'App Settings',
path: '/app-settings/:appSettingsTab?',
},
Expand All @@ -123,33 +115,37 @@ export const DesktopApp = (): JSX.Element => {
>
<Box width="100%">
<Alerts>
<Switch>
{desktopRoutes.map(
({ Component, exact, path }: RouteProps) => {
return (
<Route key={path} exact={exact} path={path}>
<Breadcrumbs />
<Box
width="100%"
height="100%"
position={POSITION_RELATIVE}
>
<Routes>
{desktopRoutes.map(({ Component, path }: RouteProps) => {
return (
<Route
key={path}
element={
<>
<Breadcrumbs />
<Box
position={POSITION_RELATIVE}
width="100%"
height="100%"
backgroundColor={COLORS.grey10}
overflow={OVERFLOW_AUTO}
>
<ModalPortalRoot />
<Component />
<Box
width="100%"
height="100%"
backgroundColor={COLORS.grey10}
overflow={OVERFLOW_AUTO}
>
<ModalPortalRoot />
<Component />
</Box>
</Box>
</Box>
</Route>
)
}
)}
<Redirect exact from="/" to="/protocols" />
</Switch>
</>
}
path={path}
/>
)
})}
<Route path="/" element={<Navigate to="/protocols" />} />
</Routes>
<RobotControlTakeover />
</Alerts>
</Box>
Expand All @@ -162,7 +158,7 @@ export const DesktopApp = (): JSX.Element => {
}

function RobotControlTakeover(): JSX.Element | null {
const deviceRouteMatch = useRouteMatch({ path: '/devices/:robotName' })
const deviceRouteMatch = useMatch('/devices/:robotName')
const params = deviceRouteMatch?.params as DesktopRouteParams
const robotName = params?.robotName
const robot = useRobot(robotName)
Expand Down
6 changes: 3 additions & 3 deletions app/src/App/DesktopAppFallback.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react'
import { useDispatch } from 'react-redux'
import { useHistory } from 'react-router-dom'
import { useNavigate } from 'react-router-dom'
import { useTranslation } from 'react-i18next'

import { useTrackEvent, ANALYTICS_DESKTOP_APP_ERROR } from '../redux/analytics'
Expand All @@ -26,14 +26,14 @@ export function DesktopAppFallback({ error }: FallbackProps): JSX.Element {
const { t } = useTranslation('app_settings')
const trackEvent = useTrackEvent()
const dispatch = useDispatch<Dispatch>()
const history = useHistory()
const navigate = useNavigate()
const handleReloadClick = (): void => {
trackEvent({
name: ANALYTICS_DESKTOP_APP_ERROR,
properties: { errorMessage: error.message },
})
// route to the root page and initiate an electron browser window reload via app-shell
history.push('/')
navigate('/', { replace: true })
dispatch(reloadUi(error.message as string))
}

Expand Down
30 changes: 19 additions & 11 deletions app/src/App/OnDeviceDisplayApp.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react'
import { useDispatch, useSelector } from 'react-redux'
import { Switch, Route, Redirect } from 'react-router-dom'
import { Routes, Route, Navigate } from 'react-router-dom'
import { css } from 'styled-components'
import { ErrorBoundary } from 'react-error-boundary'

Expand Down Expand Up @@ -252,23 +252,31 @@ export function OnDeviceDisplayAppRoutes(): JSX.Element {
`

return (
<Switch>
<Routes>
{ON_DEVICE_DISPLAY_PATHS.map(path => (
<Route key={path} exact path={path}>
<Box css={TOUCH_SCREEN_STYLE} ref={scrollRef}>
<ModalPortalRoot />
{getPathComponent(path)}
</Box>
</Route>
<Route
key={path}
path={path}
element={
<Box css={TOUCH_SCREEN_STYLE} ref={scrollRef}>
<ModalPortalRoot />
{getPathComponent(path)}
</Box>
}
/>
))}
{targetPath != null && <Redirect exact from="/" to={targetPath} />}
</Switch>
{targetPath != null && (
<Route path="/" element={<Navigate to={targetPath} />} />
)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://stackoverflow.com/questions/69868956/how-can-i-redirect-in-react-router-v6

Suggested change
{targetPath != null && (
<Route path="/" element={<Navigate to={targetPath} />} />
)}
<Route path="*" element={<Navigate to={targetPath} replace />} />

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you so much koji!

</Routes>
)
}

function TopLevelRedirects(): JSX.Element | null {
const currentRunRoute = useCurrentRunRoute()
return currentRunRoute != null ? <Redirect to={currentRunRoute} /> : null
return currentRunRoute != null ? (
<Route element={<Navigate to={currentRunRoute} />} />
) : null
}

function ProtocolReceiptToasts(): null {
Expand Down
10 changes: 5 additions & 5 deletions app/src/App/__tests__/Navbar.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react'
import { describe, it } from 'vitest'
import { screen, render } from '@testing-library/react'
import { StaticRouter } from 'react-router-dom'
import { MemoryRouter } from 'react-router-dom'

import { Navbar } from '../Navbar'

Expand All @@ -16,19 +16,19 @@ const ROUTE_PROPS: RouteProps[] = [
describe('Navbar', () => {
it('should render a NavbarLink for every nav location', () => {
render(
<StaticRouter>
<MemoryRouter>
<Navbar routes={ROUTE_PROPS} />
</StaticRouter>
</MemoryRouter>
)
screen.getByRole('link', { name: 'foo' })
screen.getByRole('link', { name: 'bar' })
screen.getByRole('link', { name: 'baz' })
})
it('should render logo, settings, and help', () => {
render(
<StaticRouter>
<MemoryRouter>
<Navbar routes={ROUTE_PROPS} />
</StaticRouter>
</MemoryRouter>
)
screen.getByRole('img', { name: 'opentrons logo' })
screen.getByTestId('Navbar_settingsLink')
Expand Down
1 change: 0 additions & 1 deletion app/src/App/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ export interface RouteProps {
* drop developed components into slots held by placeholder div components
*/
Component: React.FC
exact?: boolean
/**
* a route/page name to render in the nav bar
*/
Expand Down
6 changes: 3 additions & 3 deletions app/src/atoms/buttons/BackButton.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react'
import { useTranslation } from 'react-i18next'
import { useHistory } from 'react-router-dom'
import { useNavigate } from 'react-router-dom'

import {
ALIGN_CENTER,
Expand All @@ -16,7 +16,7 @@ export function BackButton({
onClick,
children,
}: React.HTMLProps<HTMLButtonElement>): JSX.Element {
const history = useHistory()
const navigate = useNavigate()
const { t } = useTranslation('shared')

return (
Expand All @@ -28,7 +28,7 @@ export function BackButton({
onClick != null
? onClick
: () => {
history.goBack()
navigate(-1)
}
}
>
Expand Down
16 changes: 6 additions & 10 deletions app/src/atoms/buttons/__tests__/BackButton.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as React from 'react'
import { fireEvent, screen } from '@testing-library/react'
import { describe, it, expect, vi } from 'vitest'
import '@testing-library/jest-dom/vitest'
import { MemoryRouter, Route, Switch } from 'react-router-dom'
import { MemoryRouter, Route, Routes } from 'react-router-dom'

import { renderWithProviders } from '../../../__testing-utils__'

Expand All @@ -16,14 +16,10 @@ const render = (props?: React.HTMLProps<HTMLButtonElement>) => {
initialIndex={1}
>
<BackButton {...props} />
<Switch>
<Route exact path="/current-page">
this is the current page
</Route>
<Route exact path="/previous-page">
this is the previous page
</Route>
</Switch>
<Routes>
<Route path="/current-page" element={<>this is the current page</>} />
<Route path="/previous-page" element={<>this is the previous page</>} />
</Routes>
</MemoryRouter>,
{ i18nInstance: i18n }
)[0]
Expand All @@ -49,7 +45,7 @@ describe('BackButton', () => {
expect(screen.queryByText('this is the previous page')).toBeNull()
})

it('goes back one page in history on click if no on click handler provided', () => {
it('goes back one page in navigate on click if no on click handler provided', () => {
render()

screen.getByText('this is the current page')
Expand Down
8 changes: 3 additions & 5 deletions app/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,13 @@
import React from 'react'
import ReactDom from 'react-dom/client'
import { Provider } from 'react-redux'

import { ConnectedRouter } from 'connected-react-router'
import { BrowserRouter } from 'react-router-dom'

import { ApiClientProvider } from '@opentrons/react-api-client'

import { createLogger } from './logger'

import { uiInitialized } from './redux/shell'
import { history } from './redux/reducer'
import { store } from './redux/store'

import '../src/atoms/SoftwareKeyboard/AlphanumericKeyboard'
Expand All @@ -34,10 +32,10 @@ if (container == null) throw new Error('Failed to find the root element')
const root = ReactDom.createRoot(container)
root.render(
<Provider store={store}>
<ConnectedRouter history={history}>
<BrowserRouter>
<ApiClientProvider>
<App />
</ApiClientProvider>
</ConnectedRouter>
</BrowserRouter>
</Provider>
)
10 changes: 5 additions & 5 deletions app/src/molecules/CardButton/__tests__/CardButton.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ import { COLORS } from '@opentrons/components'
import { renderWithProviders } from '../../../__testing-utils__'
import { i18n } from '../../../i18n'
import { CardButton } from '..'
import type * as ReactRouterDom from 'react-router-dom'
import type { NavigateFunction } from 'react-router-dom'

const mockPush = vi.fn()
const mockNavigate = vi.fn()

vi.mock('react-router-dom', async importOriginal => {
const reactRouterDom = await importOriginal<typeof ReactRouterDom>()
const reactRouterDom = await importOriginal<NavigateFunction>()
return {
...reactRouterDom,
useHistory: () => ({ push: mockPush } as any),
useNavigate: () => mockNavigate,
}
})

Expand Down Expand Up @@ -65,6 +65,6 @@ describe('CardButton', () => {
render(props)
const button = screen.getByRole('button')
fireEvent.click(button)
expect(mockPush).toHaveBeenCalledWith('/mockPath')
expect(mockNavigate).toHaveBeenCalledWith('/mockPath')
})
})
6 changes: 3 additions & 3 deletions app/src/molecules/CardButton/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react'
import { useHistory } from 'react-router-dom'
import { useNavigate } from 'react-router-dom'
import { css } from 'styled-components'
import {
ALIGN_CENTER,
Expand Down Expand Up @@ -76,12 +76,12 @@ interface CardButtonProps {

export function CardButton(props: CardButtonProps): JSX.Element {
const { title, iconName, description, destinationPath, disabled } = props
const history = useHistory()
const navigate = useNavigate()

return (
<Btn
onClick={() => {
history.push(destinationPath)
navigate(destinationPath)
}}
width="100%"
css={CARD_BUTTON_STYLE}
Expand Down
Loading
Loading