Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ...userSkills.filter(skill => skill.displayMode?.name !== UserSkillDisplayModes.principal), | ||
| ] | ||
|
|
||
| const displayedSkills = allSkillsByPriority.slice(0, 5) |
There was a problem hiding this comment.
[maintainability]
The use of slice(0, 5) to limit the number of displayed skills is hardcoded. Consider making this a configurable constant or prop to improve maintainability and flexibility.
| ))} | ||
| {profile.additionalSkillsCount > 0 && ( | ||
| <Tooltip | ||
| content={`${profile.additionalSkillsCount} more skill${profile.additionalSkillsCount === 1 ? '' : 's'}`} |
There was a problem hiding this comment.
[accessibility]
The Tooltip component is used to display additional skills count. Ensure that the Tooltip component is accessible and works well on all devices, especially for users relying on keyboard navigation or screen readers.
| <th>Skills</th> | ||
| <th>Principal Skills</th> | ||
| <th>Go to profile</th> | ||
| <th>{' '}</th> |
There was a problem hiding this comment.
[readability]
Changing the table header from 'Go to profile' to a space character may lead to confusion for users. Consider using a more descriptive label or an icon to indicate the purpose of this column.
| </td> | ||
| <td> | ||
| <a | ||
| className={styles.link} |
There was a problem hiding this comment.
[💡 maintainability]
The addition of className={styles.link} to the anchor tag is a good practice for styling, but ensure that the styles.link class is defined in ProfileCompletionPage.module.scss to avoid potential styling issues.
| </span> | ||
| ))} | ||
| {profile.additionalSkillsCount > 0 && ( | ||
| <span className={styles.moreIndicator}> |
There was a problem hiding this comment.
[readability]
The removal of the Tooltip component around the additional skills count may reduce the clarity of the information presented to the user. Consider reintroducing the tooltip to provide context about the additional skills count, especially if the number is significant.
| ))} | ||
| {profile.additionalSkillsCount > 0 && ( | ||
| <span className={styles.moreIndicator}> | ||
| + |
There was a problem hiding this comment.
[💡 readability]
The change from a single line to multiple lines for displaying additionalSkillsCount and the text 'skills' does not improve readability or maintainability. Consider reverting to a single line to keep the code concise.
vas3a
deleted the
PM-4198_customer-portal-completed-profiles-report-updates
branch
1 participant
Related JIRA Ticket:
https://topcoder.atlassian.net/browse/PM-4198
What's in this PR?