Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
Original file line number Diff line number Diff line change
Expand Up @@ -169,3 +169,26 @@
color: $black-80;
white-space: nowrap;
}

.moreIndicator {
display: inline-block;
background: $black-5;
border: 1px solid $black-20;
border-radius: $sp-1;
padding: $sp-1 $sp-2;
font-size: 12px;
line-height: 16px;
color: $black-80;
font-weight: 700;
min-width: 24px;
text-align: center;
cursor: help;
}

.link {
display: flex;
gap: $sp-1;
text-decoration: underline;
color: $link-blue;
cursor: pointer;
}
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,23 @@ export const ProfileCompletionPage: FC = () => {
const displayedRows = useMemo(() => profiles
.map(profile => {
const userSkills = profile.userId ? (memberSkills.get(profile.userId) || []) : []
const principalSkills = userSkills
.filter(skill => skill.displayMode?.name === UserSkillDisplayModes.principal)
.slice(0, 5)

// Prioritize principal skills, then add additional skills
const allSkillsByPriority = [
...userSkills.filter(skill => skill.displayMode?.name === UserSkillDisplayModes.principal),
...userSkills.filter(skill => skill.displayMode?.name !== UserSkillDisplayModes.principal),
]

const displayedSkills = allSkillsByPriority.slice(0, 5)
Copy link

Choose a reason for hiding this comment

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

[⚠️ 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.

const additionalSkillsCount = Math.max(0, allSkillsByPriority.length - 5)

return {
...profile,
additionalSkillsCount,
countryLabel: profile.countryCode
? countryMap.get(profile.countryCode) || profile.countryName || profile.countryCode
: profile.countryName || '-',
displayedSkills,
fullName: [profile.firstName, profile.lastName].filter(Boolean)
.join(' ')
.trim(),
Expand All @@ -133,7 +141,6 @@ export const ProfileCompletionPage: FC = () => {
: profile.countryName]
.filter(Boolean)
.join(', '),
principalSkills,
}
})
.sort((a, b) => a.handle.localeCompare(b.handle)), [profiles, countryMap, memberSkills])
Expand Down Expand Up @@ -195,8 +202,7 @@ export const ProfileCompletionPage: FC = () => {
<th>Handle</th>
<th>Location</th>
<th>Skills</th>
<th>Principal Skills</th>
<th>Go to profile</th>
<th>{' '}</th>
Copy link

Choose a reason for hiding this comment

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

[⚠️ 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.

</tr>
</thead>
<tbody>
Expand Down Expand Up @@ -224,22 +230,27 @@ export const ProfileCompletionPage: FC = () => {
</a>
</td>
<td>{profile.locationLabel || profile.countryLabel}</td>
<td>{profile.skillCount ?? '-'}</td>
<td>
{profile.principalSkills && profile.principalSkills.length > 0 ? (
{profile.displayedSkills && profile.displayedSkills.length > 0 ? (
<div className={styles.skillsList}>
{profile.principalSkills.map(skill => (
{profile.displayedSkills.map(skill => (
<span key={skill.id} className={styles.skillTag}>
{skill.name}
</span>
))}
{profile.additionalSkillsCount > 0 && (
<span className={styles.moreIndicator}>
Copy link

Choose a reason for hiding this comment

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

[⚠️ 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} skills
</span>
)}
</div>
) : (
'-'
)}
</td>
<td>
<a
className={styles.link}
Copy link

Choose a reason for hiding this comment

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

[💡 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.

href={`${EnvironmentConfig.USER_PROFILE_URL}/${profile.handle}`}
target='_blank'
rel='noreferrer noopener'
Expand Down
Loading