Show reopen button for registration phase, if submission or TG submission phase is open #1302
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| submission, | ||
| })) | ||
|
|
||
| entries.sort((a: SubmissionScoreEntry, b: SubmissionScoreEntry) => { |
There was a problem hiding this comment.
[performance]
The sorting logic in sortSubmissionsByReviewScoreDesc could be optimized by using a single comparison function. Consider using localeCompare for string comparisons or directly comparing numbers to simplify the logic.
| const selectedTab = props.selectedTab | ||
| const providedReviews = props.reviews | ||
| const providedSubmitterReviews = props.submitterReviews | ||
| const normalizedSelectedTab = useMemo( |
There was a problem hiding this comment.
[💡 maintainability]
The useMemo hook for normalizedSelectedTab is used correctly, but ensure that the normalizeTabLabel function is pure and does not have side effects, as it could lead to unexpected behavior.
| () => normalizeTabLabel(selectedTab), | ||
| [selectedTab], | ||
| ) | ||
| const shouldSortReviewTabByScore = useMemo( |
There was a problem hiding this comment.
[💡 performance]
The useMemo hook for shouldSortReviewTabByScore is used correctly, but be cautious about the dependency array. If props.isActiveChallenge or normalizedSelectedTab change frequently, it could lead to unnecessary re-renders.
| [resolvedSubmitterReviews], | ||
| ) | ||
| const reviewerRowsForReviewTab = useMemo( | ||
| () => (shouldSortReviewTabByScore |
There was a problem hiding this comment.
[💡 maintainability]
The useMemo hook for reviewerRowsForReviewTab and submitterRowsForReviewTab is used correctly, but ensure that sortSubmissionsByReviewScoreDesc is a pure function without side effects to avoid unexpected behavior.
| className?: string | ||
| } | ||
|
|
||
| const normalizePhaseName = (name?: string): string => (name ? name.trim() |
There was a problem hiding this comment.
[💡 style]
Consider using name?.trim().toLowerCase() ?? '' instead of the ternary operator for a more concise and idiomatic approach to handle optional chaining and default values.
| return | ||
| } | ||
|
|
||
| allowed.add(phase.predecessor) |
There was a problem hiding this comment.
[maintainability]
The allowed.add(phase.predecessor) call is duplicated in the challengePhases.forEach loop and the addPhaseIdentifiers function. Consider removing the duplication to improve maintainability.
| }) | ||
|
|
||
| const hasSubmissionVariantOpen = challengePhases.some(phase => ( | ||
| phase?.isOpen && SUBMISSION_PHASE_NAMES.has(normalizePhaseName(phase.name)) |
There was a problem hiding this comment.
[performance]
The normalizePhaseName function is called multiple times for the same phase name. Consider normalizing the phase name once and storing it in a variable to improve performance.
Show reopen button for registration phase, if submission or TG submission phase is open