PM-3329 show skills activity details #1419
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| skill={skill} | ||
| key={skill.id} | ||
| theme={isSkillVerified(skill) ? 'verified' : 'dark'} | ||
| fetchSkillDetails={canFetchSkillDetails ? fetchSkillDetails : undefined} |
There was a problem hiding this comment.
[correctness]
Passing undefined to fetchSkillDetails when canFetchSkillDetails is false might lead to unexpected behavior if SkillPill or GroupedSkillsUI do not handle undefined properly. Consider ensuring these components can handle undefined gracefully.
| return xhrGetBlobAsync<Blob>(`${profileUrl(handle)}/profileDownload`) | ||
| } | ||
|
|
||
| export function getMemberSkillDetails(handle: string, skillId: string): Promise<UserSkillWithActivity> { |
There was a problem hiding this comment.
[maintainability]
Consider adding error handling for the xhrGetAsync call to manage potential network or API errors gracefully. This will improve the robustness of the function.
| } | ||
|
|
||
| export type UserSkillWithActivity = { | ||
| lastUsedDate: string |
There was a problem hiding this comment.
[correctness]
Consider using a more descriptive type for lastUsedDate such as Date instead of string to ensure date operations are type-safe and consistent.
| } | ||
| } | ||
|
|
||
| .tootltipRow { |
There was a problem hiding this comment.
[❗❗ correctness]
The class name .tootltipRow appears to have a typo. It should likely be .tooltipRow for consistency and to avoid confusion.
| @@ -1,8 +1,12 @@ | |||
| import { FC, ReactNode, useCallback, useMemo } from 'react' | |||
| /* eslint-disable complexity */ | |||
| import { FC, ReactNode, useCallback, useMemo, useState } from 'react' | |||
There was a problem hiding this comment.
[💡 maintainability]
Consider removing the complexity ESLint rule disable directive if it's not necessary. Disabling rules can hide potential issues.
| <a | ||
| key={s.id} | ||
| className={classNames(styles.tootltipRow, styles.padLeft)} | ||
| href={`${EnvironmentConfig.URLS.CHALLENGES_PAGE}/${s.id}`} |
There was a problem hiding this comment.
[❗❗ security]
The target='blank' attribute should be target='_blank' to correctly open links in a new tab. Additionally, consider adding rel='noopener noreferrer' for security reasons to prevent the new page from gaining access to the window.opener property.
| <a | ||
| key={s.completionEventId} | ||
| className={classNames(styles.tootltipRow, styles.padLeft)} | ||
| href={`${EnvironmentConfig.URLS.ACADEMY_COURSE}/${s.certification}`} |
There was a problem hiding this comment.
[❗❗ security]
The target='blank' attribute should be target='_blank' to correctly open links in a new tab. Additionally, consider adding rel='noopener noreferrer' for security reasons to prevent the new page from gaining access to the window.opener property.
| <a | ||
| key={s.completionEventId} | ||
| className={classNames(styles.tootltipRow, styles.padLeft)} | ||
| href={`${EnvironmentConfig.URLS.ACADEMY_CERTIFICATION}/${s.dashedName}`} |
There was a problem hiding this comment.
[❗❗ security]
The target='blank' attribute should be target='_blank' to correctly open links in a new tab. Additionally, consider adding rel='noopener noreferrer' for security reasons to prevent the new page from gaining access to the window.opener property.
|
|
||
| return ( | ||
| <> | ||
| <div className={styles.tootltipRow}> |
There was a problem hiding this comment.
[correctness]
There is a typo in the class name styles.tootltipRow. It should be styles.tooltipRow.
|
|
||
| return ( | ||
| <> | ||
| <div className={styles.tootltipRow}> |
| <a | ||
| key={s.completionEventId} | ||
| className={classNames(styles.tootltipRow, styles.padLeft)} | ||
| href={`${EnvironmentConfig.URLS.ACADEMY_COURSE}/${s.certification}`} |
| <a | ||
| key={s.id} | ||
| className={classNames(styles.tootltipRow, styles.padLeft)} | ||
| href={`${EnvironmentConfig.URLS.CHALLENGES_PAGE}/${s.id}`} |
| <a | ||
| key={s.completionEventId} | ||
| className={classNames(styles.tootltipRow, styles.padLeft)} | ||
| href={`${EnvironmentConfig.URLS.ACADEMY_CERTIFICATION}/${s.dashedName}`} |
…M-3329_show-skills-activity-details
| key={s.id} | ||
| className={classNames(styles.tooltipRow, styles.padLeft)} | ||
| href={`${EnvironmentConfig.URLS.CHALLENGES_PAGE}/${s.id}`} | ||
| target='_blank' |
There was a problem hiding this comment.
[❗❗ security]
The target='_blank' attribute should always be accompanied by rel='noopener noreferrer' to prevent security vulnerabilities such as reverse tabnabbing. This has been correctly added here.
| key={s.completionEventId} | ||
| className={classNames(styles.tooltipRow, styles.padLeft)} | ||
| href={`${EnvironmentConfig.URLS.ACADEMY_COURSE}/${s.certification}`} | ||
| target='_blank' |
There was a problem hiding this comment.
[❗❗ security]
The target='_blank' attribute should always be accompanied by rel='noopener noreferrer' to prevent security vulnerabilities such as reverse tabnabbing. This has been correctly added here.
| key={s.completionEventId} | ||
| className={classNames(styles.tooltipRow, styles.padLeft)} | ||
| href={`${EnvironmentConfig.URLS.ACADEMY_CERTIFICATION}/${s.dashedName}`} | ||
| target='_blank' |
There was a problem hiding this comment.
[❗❗ security]
The target='_blank' attribute should always be accompanied by rel='noopener noreferrer' to prevent security vulnerabilities such as reverse tabnabbing. This has been correctly added here.
Related JIRA Ticket:
https://topcoder.atlassian.net/browse/PM-3329
What's in this PR?
Depending on how a skill was earned, hovering the skill "verified" icon will display activity info:
related PR: topcoder-platform/member-api-v6#41