-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
feature(builder): added the workType and employmentType params in experience section #2418
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughExperience sections now support employment type and work type metadata through new schema fields, enums, and validation. Twelve template files are updated to conditionally display employmentType and workType indicators inline with position and location. A form dialog gains UI controls for selecting and customizing these types, while new localization hooks map enum values to translated strings. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
♻️ Duplicate comments (5)
apps/artboard/src/templates/leafish.tsx (1)
253-270: Same code duplication concern as other templates.This template uses the same inline separator pattern for
employmentTypeandtypefields. See the suggestion in onyx.tsx for extracting this into a reusable component.apps/artboard/src/templates/chikorita.tsx (1)
244-265: Same code duplication concern as other templates.This template uses the same inline separator pattern. See the suggestion in onyx.tsx for extracting this into a reusable component.
apps/artboard/src/templates/glalie.tsx (1)
256-277: Same code duplication concern as other templates.This template uses the same inline separator pattern. See the suggestion in onyx.tsx for extracting this into a reusable component.
apps/artboard/src/templates/kakuna.tsx (1)
258-275: Same code duplication concern as other templates.This template uses the same inline separator pattern. See the suggestion in onyx.tsx for extracting this into a reusable component.
apps/artboard/src/templates/ditto.tsx (1)
287-308: Same code duplication concern as other templates.This template uses the same inline separator pattern. See the suggestion in onyx.tsx for extracting this into a reusable component.
🧹 Nitpick comments (8)
libs/schema/src/sample.ts (1)
121-121: Remove commented code.The commented line
// type: "onsite"should be removed as it no longer serves a purpose and can cause confusion.Apply this diff:
- // type: "onsite", type: "On-Site",libs/schema/src/sections/experience.ts (2)
5-15: Consider using null/undefined instead of "none" sentinel value.While using
"none"as an enum value for optional display works, it's not the most idiomatic TypeScript/Zod pattern. Consider making these fields nullable (z.string().nullable()) or truly optional without defaults, allowingnullorundefinedto indicate "don't display" rather than a magic string value.However, if this pattern is consistent with the existing codebase's approach to optional fields, it's acceptable to keep.
27-27: Remove commented code.The commented lines showing an alternative lowercase approach should be removed to keep the code clean.
Apply this diff:
type: workTypeEnum.optional().default("On-Site"), employmentType: employmentTypeEnum.optional().default("Full-Time"), - // type: z.enum(["onsite", "remote", "hybrid"]).default("onsite").optional(), });And:
type: "On-Site", employmentType: "Full-Time", - // type: "onsite", };Also applies to: 44-44
apps/artboard/src/templates/onyx.tsx (1)
259-267: Extract duplicated separator pattern into a reusable component.The conditional rendering with inline dot separators is duplicated across multiple templates (onyx, leafish, chikorita, glalie, kakuna, ditto). Consider extracting this into a shared utility component to reduce duplication and make future styling changes easier.
Example:
const InlineWithSeparator = ({ items }: { items: string[] }) => ( <div className="flex items-center gap-2"> {items.filter(Boolean).map((item, i) => ( <Fragment key={i}> {i > 0 && <span className="inline-block aspect-square h-1 rounded-full bg-black" />} <span>{item}</span> </Fragment> ))} </div> );Additionally, the hardcoded
bg-blackseparator may not respect the resume theme. Consider usingbg-currentor a theme-aware color.Also applies to: 272-280
apps/artboard/src/templates/azurill.tsx (2)
282-290: Use theme color for separator consistency.The separator dot uses
bg-blackinstead of a theme-aware class likebg-primaryortext-primaryused elsewhere in the template. This creates visual inconsistency and may not respect theme customization.Additionally, the separator size
h-1.5differs from theh-1used in bronzor.tsx (line 264 & 277), creating cross-template inconsistency.Apply this diff to align with the theme and ensure consistent separator sizing:
- <span className="inline-block aspect-square h-1.5 rounded-full bg-black" /> + <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" />Consider standardizing the separator size across all templates (either
h-1orh-1.5).
291-299: Use theme color for separator consistency.The separator dot uses
bg-blackinstead of a theme-aware class. This creates the same inconsistency as noted in the employmentType block above.Apply this diff:
- <span className="inline-block aspect-square h-1.5 rounded-full bg-black" /> + <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" />apps/artboard/src/templates/bronzor.tsx (2)
260-268: Use theme color and standardize separator size.The separator dot uses
bg-blackinstead of a theme-aware class likebg-primaryortext-primaryused elsewhere. Additionally, the separator sizeh-1here differs from azurill.tsx'sh-1.5(line 286), creating inconsistency across templates.Apply this diff to align with the theme:
- <span className="inline-block aspect-square h-1 rounded-full bg-black" /> + <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" />The size change to
h-1.5ensures consistency with azurill.tsx. Ifh-1is preferred, update azurill.tsx accordingly.
273-281: Use theme color and standardize separator size.The separator dot uses
bg-blackinstead of a theme-aware class. This creates the same inconsistency as noted in the employmentType block above.Apply this diff:
- <span className="inline-block aspect-square h-1 rounded-full bg-black" /> + <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/artboard/src/templates/azurill.tsx(1 hunks)apps/artboard/src/templates/bronzor.tsx(1 hunks)apps/artboard/src/templates/chikorita.tsx(1 hunks)apps/artboard/src/templates/ditto.tsx(1 hunks)apps/artboard/src/templates/gengar.tsx(1 hunks)apps/artboard/src/templates/glalie.tsx(1 hunks)apps/artboard/src/templates/kakuna.tsx(1 hunks)apps/artboard/src/templates/leafish.tsx(1 hunks)apps/artboard/src/templates/nosepass.tsx(1 hunks)apps/artboard/src/templates/onyx.tsx(1 hunks)apps/artboard/src/templates/pikachu.tsx(1 hunks)apps/artboard/src/templates/rhyhorn.tsx(1 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx(3 hunks)libs/schema/src/sample.ts(2 hunks)libs/schema/src/sections/experience.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx (3)
libs/ui/src/components/form.tsx (5)
FormField(11-20)FormItem(22-32)FormLabel(36-50)FormControl(54-69)FormMessage(91-112)libs/ui/src/components/select.tsx (6)
Select(6-6)SelectTrigger(12-29)SelectValue(10-10)SelectContent(33-60)SelectGroup(8-8)SelectItem(77-96)libs/schema/src/sections/experience.ts (2)
employmentTypeEnum(6-15)workTypeEnum(5-5)
🔇 Additional comments (2)
libs/schema/src/sample.ts (1)
122-123: Sample data aligns with schema.The new
typeandemploymentTypefields are correctly populated in the sample data, matching the schema enums and demonstrating both visible values and the "none" option for hiding fields.Also applies to: 138-139
libs/schema/src/sections/experience.ts (1)
25-26: Schema extension is well-structured.The new enum-based fields with defaults are properly integrated into the schema. The use of
.optional()with.default()ensures backward compatibility while providing sensible defaults.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
libs/schema/src/sample.ts (1)
121-123: Remove commented-out code.Line 121 contains a commented-out alternative value that should be removed to keep the sample data clean.
Apply this diff to remove the commented line:
url: { label: "", href: "https://creativesolutions.inc/", }, - // workType: "onsite", workType: "On-Site", employmentType: "Full-Time",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/artboard/src/templates/azurill.tsx(1 hunks)apps/artboard/src/templates/bronzor.tsx(1 hunks)apps/artboard/src/templates/chikorita.tsx(1 hunks)apps/artboard/src/templates/ditto.tsx(1 hunks)apps/artboard/src/templates/gengar.tsx(1 hunks)apps/artboard/src/templates/glalie.tsx(1 hunks)apps/artboard/src/templates/kakuna.tsx(1 hunks)apps/artboard/src/templates/leafish.tsx(1 hunks)apps/artboard/src/templates/nosepass.tsx(1 hunks)apps/artboard/src/templates/onyx.tsx(1 hunks)apps/artboard/src/templates/pikachu.tsx(1 hunks)apps/artboard/src/templates/rhyhorn.tsx(1 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx(3 hunks)libs/schema/src/sample.ts(2 hunks)libs/schema/src/sections/experience.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/artboard/src/templates/rhyhorn.tsx
- apps/artboard/src/templates/bronzor.tsx
- apps/artboard/src/templates/gengar.tsx
- apps/artboard/src/templates/glalie.tsx
- apps/artboard/src/templates/leafish.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx (3)
libs/ui/src/components/form.tsx (5)
FormField(11-20)FormItem(22-32)FormLabel(36-50)FormControl(54-69)FormMessage(91-112)libs/ui/src/components/select.tsx (6)
Select(6-6)SelectTrigger(12-29)SelectValue(10-10)SelectContent(33-60)SelectGroup(8-8)SelectItem(77-96)libs/schema/src/sections/experience.ts (2)
employmentTypeEnum(6-15)workTypeEnum(5-5)
🔇 Additional comments (4)
apps/artboard/src/templates/ditto.tsx (1)
287-308: LGTM!The conditional rendering correctly handles both undefined/null values and the "none" sentinel value by checking truthiness before comparing to "none".
apps/artboard/src/templates/nosepass.tsx (1)
309-326: LGTM!The conditional rendering properly checks for field existence before comparing to "none", preventing "undefined" from being rendered as suggested in the previous review comment.
apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx (1)
107-138: Labels successfully differentiated.The form labels now clearly distinguish between "Employment Type" and "Work Type", addressing the previous review concern.
Also applies to: 140-170
apps/artboard/src/templates/azurill.tsx (1)
282-299: LGTM!The conditional rendering is implemented consistently with the other templates and correctly handles undefined/null values and the "none" sentinel.
|
Hello Mr. Divyanshu-Vatil, I would like to ask about the text input feature for previous job types. Has this feature been implemented? Thank you @divyanshu-patil. |
|
The pull request is currently in the open stage and hasn’t been merged yet. |
Just waiting for approval for the merge, right? That means, try contacting Mr. Amruth @AmruthPillai. |
maybe we should wait for a few more days as @AmruthPillai may have other commitments. |
|
@divyanshu-patil I think there's some work with localization needed. In my current install, the Employment and Work Types (drop-down items) are garbled. These strings should be customized in messages.po located at /apps/client/src/locales/en-US/messages.po I think. Attached is a screenshot as it appears at my end. |
|
@mithunsridharan can you share details to reproduce this issue, because when I tested the values are shown perfectly |
|
@mithunsridharan can you review the changes? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx (2)
113-113: Remove empty className attributes.Lines 113 and 141 contain
className=""attributes that serve no purpose and can be removed for cleaner code.Apply this diff:
- <FormItem className=""> + <FormItem>Also applies to: 141-141
123-123: Unnecessary eslint-disable directive.The
eslint-disable react/jsx-no-comment-textnodescomments on lines 123 and 151 appear unnecessary, as there are no comment-like text nodes in the JSX. This rule typically flags cases where you write text that looks like a comment (e.g.,<div>// comment</div>), but these SelectItem components only contain properly interpolated expressions.Consider removing these directives:
- // eslint-disable-next-line react/jsx-no-comment-textnodes <SelectItem key={type} value={type}>Also applies to: 151-151
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (52)
apps/client/src/locales/af-ZA/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/am-ET/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ar-SA/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/az-AZ/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/bg-BG/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/bn-BD/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ca-ES/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/cs-CZ/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/da-DK/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/de-DE/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/el-GR/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/en-US/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/es-ES/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/fa-IR/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/fi-FI/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/fr-FR/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/he-IL/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/hi-IN/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/hu-HU/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/id-ID/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/it-IT/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ja-JP/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/km-KH/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/kn-IN/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ko-KR/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/lt-LT/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/lv-LV/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ml-IN/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/mr-IN/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ms-MY/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ne-NP/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/nl-NL/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/no-NO/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/or-IN/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/pl-PL/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/pt-BR/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/pt-PT/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ro-RO/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ru-RU/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/sk-SK/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/sq-AL/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/sr-SP/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/sv-SE/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ta-IN/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/te-IN/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/th-TH/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/tr-TR/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/uk-UA/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/uz-UZ/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/vi-VN/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/zh-CN/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/zh-TW/messages.pois excluded by!apps/client/src/locales/*/messages.po
📒 Files selected for processing (4)
apps/client/src/locales/enums.ts(1 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx(3 hunks)libs/schema/src/sample.ts(2 hunks)libs/schema/src/sections/experience.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/schema/src/sample.ts
- apps/client/src/locales/enums.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx (4)
libs/ui/src/components/form.tsx (5)
FormField(11-20)FormItem(22-32)FormLabel(36-50)FormControl(54-69)FormMessage(91-112)libs/ui/src/components/select.tsx (6)
Select(6-6)SelectTrigger(12-29)SelectValue(10-10)SelectContent(33-60)SelectGroup(8-8)SelectItem(77-96)libs/schema/src/sections/experience.ts (2)
employmentTypeEnum(6-15)workTypeEnum(5-5)apps/client/src/locales/enums.ts (2)
getEmploymentTypeLabels(22-31)getWorkTypeLabels(15-20)
🔇 Additional comments (2)
libs/schema/src/sections/experience.ts (1)
5-46: LGTM! Schema implementation is clean and well-structured.The enum definitions, schema extensions, and default values are correctly implemented. Setting both
workTypeandemploymentTypeto"none"by default ensures these fields remain hidden until users explicitly opt in, which aligns with the intended behavior.apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx (1)
109-135: LGTM! Employment Type field implementation is correct.The
employmentTypeSelect field correctly uses?? "none"as the fallback value, ensuring consistency with the schema defaults and allowing the field to remain hidden for existing experiences that don't have this data.
apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx
Outdated
Show resolved
Hide resolved
|
Is it possible to implement a feature that allows us to customize the type of work beforehand? What I mean is that users can type in their type of work. Thank you in advance, Mr @divyanshu-patil. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (7)
libs/schema/src/sections/experience.ts (1)
29-33: Require custom text when “other” is selected (schema-level guard).Currently, users can select "other" without providing
customEmploymentType/customWorkType, leading to incomplete data. Add a Zod refinement to enforce the custom fields when "other" is chosen.-export const experienceSchema = itemSchema.extend({ +export const experienceSchema = itemSchema + .extend({ company: z.string().min(1), position: z.string(), location: z.string(), date: z.string(), summary: z.string(), url: urlSchema, workType: workTypeEnum.optional(), employmentType: employmentTypeEnum.optional(), customEmploymentType: z.string().optional(), customWorkType: z.string().optional(), -}); + }) + .superRefine((val, ctx) => { + if (val.employmentType === "other" && !val.customEmploymentType?.trim()) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["customEmploymentType"], + message: "Please specify employment type", + }); + } + if (val.workType === "other" && !val.customWorkType?.trim()) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["customWorkType"], + message: "Please specify work type", + }); + } + });apps/artboard/src/templates/gengar.tsx (1)
265-277: Localize labels instead of rendering raw enum values; add “Other” fallback.Render translated labels for
employmentType/workTypeand fall back to a localized “Other” when custom text is missing. This fixes the garbled/mismatched strings reported and ensures i18n consistency across templates.Apply within the shown spans:
- <span> - {item.employmentType === "other" && item.customEmploymentType - ? item.customEmploymentType - : item.employmentType}{" "} - </span> + <span> + {item.employmentType === "other" + ? (item.customEmploymentType || i18n._(EMPLOYMENT_LABELS.other)) + : i18n._(EMPLOYMENT_LABELS[item.employmentType])} + </span>- <span> - {item.workType === "other" && item.customWorkType - ? item.customWorkType - : item.workType}{" "} - </span> + <span> + {item.workType === "other" + ? (item.customWorkType || i18n._(WORK_LABELS.other)) + : i18n._(WORK_LABELS[item.workType])} + </span>Add once near the imports (supporting code outside the hunk):
import { i18n } from "@lingui/core"; import { t } from "@lingui/macro"; import type { EmploymentTypeEnum, WorkTypeEnum } from "@reactive-resume/schema"; const EMPLOYMENT_LABELS: Record<EmploymentTypeEnum, string> = { "Full-Time": t`Full-Time`, "Part-Time": t`Part-Time`, Internship: t`Internship`, Contract: t`Contract`, Freelance: t`Freelance`, Temporary: t`Temporary`, Volunteer: t`Volunteer`, none: t`Not specified`, other: t`Other`, }; const WORK_LABELS: Record<WorkTypeEnum, string> = { "On-Site": t`On-Site`, Remote: t`Remote`, Hybrid: t`Hybrid`, none: t`Not specified`, other: t`Other`, };Side note: switch
import { Education, Experience, Volunteer }toimport type { ... }to avoid pulling non-existent runtime values.Also applies to: 282-294
apps/artboard/src/templates/nosepass.tsx (1)
309-334: Use translated labels for types; avoid raw enum output.Mirror the builder’s i18n by mapping enum values to localized strings and fallback to localized “Other” when custom is missing.
- {item.employmentType && item.employmentType !== "none" && ( + {item.employmentType && item.employmentType !== "none" && ( <> <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" /> - <span> - {item.employmentType === "other" && item.customEmploymentType - ? item.customEmploymentType - : item.employmentType}{" "} - </span> + <span> + {item.employmentType === "other" + ? (item.customEmploymentType || i18n._(EMPLOYMENT_LABELS.other)) + : i18n._(EMPLOYMENT_LABELS[item.employmentType])} + </span> </> )}- {item.workType && item.workType !== "none" && ( + {item.workType && item.workType !== "none" && ( <> <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" /> - <span> - {item.workType === "other" && item.customWorkType - ? item.customWorkType - : item.workType}{" "} - </span> + <span> + {item.workType === "other" + ? (item.customWorkType || i18n._(WORK_LABELS.other)) + : i18n._(WORK_LABELS[item.workType])} + </span> </> )}Add the same import/mapping helpers as proposed in gengar.tsx.
apps/artboard/src/templates/ditto.tsx (1)
287-299: Internationalize type badges; fallback to localized “Other”.Render translated labels instead of raw enum values.
- <span> - {item.employmentType === "other" && item.customEmploymentType - ? item.customEmploymentType - : item.employmentType}{" "} - </span> + <span> + {item.employmentType === "other" + ? (item.customEmploymentType || i18n._(EMPLOYMENT_LABELS.other)) + : i18n._(EMPLOYMENT_LABELS[item.employmentType])} + </span>- {item.workType && item.workType !== "none" && ( + {item.workType && item.workType !== "none" && ( <> - <span> - {item.workType === "other" && item.customWorkType - ? item.customWorkType - : item.workType}{" "} - </span> + <span> + {item.workType === "other" + ? (item.customWorkType || i18n._(WORK_LABELS.other)) + : i18n._(WORK_LABELS[item.workType])} + </span> <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" /> </> )}Add the same import/mapping helpers as proposed in gengar.tsx.
Also applies to: 304-316
apps/artboard/src/templates/chikorita.tsx (1)
244-256: Translate type labels in output; avoid showing token strings.Adopt localized labels and handle empty custom text gracefully.
- <span> - {item.employmentType === "other" && item.customEmploymentType - ? item.customEmploymentType - : item.employmentType}{" "} - </span> + <span> + {item.employmentType === "other" + ? (item.customEmploymentType || i18n._(EMPLOYMENT_LABELS.other)) + : i18n._(EMPLOYMENT_LABELS[item.employmentType])} + </span>- {item.workType && item.workType !== "none" && ( + {item.workType && item.workType !== "none" && ( <> - <span> - {item.workType === "other" && item.customWorkType - ? item.customWorkType - : item.workType}{" "} - </span> + <span> + {item.workType === "other" + ? (item.customWorkType || i18n._(WORK_LABELS.other)) + : i18n._(WORK_LABELS[item.workType])} + </span> <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" /> </> )}Add the same import/mapping helpers as proposed in gengar.tsx.
Also applies to: 261-273
apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx (2)
158-205: Also clear customWorkType when selection ≠ “other”; tweak placeholder.Mirror the same cleanup for work type to avoid stale data.
- <Select {...field} value={field.value ?? "none"} onValueChange={field.onChange}> + <Select + {...field} + value={field.value ?? "none"} + onValueChange={(val) => { + field.onChange(val); + if (val !== "other") { + form.setValue("customWorkType", ""); + } + }} + >- placeholder={t`Enter custom Work type`} + placeholder={t`Enter custom work type`}
109-156: Clear stale customEmploymentType when selection changes; complete locale entries for new form labels.The stale custom text issue is valid and prevents leftover values when users switch away from "other". Additionally, the new form labels ("Specify Other Employment Type", "Enter custom employment type") are missing from locale catalogs—they will fall back to English if not added.
- <Select {...field} value={field.value ?? "none"} onValueChange={field.onChange}> + <Select + {...field} + value={field.value ?? "none"} + onValueChange={(val) => { + field.onChange(val); + if (val !== "other") { + // clear stale custom text + form.setValue("customEmploymentType", ""); + } + }} + >Optional: unify casing in form label.
- <FormLabel>{t`Specify Other Employment Type`}</FormLabel> + <FormLabel>{t`Specify other employment type`}</FormLabel>Locale follow-up: Add missing msgid/msgstr entries to all catalogs in
apps/client/src/locales/*/messages.pofor "Specify other employment type" and "Enter custom employment type" to ensure translations are complete.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/artboard/src/templates/azurill.tsx(1 hunks)apps/artboard/src/templates/bronzor.tsx(1 hunks)apps/artboard/src/templates/chikorita.tsx(1 hunks)apps/artboard/src/templates/ditto.tsx(1 hunks)apps/artboard/src/templates/gengar.tsx(2 hunks)apps/artboard/src/templates/glalie.tsx(1 hunks)apps/artboard/src/templates/kakuna.tsx(1 hunks)apps/artboard/src/templates/leafish.tsx(1 hunks)apps/artboard/src/templates/nosepass.tsx(1 hunks)apps/artboard/src/templates/onyx.tsx(1 hunks)apps/artboard/src/templates/pikachu.tsx(1 hunks)apps/artboard/src/templates/rhyhorn.tsx(1 hunks)apps/client/src/locales/enums.ts(1 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx(3 hunks)libs/schema/src/sections/experience.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- apps/artboard/src/templates/leafish.tsx
- apps/artboard/src/templates/glalie.tsx
- apps/artboard/src/templates/bronzor.tsx
- apps/artboard/src/templates/pikachu.tsx
- apps/artboard/src/templates/kakuna.tsx
- apps/artboard/src/templates/azurill.tsx
- apps/artboard/src/templates/rhyhorn.tsx
- apps/artboard/src/templates/onyx.tsx
- apps/client/src/locales/enums.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx (4)
libs/ui/src/components/form.tsx (5)
FormField(11-20)FormItem(22-32)FormLabel(36-50)FormControl(54-69)FormMessage(91-112)libs/ui/src/components/select.tsx (6)
Select(6-6)SelectTrigger(12-29)SelectValue(10-10)SelectContent(33-60)SelectGroup(8-8)SelectItem(77-96)libs/schema/src/sections/experience.ts (2)
employmentTypeEnum(6-16)workTypeEnum(5-5)apps/client/src/locales/enums.ts (2)
getEmploymentTypeLabels(23-33)getWorkTypeLabels(15-21)
🔇 Additional comments (1)
libs/schema/src/sections/experience.ts (1)
47-48: Defaults align with “opt-in” rendering.Using
"none"avoids unintended badges on new entries. LGTM.
|
@divyanshu-patil Yes, the fixes almost work (I'd say about 99%) pending a minor fix, I reckon. When choosing the "Other" as the option in the drop-down, the text is garbled, probably due to the missing mapping with a Default message. Honestly, I think this option is redundant and could be safely left out. Nice work and fast turn-around time too! Impressive! |
@bimamib As per the current logic, this feature is available when you select the "Other" option, available in both, Work Type and Employment Type drop-downs. Do you have other ideas that should be included? |
…type is selected as other
…value is not other, added i18n in templates
To reproduce the issue, I copied over the changes file-by-file and rebuilt the docker image from scratch issuing the command:
Then I followed the usual flow of logging in via localhost and creating a new entry in the Experience section. BTW, the string "Other" should also be added to the messages.po file for the fixes to properly work |
|
@mithunsridharan thank you for your feedback !
i also added translations for all 54 languages including the "Other" word |
|
@AmruthPillai Sir, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/artboard/src/templates/bronzor.tsx (1)
1-1: Cross-package import may cause architectural issues.The import path
"../../../client/src/locales/enums"crosses package boundaries (artboard → client), which can lead to:
- Tight coupling between packages
- Build/bundling complications in a monorepo
- Fragile relative paths
Consider moving
getEmploymentTypeLabelsandgetWorkTypeLabelsto a shared package like@reactive-resume/utilsor@reactive-resume/schema, or re-exporting them through proper package entry points.Note: This pattern appears in all 8 template files being modified (bronzor, rhyhorn, leafish, kakuna, chikorita, azurill, onyx, nosepass).
Also applies to: 23-23
apps/artboard/src/templates/azurill.tsx (1)
284-309: Consider usingbg-primaryfor consistency.Lines 288 and 301 use
bg-blackfor the separator dots, while other decorative elements in this template (e.g., the Rating component on line 119, section bullets on lines 94/96/200/202) usebg-primary. Consider usingbg-primaryfor visual consistency unlessbg-blackis intentional for this specific context.Otherwise, the conditional rendering logic is correct with proper guards and fallbacks.
apps/artboard/src/templates/gengar.tsx (1)
267-296: Consider guarding the separator dots when adjacent content is empty.The current implementation always renders the separator dot (
•) within the conditional blocks, which can result in awkward displays:
- Lines 267-279: If
item.positionis empty, the output shows• Full-Time(leading separator with no position).- Lines 284-296: If
item.locationis empty, the output showsRemote •(trailing separator with no location).While
positionis typically filled in practice,locationis more commonly optional—especially for remote positions—and the trailing dot becomes noticeable.Consider conditionally rendering the separator only when both the badge field and its adjacent content are present:
For employmentType (lines 267-279):
<div className="flex items-center gap-2"> <span>{item.position}</span> - {item.employmentType && item.employmentType !== "none" && ( + {item.position && item.employmentType && item.employmentType !== "none" && ( <> <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" /> <span> {item.employmentType === "other" ? item.customEmploymentType || i18n._(getEmploymentTypeLabels().other) : i18n._(getEmploymentTypeLabels()[item.employmentType])} </span> </> )} </div>For workType (lines 284-296):
<div className="flex items-center gap-2"> {item.workType && item.workType !== "none" && ( <> <span> {item.workType === "other" ? item.customWorkType || i18n._(getWorkTypeLabels().other) : i18n._(getWorkTypeLabels()[item.workType])} </span> - <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" /> + {item.location && ( + <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" /> + )} </> )} - <span>{item.location}</span> + {item.location && <span>{item.location}</span>} </div>apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx (1)
109-224: Minor style improvements: remove empty className and fix placeholder capitalization.Two small refinements would improve code cleanliness:
Lines 113 and 172: The
className=""props are empty and serve no purpose. Consider removing them.Line 214: The placeholder text uses
"Enter custom Work type"with a capital "W", while line 156 consistently uses lowercase"Enter custom employment type". For consistency, change to lowercase:- name="employmentType" control={form.control} render={({ field }) => ( - <FormItem className=""> + <FormItem>- name="workType" control={form.control} render={({ field }) => ( - <FormItem className=""> + <FormItem>- placeholder={t`Enter custom Work type`} + placeholder={t`Enter custom work type`}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (52)
apps/client/src/locales/af-ZA/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/am-ET/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ar-SA/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/az-AZ/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/bg-BG/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/bn-BD/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ca-ES/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/cs-CZ/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/da-DK/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/de-DE/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/el-GR/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/en-US/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/es-ES/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/fa-IR/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/fi-FI/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/fr-FR/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/he-IL/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/hi-IN/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/hu-HU/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/id-ID/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/it-IT/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ja-JP/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/km-KH/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/kn-IN/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ko-KR/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/lt-LT/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/lv-LV/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ml-IN/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/mr-IN/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ms-MY/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ne-NP/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/nl-NL/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/no-NO/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/or-IN/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/pl-PL/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/pt-BR/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/pt-PT/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ro-RO/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ru-RU/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/sk-SK/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/sq-AL/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/sr-SP/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/sv-SE/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/ta-IN/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/te-IN/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/th-TH/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/tr-TR/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/uk-UA/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/uz-UZ/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/vi-VN/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/zh-CN/messages.pois excluded by!apps/client/src/locales/*/messages.poapps/client/src/locales/zh-TW/messages.pois excluded by!apps/client/src/locales/*/messages.po
📒 Files selected for processing (14)
apps/artboard/src/templates/azurill.tsx(3 hunks)apps/artboard/src/templates/bronzor.tsx(3 hunks)apps/artboard/src/templates/chikorita.tsx(3 hunks)apps/artboard/src/templates/ditto.tsx(3 hunks)apps/artboard/src/templates/gengar.tsx(3 hunks)apps/artboard/src/templates/glalie.tsx(2 hunks)apps/artboard/src/templates/kakuna.tsx(2 hunks)apps/artboard/src/templates/leafish.tsx(2 hunks)apps/artboard/src/templates/nosepass.tsx(2 hunks)apps/artboard/src/templates/onyx.tsx(2 hunks)apps/artboard/src/templates/pikachu.tsx(2 hunks)apps/artboard/src/templates/rhyhorn.tsx(2 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx(3 hunks)libs/schema/src/sections/experience.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/artboard/src/templates/glalie.tsx
- apps/artboard/src/templates/pikachu.tsx
- apps/artboard/src/templates/ditto.tsx
🧰 Additional context used
🧬 Code graph analysis (11)
apps/artboard/src/templates/leafish.tsx (1)
apps/client/src/locales/enums.ts (2)
getEmploymentTypeLabels(23-33)getWorkTypeLabels(15-21)
apps/artboard/src/templates/bronzor.tsx (1)
apps/client/src/locales/enums.ts (2)
getEmploymentTypeLabels(23-33)getWorkTypeLabels(15-21)
apps/artboard/src/templates/onyx.tsx (1)
apps/client/src/locales/enums.ts (2)
getEmploymentTypeLabels(23-33)getWorkTypeLabels(15-21)
apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx (4)
libs/ui/src/components/form.tsx (5)
FormField(11-20)FormItem(22-32)FormLabel(36-50)FormControl(54-69)FormMessage(91-112)libs/ui/src/components/select.tsx (6)
Select(6-6)SelectTrigger(12-29)SelectValue(10-10)SelectContent(33-60)SelectGroup(8-8)SelectItem(77-96)libs/schema/src/sections/experience.ts (2)
employmentTypeEnum(6-16)workTypeEnum(5-5)apps/client/src/locales/enums.ts (2)
getEmploymentTypeLabels(23-33)getWorkTypeLabels(15-21)
apps/artboard/src/templates/gengar.tsx (1)
apps/client/src/locales/enums.ts (2)
getEmploymentTypeLabels(23-33)getWorkTypeLabels(15-21)
apps/artboard/src/templates/azurill.tsx (1)
apps/client/src/locales/enums.ts (2)
getEmploymentTypeLabels(23-33)getWorkTypeLabels(15-21)
libs/schema/src/sections/experience.ts (1)
libs/dto/src/resume/url.ts (1)
urlSchema(4-4)
apps/artboard/src/templates/nosepass.tsx (1)
apps/client/src/locales/enums.ts (2)
getEmploymentTypeLabels(23-33)getWorkTypeLabels(15-21)
apps/artboard/src/templates/kakuna.tsx (1)
apps/client/src/locales/enums.ts (2)
getEmploymentTypeLabels(23-33)getWorkTypeLabels(15-21)
apps/artboard/src/templates/chikorita.tsx (1)
apps/client/src/locales/enums.ts (2)
getEmploymentTypeLabels(23-33)getWorkTypeLabels(15-21)
apps/artboard/src/templates/rhyhorn.tsx (1)
apps/client/src/locales/enums.ts (2)
getEmploymentTypeLabels(23-33)getWorkTypeLabels(15-21)
🔇 Additional comments (8)
apps/artboard/src/templates/bronzor.tsx (1)
263-292: LGTM! Employment and work type rendering is well-implemented.The conditional rendering correctly:
- Guards against undefined/missing fields with truthy checks
- Excludes "none" values from display
- Handles "other" type with custom fallbacks
- Uses internationalized labels consistently
- Provides visual separators for readability
apps/artboard/src/templates/rhyhorn.tsx (1)
261-290: LGTM! Conditional rendering is correctly implemented.The truthy checks properly guard against legacy data where
employmentType/workTypeare undefined, preventing "undefined" from being rendered. The "other" type handling with custom fields is well-structured.Note: The cross-package import architectural concern mentioned for bronzor.tsx also applies here.
apps/artboard/src/templates/leafish.tsx (1)
256-281: LGTM! Implementation follows established pattern.The vertical layout (position → employmentType, then location → workType on separate lines) is appropriate for this template's design. Conditional logic and i18n integration are correct.
apps/artboard/src/templates/kakuna.tsx (1)
261-286: LGTM! Clean implementation with proper guards.The rendering logic correctly handles all cases including legacy data, "none" values, and "other" with custom fields. The vertical layout fits the template design.
apps/artboard/src/templates/chikorita.tsx (1)
247-276: LGTM! Horizontal layout implementation is sound.The split-column rendering (position + employmentType on left, date + workType + location on right) maintains the template's visual structure while correctly integrating the new fields.
apps/artboard/src/templates/onyx.tsx (1)
261-290: LGTM! Implementation is consistent and correct.The horizontal layout with proper conditional rendering and i18n labels integrates well with the template's existing structure.
apps/artboard/src/templates/nosepass.tsx (1)
312-337: LGTM! Legacy data handling is now correct.The truthy checks (
item.employmentType &&anditem.workType &&) properly prevent rendering "undefined" for older resume entries. The implementation correctly integrates with the template's vertical layout and date-based structure.libs/schema/src/sections/experience.ts (1)
5-66: LGTM! Schema design and validation are well-structured.The schema changes implement the new employment and work type features correctly:
- Enums include appropriate values plus "none" (for hiding) and "other" (for custom input)
- Fields are optional, ensuring backward compatibility with existing resumes
- The
.superRefine()validation properly enforces that custom fields are non-empty (using.trim()) when "other" is selected- Default values of
"none"align with the opt-in behavior described in the PR objectives
|
@mithunsridharan NX apps/client/src/locales/enums.tsx (12:9): "t" is not exported by "node_modules/.pnpm/@[email protected]_@[email protected][email protected][email protected][email protected]/node_modules/@lingui/macro/dist/index.mjs", imported by "apps/client/src/locales/enums.tsx".
76.47
76.47 file: /app/apps/client/src/locales/enums.tsx:12:9
76.47
76.47 10: */
76.47 11:
76.47 12: import { t } from "@lingui/macro";
76.47 ^
76.47 13: import type { EmploymentTypeEnum, WorkTypeEnum } from "@reactive-resume/schema"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/artboard/src/templates/azurill.tsx(1 hunks)apps/artboard/src/templates/bronzor.tsx(1 hunks)apps/artboard/src/templates/chikorita.tsx(1 hunks)apps/artboard/src/templates/ditto.tsx(1 hunks)apps/artboard/src/templates/gengar.tsx(2 hunks)apps/artboard/src/templates/glalie.tsx(1 hunks)apps/artboard/src/templates/kakuna.tsx(1 hunks)apps/artboard/src/templates/leafish.tsx(1 hunks)apps/artboard/src/templates/nosepass.tsx(1 hunks)apps/artboard/src/templates/onyx.tsx(1 hunks)apps/artboard/src/templates/pikachu.tsx(1 hunks)apps/artboard/src/templates/rhyhorn.tsx(1 hunks)apps/client/src/locales/enums.i18n.ts(1 hunks)apps/client/src/locales/enums.tsx(1 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/client/src/locales/enums.i18n.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- apps/artboard/src/templates/glalie.tsx
- apps/artboard/src/templates/chikorita.tsx
- apps/artboard/src/templates/leafish.tsx
- apps/artboard/src/templates/kakuna.tsx
- apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx
- apps/artboard/src/templates/nosepass.tsx
- apps/artboard/src/templates/azurill.tsx
- apps/artboard/src/templates/gengar.tsx
- apps/artboard/src/templates/rhyhorn.tsx
- apps/artboard/src/templates/bronzor.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/client/src/locales/enums.tsx (1)
libs/schema/src/sections/experience.ts (2)
WorkTypeEnum(18-18)EmploymentTypeEnum(19-19)
| <div className="flex items-center gap-2"> | ||
| <span>{item.position}</span> | ||
| {item.employmentType && item.employmentType !== "none" && ( | ||
| <> | ||
| <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" /> | ||
| <span> | ||
| {item.employmentType === "other" && item.customEmploymentType | ||
| ? item.customEmploymentType | ||
| : item.employmentType}{" "} | ||
| </span> | ||
| </> | ||
| )} | ||
| </div> | ||
| </div> | ||
|
|
||
| <div className="shrink-0 text-right"> | ||
| <div className="font-bold">{item.date}</div> | ||
| <div>{item.location}</div> | ||
| <div className="flex items-center gap-2"> | ||
| {item.workType && item.workType !== "none" && ( | ||
| <> | ||
| <span> | ||
| {item.workType === "other" && item.customWorkType | ||
| ? item.customWorkType | ||
| : item.workType}{" "} | ||
| </span> | ||
| <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" /> | ||
| </> | ||
| )} | ||
| <span>{item.location}</span> | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Localize employment/work type badges in Ditto
Ditto displays the raw enum tokens, so non-English resumes show English text (or other). Please translate the labels (and fall back to the localized “Other”) before rendering.
@@
-import type {
+import type {
Award,
Certification,
CustomSection,
CustomSectionGroup,
+ EmploymentTypeEnum,
Interest,
Language,
Profile,
Project,
Publication,
Reference,
SectionKey,
SectionWithItem,
Skill,
URL,
+ WorkTypeEnum,
} from "@reactive-resume/schema";
import { Education, Experience, Volunteer } from "@reactive-resume/schema";
import { cn, isEmptyString, isUrl, sanitize } from "@reactive-resume/utils";
import get from "lodash.get";
-import { Fragment } from "react";
+import { Fragment, useMemo } from "react";
+import { msg } from "@lingui/macro";
+import { useLingui } from "@lingui/react";
@@
-const Experience = () => {
- const section = useArtboardStore((state) => state.resume.sections.experience);
-
- return (
- <Section<Experience> section={section} urlKey="url" summaryKey="summary">
- {(item) => (
+const Experience = () => {
+ const section = useArtboardStore((state) => state.resume.sections.experience);
+ const { _ } = useLingui();
+ const employmentTypeLabels = useMemo<Record<EmploymentTypeEnum, string>>(
+ () => ({
+ "Full-Time": _(msg`Full-Time`),
+ "Part-Time": _(msg`Part-Time`),
+ Internship: _(msg`Internship`),
+ Contract: _(msg`Contract`),
+ Freelance: _(msg`Freelance`),
+ Temporary: _(msg`Temporary`),
+ Volunteer: _(msg`Volunteer`),
+ none: _(msg`Not specified`),
+ other: _(msg`Other`),
+ }),
+ [_],
+ );
+ const workTypeLabels = useMemo<Record<WorkTypeEnum, string>>(
+ () => ({
+ "On-Site": _(msg`On-Site`),
+ Remote: _(msg`Remote`),
+ Hybrid: _(msg`Hybrid`),
+ none: _(msg`Not specified`),
+ other: _(msg`Other`),
+ }),
+ [_],
+ );
+
+ return (
+ <Section<Experience> section={section} urlKey="url" summaryKey="summary">
+ {(item) => {
+ const employmentLabel =
+ item.employmentType && item.employmentType !== "none"
+ ? item.employmentType === "other"
+ ? item.customEmploymentType?.trim() || employmentTypeLabels.other
+ : employmentTypeLabels[item.employmentType]
+ : undefined;
+ const workLabel =
+ item.workType && item.workType !== "none"
+ ? item.workType === "other"
+ ? item.customWorkType?.trim() || workTypeLabels.other
+ : workTypeLabels[item.workType]
+ : undefined;
+
+ return (
<div className="flex items-start justify-between group-[.sidebar]:flex-col group-[.sidebar]:items-start">
@@
- <span>{item.position}</span>
- {item.employmentType && item.employmentType !== "none" && (
- <>
- <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" />
- <span>
- {item.employmentType === "other" && item.customEmploymentType
- ? item.customEmploymentType
- : item.employmentType}{" "}
- </span>
- </>
- )}
+ <span>{item.position}</span>
+ {employmentLabel && (
+ <>
+ <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" />
+ <span>{employmentLabel}</span>
+ </>
+ )}
@@
- {item.workType && item.workType !== "none" && (
- <>
- <span>
- {item.workType === "other" && item.customWorkType
- ? item.customWorkType
- : item.workType}{" "}
- </span>
- <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" />
- </>
- )}
+ {workLabel && (
+ <>
+ <span>{workLabel}</span>
+ <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" />
+ </>
+ )}
<span>{item.location}</span>
</div>
</div>
- )}
+ );
+ }}
</Section>
);
};Committable suggestion skipped: line range outside the PR's diff.
| <div className="flex items-center gap-2"> | ||
| <span>{item.position}</span> | ||
| {item.employmentType && item.employmentType !== "none" && ( | ||
| <> | ||
| <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" /> | ||
| <span> | ||
| {item.employmentType === "other" && item.customEmploymentType | ||
| ? item.customEmploymentType | ||
| : item.employmentType}{" "} | ||
| </span> | ||
| </> | ||
| )} | ||
| </div> | ||
| </div> | ||
|
|
||
| <div className="shrink-0 text-right"> | ||
| <div className="font-bold">{item.date}</div> | ||
| <div>{item.location}</div> | ||
| <div className="flex items-center gap-2"> | ||
| {item.workType && item.workType !== "none" && ( | ||
| <> | ||
| <span> | ||
| {item.workType === "other" && item.customWorkType | ||
| ? item.customWorkType | ||
| : item.workType}{" "} | ||
| </span> | ||
| <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" /> | ||
| </> | ||
| )} | ||
| <span>{item.location}</span> | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Localize employment/work type labels before rendering
Raw enum values ("Full-Time", "Remote", "other") are leaking into the resume. That breaks localization and literally prints other when no custom text is provided. Resolve these badges through Lingui so every locale gets the right copy.
@@
-import type {
+import type {
Award,
Certification,
CustomSection,
CustomSectionGroup,
+ EmploymentTypeEnum,
Interest,
Language,
Project,
Publication,
Reference,
SectionKey,
SectionWithItem,
Skill,
URL,
+ WorkTypeEnum,
} from "@reactive-resume/schema";
import { Education, Experience, Volunteer } from "@reactive-resume/schema";
-import { cn, isEmptyString, isUrl, sanitize } from "@reactive-resume/utils";
+import { msg } from "@lingui/macro";
+import { useLingui } from "@lingui/react";
+import { cn, isEmptyString, isUrl, sanitize } from "@reactive-resume/utils";
@@
-const Experience = () => {
- const section = useArtboardStore((state) => state.resume.sections.experience);
-
- return (
- <Section<Experience> section={section} urlKey="url" summaryKey="summary">
- {(item) => (
+const Experience = () => {
+ const section = useArtboardStore((state) => state.resume.sections.experience);
+ const { _ } = useLingui();
+ const employmentTypeLabels = React.useMemo<Record<EmploymentTypeEnum, string>>(
+ () => ({
+ "Full-Time": _(msg`Full-Time`),
+ "Part-Time": _(msg`Part-Time`),
+ Internship: _(msg`Internship`),
+ Contract: _(msg`Contract`),
+ Freelance: _(msg`Freelance`),
+ Temporary: _(msg`Temporary`),
+ Volunteer: _(msg`Volunteer`),
+ none: _(msg`Not specified`),
+ other: _(msg`Other`),
+ }),
+ [_],
+ );
+ const workTypeLabels = React.useMemo<Record<WorkTypeEnum, string>>(
+ () => ({
+ "On-Site": _(msg`On-Site`),
+ Remote: _(msg`Remote`),
+ Hybrid: _(msg`Hybrid`),
+ none: _(msg`Not specified`),
+ other: _(msg`Other`),
+ }),
+ [_],
+ );
+
+ return (
+ <Section<Experience> section={section} urlKey="url" summaryKey="summary">
+ {(item) => {
+ const employmentLabel =
+ item.employmentType && item.employmentType !== "none"
+ ? item.employmentType === "other"
+ ? item.customEmploymentType?.trim() || employmentTypeLabels.other
+ : employmentTypeLabels[item.employmentType]
+ : undefined;
+ const workLabel =
+ item.workType && item.workType !== "none"
+ ? item.workType === "other"
+ ? item.customWorkType?.trim() || workTypeLabels.other
+ : workTypeLabels[item.workType]
+ : undefined;
+
+ return (
<div className="flex items-start justify-between">
@@
- <span>{item.position}</span>
- {item.employmentType && item.employmentType !== "none" && (
- <>
- <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" />
- <span>
- {item.employmentType === "other" && item.customEmploymentType
- ? item.customEmploymentType
- : item.employmentType}{" "}
- </span>
- </>
- )}
+ <span>{item.position}</span>
+ {employmentLabel && (
+ <>
+ <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" />
+ <span>{employmentLabel}</span>
+ </>
+ )}
@@
- {item.workType && item.workType !== "none" && (
- <>
- <span>
- {item.workType === "other" && item.customWorkType
- ? item.customWorkType
- : item.workType}{" "}
- </span>
- <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" />
- </>
- )}
+ {workLabel && (
+ <>
+ <span>{workLabel}</span>
+ <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" />
+ </>
+ )}
<span>{item.location}</span>
</div>
</div>
- )}
+ );
+ }}
</Section>
);
};🤖 Prompt for AI Agents
In apps/artboard/src/templates/onyx.tsx around lines 259 to 288, employmentType
and workType enum values are rendered raw; replace those with Lingui
translations. Import Lingui's t (or use <Trans>) and map each enum value to a
localized string (e.g., t`fullTime`, t`partTime`, t`remote`, etc.), and for the
"other" case render the localized label only if
customEmploymentType/customWorkType is absent (use a localized "Other" string as
fallback). Ensure you still skip "none", update both the employmentType and
workType branches, and keep existing DOM structure while calling the translation
function before rendering.
| <div className="flex items-center gap-2"> | ||
| <span>{item.position}</span> | ||
| {item.employmentType && item.employmentType !== "none" && ( | ||
| <> | ||
| <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" /> | ||
| <span> | ||
| {item.employmentType === "other" && item.customEmploymentType | ||
| ? item.customEmploymentType | ||
| : item.employmentType}{" "} | ||
| </span> | ||
| </> | ||
| )} | ||
| </div> | ||
| </div> | ||
|
|
||
| <div className="shrink-0 text-right group-[.sidebar]:text-left"> | ||
| <div className="font-bold">{item.date}</div> | ||
| <div>{item.location}</div> | ||
| <div className="flex items-center gap-2"> | ||
| {item.workType && item.workType !== "none" && ( | ||
| <> | ||
| <span> | ||
| {item.workType === "other" && item.customWorkType | ||
| ? item.customWorkType | ||
| : item.workType}{" "} | ||
| </span> | ||
| <span className="inline-block aspect-square h-1.5 rounded-full bg-primary" /> | ||
| </> | ||
| )} | ||
| <span>{item.location}</span> | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Localize employment & work type badges
The resume payload stores these fields as enum keys (e.g., full_time, contract, or even the Lingui message id when a translation is missing). Rendering {item.employmentType} / {item.workType} directly leaks those raw keys onto the template, so non-English resumes (or locales missing a string) will show values like builder.experience.workType.hybrid instead of a human label. Please resolve the localized label via the same helper/map the builder uses (while still honoring custom* when present) before rendering.
🤖 Prompt for AI Agents
In apps/artboard/src/templates/pikachu.tsx around lines 293 to 322 the template
is rendering raw enum keys for employmentType and workType which can expose
message ids; resolve those to localized labels using the same helper/map the
builder uses (import the map/lookup function), use the localized label when
available, but still prefer customEmploymentType/customWorkType when present,
and fall back to a safe humanized or the original key only if no translation
exists; replace direct {item.employmentType} and {item.workType} usages with the
resolved label variables before rendering.
That's interesting! On my Docker environment, the strings work only when the mapping is available in the locales (messages.po)! |
Here's my build script for the Docker environment that I'm using:
I copied it to what I call Makefile and when I source it, this script builds a scratch Docker image and instance. |
|
Can you verify by testing this entire pull request rather than copying files Manually |
I'm working on a separate fork, so merging these changes will be challenging. Nevertheless, the messages.po in en-US file should contain all the strings required for successful rendering, which happens at my end.
I'll check out your commits tomorrow. For some extra credits, check out the Issues queue where somebody wanted to add the Cover Letter feature. That's a feature worthy of Hacktoberfest (why not HackerDiwali?)! |
Well, overall it's good, and it does need that feature so that people know what kind of work they have done before. Because in my country, Indonesia, there are activities provided by the government such as "Kampus Merdeka", where there are various activities such as "Internships, MSIB, etc.". So I need a job type that can be typed in, such as the one I am currently doing. Thank you to Mr. @divyanshu-patil for implementing this, and thank you to Mr. @mithunsridharan for helping to answer. We are just waiting for approval from Mr. @AmruthPillai . For now, that is all I have in terms of ideas, Mr. @mithunsridharan. If any other ideas come to mind later, I may submit a request for new features that could potentially be useful to other users. |
|
@mithunsridharan sure sir |




this PR adds feature requested in #2395
adds the work type and employment type option in the resume
changes:

note: users can select option as none to hide it in the resume
currently style is kept basic, please provide feedback over styling
Summary by CodeRabbit