Skip to content

PM- 1502 Scorecards filtering and listing UI #1184

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.

Sign up for GitHub

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

Merged
merged 15 commits into from
Aug 14, 2025
Merged

PM- 1502 Scorecards filtering and listing UI #1184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
a826cad
Add Scorecards list and filter UI with mock data
himaniraghav3 Aug 8, 2025
74595fc
PM-1502 Integrate API,styling, code cleanup
himaniraghav3 Aug 12, 2025
c6d6c32
PM-1505 - clone scorecard
vas3a Aug 12, 2025
6138ea4
scorecard clone - error handling
vas3a Aug 12, 2025
63af3a0
Change pagination
himaniraghav3 Aug 12, 2025
d978668
Fix type filter
himaniraghav3 Aug 12, 2025
cf00e0c
feat: route modifications for scorecard
hentrymartin Aug 12, 2025
f473d90
fix: lint
hentrymartin Aug 12, 2025
6480aba
fix: lint
hentrymartin Aug 12, 2025
12ea744
Merge pull request #1183 from topcoder-platform/pm-1503
himaniraghav3 Aug 13, 2025
96f77b1
Fix active tab
himaniraghav3 Aug 13, 2025
05a2c77
Merge branch 'PM-1502-scorecards' of github.com:topcoder-platform/pla…
vas3a Aug 13, 2025
6462088
Merge pull request #1182 from topcoder-platform/PM-1505_clone-scorecard
vas3a Aug 13, 2025
578cb10
css touch up
himaniraghav3 Aug 13, 2025
8e2377e
Add media queries for filters
himaniraghav3 Aug 14, 2025
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
41 changes: 38 additions & 3 deletions src/apps/admin/src/lib/components/common/Pagination/Pagination.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,49 @@
justify-content: flex-end;
align-items: center;
padding: 16px 0;
gap: $sp-4;
gap: $sp-2;
Copy link

Choose a reason for hiding this comment

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

Consider verifying if changing the gap from $sp-4 to $sp-2 aligns with the overall design consistency and spacing guidelines of the application.


.pageNumbers button,
.previous,
.disabled,
.first,
.last,
.next {
box-shadow: none;
border: 1px solid #E9ECEF;
border-radius: 4px;
color: #0A0A0A;
font-weight: 400;
font-size: 14px;
}

.previous,
.first,
.last,
.next {
padding: 7px 11px;

&:disabled {
background-color: #E9ECEF !important;
Copy link

Choose a reason for hiding this comment

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

The use of !important can lead to specificity issues. Consider refactoring the CSS to avoid using !important if possible.

}
}


.pageNumbers {
display: flex;
justify-content: center;
align-items: center;
gap: $sp-1;
gap: $sp-2;

button {
padding: 3px 12px;
}

button.active {
color: $black-60;
pointer-events: none;
box-shadow: inset 0 0 0 2px #{$black-60};
background-color: $teal-160;
color: white;
}
}

Expand All @@ -35,4 +66,8 @@
@media (max-width: #{$mobile-max}) {
justify-content: center;
}

