Fixes for work experience display with imported traits #1286
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| } | ||
| } | ||
|
|
||
| const companyName: string | undefined = formValues.company as string | undefined |
There was a problem hiding this comment.
[💡 maintainability]
The variable companyName is assigned the value of formValues.company cast to string | undefined. However, formValues.company is already typed as string | undefined, making the cast redundant.
| size='lg' | ||
| /> | ||
| workExpirence?.map((work: UserTrait, indx: number) => { | ||
| const companyName: string | undefined = work.company || work.companyName |
There was a problem hiding this comment.
[correctness]
The uniqueKey generation logic uses work.timePeriodFrom || work.startDate. If both timePeriodFrom and startDate are defined, timePeriodFrom will always be used. Consider whether this logic is intentional and if it covers all edge cases.
| <div | ||
| className={styles.workExpirenceCardWrap} | ||
| key={uniqueKey || `${work.position}-${indx}`} | ||
| > |
There was a problem hiding this comment.
[correctness]
The fallback key ${work.position}-${indx} might not be unique if multiple entries have the same position and index. Consider using a more robust unique identifier.
| /> | ||
| )) | ||
| ? workExpirence?.map((work: UserTrait, index: number) => { | ||
| const companyName: string | undefined = work.company || work.companyName |
There was a problem hiding this comment.
[correctness]
The variable companyName is assigned using a fallback mechanism (work.company || work.companyName). Ensure that work.companyName is a valid alternative and that this fallback logic is intended. If both fields can be undefined, consider handling this case explicitly.
| )) | ||
| ? workExpirence?.map((work: UserTrait, index: number) => { | ||
| const companyName: string | undefined = work.company || work.companyName | ||
| const uniqueKey: string = [ |
There was a problem hiding this comment.
[❗❗ correctness]
The uniqueKey generation logic uses multiple fields to create a key. Ensure that all fields used (companyName, work.industry, work.position, work.timePeriodFrom, work.startDate) are consistently available and that their combination will always result in a unique key. If any field can be undefined, this might lead to non-unique keys.
| { | ||
| props.work.timePeriodFrom || props.work.timePeriodTo || props.work.working ? ( | ||
| <div className={styles.workExpirenceCardHeaderRight}> | ||
| const formatDateRange = (work: UserTrait): string | undefined => { |
There was a problem hiding this comment.
[performance]
The function formatDateRange uses moment for date formatting. Consider using a more modern date library like date-fns or luxon for better performance and smaller bundle size, as moment is known to be relatively large and is no longer actively maintained.
| } | ||
|
|
||
| const WorkExpirenceCard: FC<WorkExpirenceCardProps> = (props: WorkExpirenceCardProps) => { | ||
| const companyName: string | undefined = props.work.company || props.work.companyName |
There was a problem hiding this comment.
[correctness]
The variable companyName is assigned using a fallback to props.work.companyName. Ensure that companyName is a valid and expected property of UserTrait to avoid potential runtime errors.
|
|
||
| const WorkExpirenceCard: FC<WorkExpirenceCardProps> = (props: WorkExpirenceCardProps) => { | ||
| const companyName: string | undefined = props.work.company || props.work.companyName | ||
| const city: string | undefined = props.work.cityTown || props.work.city |
There was a problem hiding this comment.
[correctness]
The variable city is assigned using a fallback to props.work.city. Ensure that city is a valid and expected property of UserTrait to avoid potential runtime errors.
| if (targetUrl) { | ||
| if (typeof window !== 'undefined') { | ||
| const openedWindow = window.open(targetUrl, '_blank', 'noopener,noreferrer') | ||
| window.open(targetUrl, '_blank', 'noopener,noreferrer') |
There was a problem hiding this comment.
[maintainability]
The variable openedWindow was removed, which previously held the reference to the opened window. If there was any intention to use this reference later (e.g., to manipulate the window or check if it was successfully opened), this change could lead to issues. Ensure that the removal of this variable does not affect any other part of the code that might rely on it.
Fixes for work experience display with imported traits