Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/apps/review/src/config/routes.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ export const openOpportunitiesRouteId = 'open-opportunities'
export const pastReviewAssignmentsRouteId = 'past-review-assignments'
export const challengeDetailRouteId = ':challengeId'
export const scorecardRouteId = 'scorecard'
export const viewScorecardRouteId = ':scorecardId'
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export const TableScorecards: FC<Props> = (props: Props) => {
label: 'Scorecard',
propertyName: 'name',
renderer: (data: Scorecard) => (
<Link to={`${data.id}/details`}>
<Link to={`${data.id}`}>
{data.name}
</Link>
),
Expand Down
40 changes: 40 additions & 0 deletions src/apps/review/src/pages/scorecards/ScorecardsContainer.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/**
* The router outlet.
*/

import { FC, PropsWithChildren, useContext, useEffect, useMemo } from 'react'

Choose a reason for hiding this comment

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

Consider removing unused import PropsWithChildren since ScorecardsContainer does not utilize any children props.

import { Outlet, Routes, useLocation } from 'react-router-dom'

import { routerContext, RouterContextData } from '~/libs/core'

import { reviewRoutes } from '../../review-app.routes'
import { scorecardRouteId } from '../../config/routes.config'

export const ScorecardsContainer: FC<PropsWithChildren> = () => {
const location = useLocation()
const childRoutes = useChildRoutes()

useEffect(() => {
window.scrollTo(0, 0)
}, [location.pathname])

return (
<>
<Outlet />

Choose a reason for hiding this comment

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

The <Outlet /> component is typically used to render child routes. Ensure that the routing logic correctly supports this use case, as it might be redundant with the <Routes>{childRoutes}</Routes> rendering.

<Routes>{childRoutes}</Routes>
</>
)
}

function useChildRoutes(): Array<JSX.Element> | undefined {
const { getRouteElement }: RouterContextData = useContext(routerContext)
const childRoutes = useMemo(
() => reviewRoutes[0].children
?.find(r => r.id === scorecardRouteId)
?.children?.map(getRouteElement),
[getRouteElement],
)
return childRoutes
}

export default ScorecardsContainer
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { FC, useCallback, useMemo, useState } from 'react'
import { PageTitle } from '~/libs/ui'
import { TableLoading } from '~/apps/admin/src/lib'

import { PageWrapper, ScorecardsFilter, TableNoRecord, TableScorecards } from '../../lib'
import { ScorecardsResponse, useFetchScorecards } from '../../lib/hooks'
import { PageWrapper, ScorecardsFilter, TableNoRecord, TableScorecards } from '../../../lib'

Choose a reason for hiding this comment

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

The import path for PageWrapper, ScorecardsFilter, TableNoRecord, and TableScorecards has been changed from ../../lib to ../../../lib. Verify that this change is correct and that the new path accurately reflects the location of these modules.

import { ScorecardsResponse, useFetchScorecards } from '../../../lib/hooks'

Choose a reason for hiding this comment

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

The import path for ScorecardsResponse and useFetchScorecards has been changed from ../../lib/hooks to ../../../lib/hooks. Ensure that this change is correct and that the new path accurately reflects the location of these hooks.


import styles from './ScorecardsListPage.module.scss'

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { FC } from 'react'

const ViewScorecardPage: FC = () => (
<div>View scorecard</div>
)

Choose a reason for hiding this comment

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

Consider adding a semicolon at the end of the line for consistency with the rest of the codebase.


export default ViewScorecardPage
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default as ViewScorecardPage } from './ViewScorecardPage'
25 changes: 24 additions & 1 deletion src/apps/review/src/review-app.routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,21 @@ const ScorecardDetailsPage: LazyLoadedComponent = lazyLoad(
'ScorecardDetailsPage',
)

const ScorecardsContainer: LazyLoadedComponent = lazyLoad(
() => import('./pages/scorecards/ScorecardsContainer'),
'ScorecardsContainer',
)

const ScorecardsListPage: LazyLoadedComponent = lazyLoad(
() => import('./pages/scorecards/ScorecardsListPage'),
'ScorecardsListPage',
)

const ViewScorecardPage: LazyLoadedComponent = lazyLoad(
() => import('./pages/scorecards/ViewScorecardPage'),
'ViewScorecardPage',
)

export const toolTitle: string = ToolTitle.review

export const reviewRoutes: ReadonlyArray<PlatformRoute> = [
Expand Down Expand Up @@ -85,7 +95,20 @@ export const reviewRoutes: ReadonlyArray<PlatformRoute> = [
route: activeReviewAssigmentsRouteId,
},
{
element: <ScorecardsListPage />,
children: [
{
element: <ScorecardsListPage />,
id: 'list-scorecards-page',
route: '',
},
{
element: <ViewScorecardPage />,
id: 'view-scorecard-page',
route: ':scorecardId',

Choose a reason for hiding this comment

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

The route :scorecardId suggests dynamic routing based on scorecard IDs. Ensure that the ViewScorecardPage component is equipped to handle cases where the scorecardId might be invalid or missing, and consider implementing error handling or redirection logic.

},

],
element: <ScorecardsContainer />,

Choose a reason for hiding this comment

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

The element property is added here, but ensure that <ScorecardsContainer /> is correctly imported and defined in the file. If it is not imported, the application will fail to render this component.

id: scorecardRouteId,

Choose a reason for hiding this comment

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

The id and route properties are set to scorecardRouteId. Verify that scorecardRouteId is defined and correctly imported in this file. If it is not defined, it will cause errors in routing.

route: scorecardRouteId,
},
Expand Down