:global(.btn-style-secondary) {
Copy link

Choose a reason for hiding this comment

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

Avoid using !important as it can make the CSS harder to maintain and override. Consider refactoring the styles to achieve the desired effect without using !important.

box-shadow: none !important;
}
}
5 changes: 0 additions & 5 deletions src/apps/admin/src/lib/components/common/Pagination/Pagination.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ const Pagination: FC<PaginationProps> = (props: PaginationProps) => {
size='md'
icon={IconOutline.ChevronDoubleLeftIcon}
iconToLeft
label='FIRST'
disabled={props.page === 1 || props.disabled}
className={styles.first}
/>
Expand All @@ -84,7 +83,6 @@ const Pagination: FC<PaginationProps> = (props: PaginationProps) => {
size='md'
icon={IconOutline.ChevronLeftIcon}
iconToLeft
label='PREVIOUS'
disabled={props.page === 1 || props.disabled}
Copy link

Choose a reason for hiding this comment

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

The label prop has been removed from the Button component. Ensure that this change is intentional and that the button still meets accessibility requirements, as removing text labels can affect screen readers.

className={styles.previous}
/>
Expand All @@ -93,7 +91,6 @@ const Pagination: FC<PaginationProps> = (props: PaginationProps) => {
<Button
key={`page-${i}`}
secondary
variant='round'
label={`${i}`}
onClick={createHandlePageClick(i)}
className={i === props.page ? styles.active : ''}
Copy link

Choose a reason for hiding this comment

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

The variant='round' prop has been removed from the Button component. If this was intentional, ensure that the design requirements do not require the buttons to have a round variant. If it was removed by mistake, consider adding it back to maintain the intended button style.

Expand All @@ -107,7 +104,6 @@ const Pagination: FC<PaginationProps> = (props: PaginationProps) => {
size='md'
Copy link

Choose a reason for hiding this comment

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

The label='NEXT' prop has been removed from the Button component. Ensure that this change is intentional and that the button's functionality and accessibility are not affected by the removal of the label.

icon={IconOutline.ChevronRightIcon}
iconToRight
label='NEXT'
disabled={props.page === totalPages || props.disabled}
className={styles.next}
/>
Expand All @@ -118,7 +114,6 @@ const Pagination: FC<PaginationProps> = (props: PaginationProps) => {
size='md'
Copy link

Choose a reason for hiding this comment

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

The label='LAST' prop was removed from the Button component. Ensure that this change is intentional and that the UI/UX is not negatively impacted by the absence of this label.

icon={IconOutline.ChevronDoubleRightIcon}
iconToRight
label='LAST'
disabled={props.page === totalPages || props.disabled}
className={styles.last}
/>
Expand Down
2 changes: 2 additions & 0 deletions src/apps/review/src/config/routes.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@ export const activeReviewAssigmentsRouteId = 'active-review-assigments'
export const openOpportunitiesRouteId = 'open-opportunities'
export const pastReviewAssignmentsRouteId = 'past-review-assignments'
export const challengeDetailRouteId = ':challengeId'
export const scorecardRouteId = 'scorecard'
export const viewScorecardRouteId = ':scorecardId'
22 changes: 18 additions & 4 deletions src/apps/review/src/lib/components/NavTabs/NavTabs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import {
FC,
SetStateAction,
useCallback,
useContext,
useEffect,
useMemo,
useRef,
useState,
Expand All @@ -13,23 +15,35 @@ import classNames from 'classnames'

import { useClickOutside } from '~/libs/shared/lib/hooks'

import { getTabIdFromPathName, TabsConfig } from './config'
import { ReviewAppContext } from '../../contexts'
import { ReviewAppContextModel } from '../../models'

import { getTabIdFromPathName, getTabsConfig } from './config'
Copy link

Choose a reason for hiding this comment

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

The function getTabsConfig is imported but not used in this file. Consider removing the import if it is not needed.

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

const NavTabs: FC = () => {
const navigate: NavigateFunction = useNavigate()
const [isOpen, setIsOpen] = useState<boolean>(false)
const triggerRef = useRef<HTMLDivElement>(null)
const { pathname }: { pathname: string } = useLocation()

const { loginUserInfo }: ReviewAppContextModel = useContext(ReviewAppContext)
const userRoles = useMemo(() => loginUserInfo?.roles || [], [loginUserInfo?.roles])
const tabs = useMemo(() => getTabsConfig(userRoles), [userRoles])

const activeTabPathName: string = useMemo<string>(
() => getTabIdFromPathName(pathname),
[pathname],
() => getTabIdFromPathName(pathname, userRoles),
Copy link

Choose a reason for hiding this comment

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

The function getTabIdFromPathName now takes an additional parameter userRoles. Ensure that this function is designed to handle the new parameter correctly and that it doesn't introduce any unintended side effects.

[pathname, userRoles],
)
const [activeTab, setActiveTab]: [
string,
Dispatch<SetStateAction<string>>
] = useState<string>(activeTabPathName)

useEffect(() => {
setActiveTab(activeTabPathName)
Copy link

Choose a reason for hiding this comment

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

The useEffect hook is used to update activeTab whenever activeTabPathName changes. Verify that this does not cause unnecessary re-renders or side effects, especially if activeTabPathName changes frequently.

}, [activeTabPathName])

const handleTabChange = useCallback(
(tabId: string) => {
setActiveTab(tabId)
Expand Down Expand Up @@ -58,7 +72,7 @@ const NavTabs: FC = () => {
Review
</div>
<ul className={styles.tab}>
{TabsConfig.map(tab => (
{tabs.map(tab => (
Copy link

Choose a reason for hiding this comment

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

The variable tabs is used here, but it is not clear from the diff if it has been defined or imported correctly. Ensure that tabs is properly defined or imported in the component to avoid runtime errors.

<li
key={tab.id}
className={
Expand Down
44 changes: 28 additions & 16 deletions src/apps/review/src/lib/components/NavTabs/config/tabs-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,37 @@ import {
activeReviewAssigmentsRouteId,
Copy link

Choose a reason for hiding this comment

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

The variable name activeReviewAssigmentsRouteId seems to have a typo. It should be activeReviewAssignmentsRouteId instead of activeReviewAssigmentsRouteId. Please correct the spelling.

openOpportunitiesRouteId,
pastReviewAssignmentsRouteId,
scorecardRouteId,
} from '~/apps/review/src/config/routes.config'

export const TabsConfig: TabsNavItem[] = [
{
id: activeReviewAssigmentsRouteId,
title: 'Active Review Assignments',
},
{
id: openOpportunitiesRouteId,
title: 'Open Opportunities',
},
{
id: pastReviewAssignmentsRouteId,
title: 'Past Review Assignments',
},
]
export function getTabsConfig(userRoles: string[]): TabsNavItem[] {
const tabs: TabsNavItem[] = [
{
id: activeReviewAssigmentsRouteId,
title: 'Active Review Assignments',
},
{
id: openOpportunitiesRouteId,
title: 'Open Opportunities',
},
{
id: pastReviewAssignmentsRouteId,
title: 'Past Review Assignments',
},
]

export function getTabIdFromPathName(pathname: string): string {
const matchItem = _.find(TabsConfig, item => pathname.includes(`/${item.id}`))
if (userRoles.includes('administrator')) {
Copy link

Choose a reason for hiding this comment

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

Consider checking for null or undefined values for userRoles before calling includes. This will prevent potential runtime errors if userRoles is not properly initialized.

tabs.push({
id: scorecardRouteId,
title: 'Scorecards',
})
}

return tabs
}

export function getTabIdFromPathName(pathname: string, userRoles: string[]): string {
const matchItem = _.find(getTabsConfig(userRoles), item => pathname.includes(`/${item.id}`))

if (matchItem) {
return matchItem.id
Expand Down
176 changes: 176 additions & 0 deletions src/apps/review/src/lib/components/ScorecardsFilter/ScorecardsFilter.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
@import '@libs/ui/styles/includes';

.filtersContainer {
display: flex;
flex-wrap: wrap;
gap: 1rem;
margin-top: 32px;
align-items: flex-end;
padding: 20px;
background-color: #E0E4E84D;
justify-content: center;
align-items: center;
Copy link

Choose a reason for hiding this comment

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

The property align-items: center; is duplicated on line 8. Consider removing one of them to avoid redundancy.


.inputWrapper {
Copy link

Choose a reason for hiding this comment

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

The .inputWrapper class has gap: $sp-1;, but $sp-1 is not defined in the provided code. Ensure that $sp-1 is defined in the imported styles or replace it with a valid value.

gap: $sp-1;
flex: 5;
min-width: 355px;
width: 100%;
margin-bottom: 0;
justify-content: end;

label {
flex-direction: row;
align-items: center;
justify-content: space-between;
}
}

.inputText {
color: $black-60;
box-sizing: border-box;
border: 0;
width: 100%;
padding: 0;
margin: 0;
height: auto;
border-radius: 0;

&:focus {
box-shadow: none;
border: none;
outline: none;
color: $black-100;
}

&::placeholder {
color: $black-60;
opacity: 1;
text-transform: none;
}
}

.inputIcon {
color: $black-60;
Copy link

Choose a reason for hiding this comment

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

The .inputIcon class uses $black-60, but this variable is not defined in the provided code. Ensure that $black-60 is defined in the imported styles or replace it with a valid value.

}



.searchInput {
flex: 5;
min-width: 355px;
width: 100%;
margin-bottom: 0;

}

.typeSelect {
flex: 2;
margin-bottom: 0;

> div:first-child {
padding: 3px 16px 4px 16px;
}
}

.projectTypeSelect {
flex: 1.5;
margin-bottom: 0;

> div:first-child {
padding: 3px 16px 4px 16px;
}
}

.categorySelect {
flex: 1.5;
margin-bottom: 0;

> div:first-child {
padding: 3px 16px 4px 16px;
}
}

.statusSelect {
flex: 1;
margin-bottom: 0;

> div:first-child {
padding: 3px 16px 4px 16px;
}
}

.clearButton {
flex: 0.5;
color: #767676;
text-transform: capitalize;
border-radius: 4px;
border: 1px #A8A8A8 solid;
box-shadow: none;
font-weight: 700;
}

}

@media (max-width: 768px) {
.filtersContainer {
Copy link

Choose a reason for hiding this comment

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

Consider adding a comment to explain the purpose of the flex-wrap: wrap; rule for .filtersContainer to improve readability.

flex-wrap: wrap;
justify-content: center;
gap: 1rem;

.inputWrapper {
flex: 1 1 100%;
min-width: 0 !important;
}

// Two filters per row
.typeSelect,
.projectTypeSelect,
.categorySelect,
.statusSelect,
.clearButton {
flex: 1 1 calc(50% - 0.5rem);
Copy link

Choose a reason for hiding this comment

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

The comment // Two filters per row should be removed as it violates the instruction to never suggest adding comments to the code.

min-width: 0;
}
}
}

@media (max-width: 1285px) and (min-width: 769px) {

.filtersContainer {

.typeSelect,
.projectTypeSelect,
.categorySelect,
.statusSelect {
flex: 1 1 20%;
min-width: 0;

> div:first-child {
white-space: normal;
min-width: 0;
}
}
}
}


@media (max-width: 500px) {
.filtersContainer > * {
flex: 1 1 100%;
}
}

:global(.input-el) {
margin-bottom: 0;
padding: 3px 16px 3px 16px;
}

:global(.input-wrapper) {
height: 38px;
}

:global(.btn-size-lg) {
padding: 6px 24px;
}

Loading