Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
@@ -1,10 +1,10 @@
import { FC, useCallback, useMemo, useState } from 'react'
import { FC, useCallback, useContext, useMemo, useState } from 'react'

Choose a reason for hiding this comment

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

It seems like useContext has been added to the import statement, but there is no usage of useContext in the current code diff. Ensure that useContext is actually needed; otherwise, it should be removed to avoid unnecessary imports.


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,8 @@
const ViewScorecardPage = () => {
return (
<div>View scorecard</div>
)
};

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,9 +95,22 @@ export const reviewRoutes: ReadonlyArray<PlatformRoute> = [
route: activeReviewAssigmentsRouteId,
},
{
element: <ScorecardsListPage />,
element: <ScorecardsContainer />,

Choose a reason for hiding this comment

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

The ScorecardsContainer component is introduced here, but ensure that it properly handles the routing logic for its children components ScorecardsListPage and ViewScorecardPage. Verify that the container is necessary and that it doesn't introduce unnecessary complexity.

id: scorecardRouteId,
route: scorecardRouteId,
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.

},

],
},
],
domain: AppSubdomain.review,
Expand Down