release: v4.3.1 #2140
release: v4.3.1 #2140
Conversation
|
Warning Rate limit exceeded@AmruthPillai has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 38 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe pull request encompasses multiple files across different sections of the application, focusing on enhancing form interactions, improving accessibility, and refining UI components. Key changes include updating form state synchronization in various dialogs, transitioning from context menus to dropdown menus, adding ref forwarding, and introducing coercion for number inputs in schemas. Changes
Assessment against linked issues
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
View your CI Pipeline Execution ↗ for commit 3b73dcf.
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (15)
apps/client/src/pages/builder/sidebars/left/dialogs/volunteer.tsx (1)
118-118: Consider documenting thetrueparameter insetContent.The second parameter
trueineditor.commands.setContent(value, true)is important for preserving the editor's history stack. Consider adding a brief comment explaining its purpose for maintainability.- editor.commands.setContent(value, true); + // Preserve editor history stack for undo/redo functionality + editor.commands.setContent(value, true);apps/client/src/pages/builder/sidebars/left/sections/summary.tsx (1)
35-41: Consider memoizing the onChange handler.The current implementation creates a new function on every render. Consider memoizing the handler using useCallback to optimize performance:
+ import { useCallback } from 'react'; export const SummarySection = () => { const setValue = useResumeStore((state) => state.setValue); + const handleAiChange = useCallback((editor) => (value) => { + editor.commands.setContent(value, true); + setValue("sections.summary.content", value); + }, [setValue]); return ( // ... <RichInput footer={(editor) => ( <AiActions value={editor.getText()} - onChange={(value) => { - editor.commands.setContent(value, true); - setValue("sections.summary.content", value); - }} + onChange={handleAiChange(editor)} /> )} /> ); };apps/client/src/pages/builder/sidebars/left/sections/shared/section-list-item.tsx (1)
Line range hint
77-77: Update cursor style to match new interaction model.The class
cursor-context-menusuggests right-click interaction, which doesn't align with the new dropdown menu behavior.-"flex-1 cursor-context-menu p-4 hover:bg-secondary-accent", +"flex-1 cursor-pointer p-4 hover:bg-secondary-accent",apps/client/src/pages/builder/sidebars/left/sections/shared/section-base.tsx (1)
Line range hint
134-146: Consider memoizing event handler callbacks.The event handlers in the map callback are recreated on each render. Consider optimizing performance by memoizing these callbacks or moving them outside the map function.
Here's how you could optimize it:
{section.items.map((item, index) => { + const handleUpdate = () => onUpdate(item as T); + const handleDelete = () => onDelete(item as T); + const handleDuplicate = () => onDuplicate(item as T); + const handleToggleVisibility = () => onToggleVisibility(index); + return ( <SectionListItem key={item.id} id={item.id} visible={item.visible} title={title(item as T)} description={description?.(item as T)} - onUpdate={() => { - onUpdate(item as T); - }} - onDelete={() => { - onDelete(item as T); - }} - onDuplicate={() => { - onDuplicate(item as T); - }} - onToggleVisibility={() => { - onToggleVisibility(index); - }} + onUpdate={handleUpdate} + onDelete={handleDelete} + onDuplicate={handleDuplicate} + onToggleVisibility={handleToggleVisibility} /> ); })}For even better performance, you could use
useCallbackif these handlers are passed to memoized child components:const handleUpdate = useCallback((item: T) => onUpdate(item), [onUpdate]); const handleDelete = useCallback((item: T) => onDelete(item), [onDelete]); const handleDuplicate = useCallback((item: T) => onDuplicate(item), [onDuplicate]);apps/client/src/pages/builder/sidebars/left/dialogs/languages.tsx (1)
77-80: LGTM! Consider adding aria-label for better screen reader support.The conditional rendering logic is clear and consistent. The level is now properly displayed when greater than 0, and "Hidden" is shown when the level is 0.
Consider adding an aria-label to improve screen reader experience:
- {field.value > 0 ? ( - <span className="text-base font-bold">{field.value}</span> - ) : ( - <span className="text-base font-bold">{t`Hidden`}</span> - )} + {field.value > 0 ? ( + <span className="text-base font-bold" aria-label={t`Language proficiency level: ${field.value}`}>{field.value}</span> + ) : ( + <span className="text-base font-bold" aria-label={t`Language proficiency level: Hidden`}>{t`Hidden`}</span> + )}apps/client/src/pages/builder/sidebars/left/dialogs/skills.tsx (1)
91-94: LGTM! Consider accessibility and performance improvements.The conditional rendering logic matches the languages dialog, maintaining consistency across the application.
Consider these improvements:
- Add aria-label for better screen reader support:
- {field.value > 0 ? ( - <span className="text-base font-bold">{field.value}</span> - ) : ( - <span className="text-base font-bold">{t`Hidden`}</span> - )} + {field.value > 0 ? ( + <span className="text-base font-bold" aria-label={t`Skill level: ${field.value}`}>{field.value}</span> + ) : ( + <span className="text-base font-bold" aria-label={t`Skill level: Hidden`}>{t`Hidden`}</span> + )}
- Consider memoizing the AnimatePresence children to optimize performance when there are many keywords:
const AnimatedBadge = memo(({ item, index, onRemove }) => ( <motion.div key={item} layout initial={{ opacity: 0, y: -50 }} animate={{ opacity: 1, y: 0, transition: { delay: index * 0.1 } }} exit={{ opacity: 0, x: -50 }} > <Badge className="cursor-pointer" onClick={() => onRemove(item)} > <span className="mr-1">{item}</span> <X size={12} weight="bold" /> </Badge> </motion.div> ));apps/client/src/pages/dashboard/settings/_sections/openai.tsx (3)
42-43: Consider adding Ollama API key validation.The simplified
isEnabledlogic correctly depends only onapiKey. However, since we now support both OpenAI and Ollama, consider updating the API key validation to accommodate Ollama's format.const formSchema = z.object({ apiKey: z .string() - .regex(/^sk-.+$/, "That doesn't look like a valid OpenAI API key.") + .regex(/^(sk-.+|sk-\d{16})$/, "That doesn't look like a valid OpenAI/Ollama API key.") .default(""), // ... rest of the schema });
104-107: Enhance Ollama setup instructions.Consider improving the Ollama setup instructions by:
- Making the example API key less likely to be copied directly
- Adding a link to Ollama's documentation
- You can also integrate with Ollama simply by setting the API key to - `sk-1234567890abcdef` and the Base URL to your Ollama URL, i.e. - `http://localhost:11434/v1`. You can also pick and choose models and set the max tokens - as per your preference. + You can also integrate with <a href="https://ollama.ai/download" target="_blank" rel="noopener noreferrer">Ollama</a> by: + 1. Setting the API key to any 16-digit value prefixed with `sk-` (e.g., `sk-0000000000000000`) + 2. Setting the Base URL to your Ollama endpoint (default: `http://localhost:11434/v1`) + 3. Choosing from available <a href="https://ollama.ai/library" target="_blank" rel="noopener noreferrer">Ollama models</a>
Line range hint
173-216: Update disclaimer to include Ollama.The disclaimer section only mentions OpenAI's terms and privacy policy. Consider adding a similar disclaimer for Ollama or making it generic for both services.
<Trans> <span className="font-medium">Note: </span> - By utilizing the OpenAI API, you acknowledge and accept the{" "} + By utilizing the OpenAI or Ollama APIs, you acknowledge and accept their respective terms of use + and privacy policies ({" "} <a href="https://openai.com/policies/terms-of-use" rel="noopener noreferrer nofollow" target="_blank" > - terms of use + OpenAI Terms </a>{" "} - and{" "} + |{" "} <a href="https://openai.com/policies/privacy-policy" rel="noopener noreferrer nofollow" target="_blank" > - privacy policy + Privacy </a>{" "} + |{" "} + <a + href="https://github.com/ollama/ollama/blob/main/LICENSE" + rel="noopener noreferrer nofollow" + target="_blank" + > + Ollama License + </a> ). Please note that Reactive Resume bears no responsibility for any improper or unauthorized utilization of the service, and any resulting repercussions or liabilities solely rest on the user. </Trans>apps/client/src/pages/builder/sidebars/left/dialogs/references.tsx (1)
87-93: LGTM! Consider documenting the setContent parameterThe implementation correctly synchronizes the editor content with the form state. However, consider adding a comment explaining the purpose of the
trueparameter insetContent(value, true).onChange={(value) => { + // The second parameter (true) prevents the editor from creating a new history entry editor.commands.setContent(value, true); field.onChange(value); }}apps/client/src/pages/builder/sidebars/left/dialogs/certifications.tsx (1)
101-107: Consider extracting the common AiActions patternWhile the implementation is correct, this pattern is duplicated across multiple dialog components. Consider extracting it into a reusable hook or component.
Example implementation:
// hooks/useAiActionsHandler.ts export const useAiActionsHandler = (editor: Editor, field: ControllerRenderProps) => { return useCallback( (value: string) => { editor.commands.setContent(value, true); field.onChange(value); }, [editor, field] ); }; // Usage in dialog components: const handleAiChange = useAiActionsHandler(editor, field); <AiActions value={editor.getText()} onChange={handleAiChange} />apps/client/src/pages/builder/sidebars/left/dialogs/profiles.tsx (1)
54-54: Consider using more inclusive placeholder examples.While GitHub is commonly used, consider using more generic examples or multiple examples to better indicate that users can add any social network profile. This would make the form more intuitive for users who want to add other platforms like LinkedIn, Twitter, etc.
-<Input {...field} placeholder="GitHub" /> +<Input {...field} placeholder="GitHub, LinkedIn, Twitter, etc." /> -<URLInput {...field} placeholder="https://github.com/johndoe" /> +<URLInput {...field} placeholder="https://github.com/johndoe or https://linkedin.com/in/johndoe" /> -placeholder="github" +placeholder="github, linkedin, twitter"Also applies to: 82-82, 103-103
apps/client/src/pages/builder/sidebars/left/dialogs/education.tsx (1)
144-150: Consider extracting the common RichInput pattern.The same
RichInputconfiguration with AI actions appears in multiple dialog components. Consider extracting this into a reusable component to reduce duplication and maintain consistency.Example implementation:
// components/RichInputWithAI.tsx interface RichInputWithAIProps extends RichInputProps { field: ControllerRenderProps; } export const RichInputWithAI: React.FC<RichInputWithAIProps> = ({ field, ...props }) => ( <RichInput {...field} {...props} content={field.value} footer={(editor) => ( <AiActions value={editor.getText()} onChange={(value) => { editor.commands.setContent(value, true); field.onChange(value); }} /> )} onChange={(value) => { field.onChange(value); }} /> );apps/client/src/pages/builder/sidebars/left/dialogs/projects.tsx (1)
114-120: LGTM! Proper form state synchronization implemented.The implementation correctly synchronizes the form state with AI-generated content by calling both
editor.commands.setContentandfield.onChange. This ensures data consistency when using AI actions.Consider extracting this pattern into a custom hook (e.g.,
useRichInputSync) since it's being reused across multiple dialog components, as evident from the AI summary.apps/client/src/pages/builder/sidebars/left/sections/custom/section.tsx (1)
56-58: Add error handling for icon loadingConsider adding error handling for cases where the icon fails to load or doesn't exist:
- {field.icon ? <i className={cn(`ph ph-${field.icon}`)} /> : <Envelope />} + {field.icon ? ( + <i className={cn(`ph ph-${field.icon}`)} onError={() => handleChange("icon", "")} /> + ) : ( + <Envelope /> + )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
apps/client/src/pages/builder/sidebars/left/dialogs/awards.tsx(1 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/certifications.tsx(1 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/custom-section.tsx(1 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/education.tsx(1 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx(1 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/languages.tsx(1 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/profiles.tsx(3 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/projects.tsx(1 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/publications.tsx(1 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/references.tsx(1 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/skills.tsx(1 hunks)apps/client/src/pages/builder/sidebars/left/dialogs/volunteer.tsx(1 hunks)apps/client/src/pages/builder/sidebars/left/sections/custom/section.tsx(5 hunks)apps/client/src/pages/builder/sidebars/left/sections/shared/section-base.tsx(1 hunks)apps/client/src/pages/builder/sidebars/left/sections/shared/section-dialog.tsx(3 hunks)apps/client/src/pages/builder/sidebars/left/sections/shared/section-list-item.tsx(3 hunks)apps/client/src/pages/builder/sidebars/left/sections/summary.tsx(1 hunks)apps/client/src/pages/dashboard/resumes/_layouts/grid/_components/base-card.tsx(2 hunks)apps/client/src/pages/dashboard/resumes/_layouts/grid/_components/resume-card.tsx(4 hunks)apps/client/src/pages/dashboard/settings/_sections/openai.tsx(6 hunks)apps/server/src/mail/mail.service.ts(1 hunks)libs/schema/src/sections/language.ts(1 hunks)libs/schema/src/sections/skill.ts(1 hunks)package.json(2 hunks)
🔇 Additional comments (21)
apps/client/src/pages/builder/sidebars/left/dialogs/volunteer.tsx (1)
115-121: LGTM! Proper synchronization between editor and form state.The implementation correctly handles both the editor content update and form state management, ensuring consistency when AI actions are triggered.
apps/client/src/pages/builder/sidebars/left/sections/summary.tsx (1)
35-41: LGTM! Good state synchronization approach.The implementation ensures proper synchronization between the editor's content and form state, maintaining consistency with similar changes across dialog components.
libs/schema/src/sections/language.ts (1)
9-9: LGTM! Good fix for the slider validation issue.Adding coercion to the level field is a good solution that will automatically handle string-to-number conversion while maintaining the validation constraints. This should resolve the slider component validation errors mentioned in issue #2139.
libs/schema/src/sections/skill.ts (1)
9-9: LGTM! Consistent with language schema changes.Good to see the same coercion approach being applied consistently across similar schemas. This maintains a uniform behavior for level fields throughout the application.
apps/client/src/pages/builder/sidebars/left/sections/shared/section-list-item.tsx (2)
6-11: LGTM! Import changes align with accessibility improvements.The replacement of ContextMenu imports with DropdownMenu components aligns with the PR objective to enhance accessibility.
Line range hint
72-103: LGTM! Excellent accessibility improvements.The conversion from ContextMenu to DropdownMenu enhances accessibility by:
- Supporting keyboard navigation
- Removing the requirement for right-click interactions
- Maintaining all existing functionality in a more accessible way
apps/client/src/pages/builder/sidebars/left/sections/shared/section-base.tsx (1)
73-84: LGTM! Well-structured event handlers.The new event handlers are well-implemented with consistent patterns and proper type safety.
apps/client/src/pages/builder/sidebars/left/sections/shared/section-dialog.tsx (1)
4-4: LGTM! Good accessibility improvement.The addition of
VisuallyHiddenandDialogDescriptioncomponents from Radix UI demonstrates a commitment to improving accessibility.Also applies to: 18-18
package.json (1)
4-4: LGTM! Version bump and dependency addition look good.The version bump to 4.3.1 and addition of
@radix-ui/react-visually-hiddendependency are properly implemented.Also applies to: 169-169
apps/client/src/pages/dashboard/settings/_sections/openai.tsx (1)
31-31: LGTM! Schema change allows flexibility for both OpenAI and Ollama.The addition of
.or(z.literal(""))correctly allows empty baseURL for OpenAI while maintaining validation for Ollama URLs.apps/client/src/pages/builder/sidebars/left/dialogs/publications.tsx (1)
101-107: LGTM! Consistent implementation across dialogsThe implementation maintains consistency with other dialog components, which is excellent for code maintainability.
apps/client/src/pages/builder/sidebars/left/dialogs/awards.tsx (1)
107-113: LGTM! Well-implemented form state synchronization.The implementation correctly ensures that both the editor's content and form state are updated when AI-generated content is applied, maintaining consistency in the application state.
apps/client/src/pages/builder/sidebars/left/dialogs/experience.tsx (1)
120-126: LGTM! Consistent implementation across components.The changes maintain consistency with other dialog components, following the same pattern of synchronizing editor content with form state.
apps/client/src/pages/builder/sidebars/left/dialogs/education.tsx (1)
144-150: LGTM! Consistent implementation.The changes maintain consistency with other dialog components in handling AI-generated content.
apps/client/src/pages/builder/sidebars/left/dialogs/custom-section.tsx (1)
134-140: LGTM! Consistent implementation across dialog components.The implementation follows the same pattern as other dialog components, ensuring proper synchronization between the editor's content and form state when using AI actions.
This implementation mirrors the pattern seen in projects.tsx and other dialog components. The earlier suggestion about extracting this pattern into a custom hook applies here as well.
apps/client/src/pages/dashboard/resumes/_layouts/grid/_components/base-card.tsx (1)
3-3: LGTM! Well-structured implementation of ref forwarding.The implementation correctly uses
forwardRefto pass refs to the underlyingCardcomponent, which is essential for the DropdownMenu integration. The type safety is maintained with proper TypeScript annotations.Also applies to: 14-28
apps/client/src/pages/dashboard/resumes/_layouts/grid/_components/resume-card.tsx (2)
12-16: LGTM! Clean import declarations.The imports are properly organized and specifically import only the required DropdownMenu components.
Line range hint
60-124: LGTM! Improved accessibility with DropdownMenu.The transition from ContextMenu to DropdownMenu enhances accessibility by:
- Making the menu accessible via both click and keyboard navigation
- Maintaining all existing functionality while improving usability
- Using appropriate cursor styling with
cursor-context-menuapps/client/src/pages/builder/sidebars/left/sections/custom/section.tsx (3)
43-43: Improved button accessibility with ghost variantThe change from
variant="link"tovariant="ghost"for icon buttons provides better visual feedback and hover states, improving accessibility.Also applies to: 55-55, 106-106
60-82: Enhanced user guidance with icon selection helpGood addition of the help text and link to Phosphor Icons. This improves the user experience by providing clear guidance on available icons.
130-130: Consider adding icon validationWhile setting "envelope" as the default icon is a good UX improvement, consider adding validation to ensure the icon exists in the Phosphor Icons set.
Run this script to verify the icon's existence:
apps/client/src/pages/builder/sidebars/left/sections/shared/section-dialog.tsx
Show resolved
Hide resolved
apps/client/src/pages/dashboard/resumes/_layouts/grid/_components/resume-card.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.eslintrc.json (1)
12-12: Consider environment-specific ESLint configuration.While removing console statements is good for production, having them error during development might hinder debugging. Consider using environment variables or separate ESLint configurations:
- "no-console": "error", + "no-console": ["error", { + "allow": process.env.NODE_ENV === "development" ? ["warn", "error"] : [] + }],This would:
- Allow
console.warnandconsole.errorduring development- Maintain strict enforcement in production
- Provide a better developer experience while ensuring clean production code
apps/server/src/printer/printer.service.ts (1)
Line range hint
91-209: Consider breaking down generateResume into smaller functionsThe
generateResumefunction is handling multiple concerns and is quite long (200+ lines). This makes it harder to maintain and test.Consider breaking it down into smaller, focused functions:
setupBrowserPage: Handle browser setup and configurationgeneratePdfPages: Handle PDF generation for each pageembedFonts: Handle font loading and embeddingmergePdfPages: Handle merging of PDF pagessaveAndUpload: Handle storage operationsExample structure:
private async setupBrowserPage(resume: ResumeDto) { const browser = await this.getBrowser(); const page = await browser.newPage(); // ... setup code ... return { browser, page }; } private async generatePdfPages(page: Page, numberPages: number) { const pagesBuffer: Buffer[] = []; // ... PDF generation code ... return pagesBuffer; } private async embedFonts(pdf: PDFDocument, fontData: FontData) { // ... font embedding code ... } private async mergePdfPages(pdf: PDFDocument, pagesBuffer: Buffer[]) { // ... PDF merging code ... return pdf; } private async saveAndUpload(buffer: Buffer, resume: ResumeDto) { // ... storage code ... return resumeUrl; } async generateResume(resume: ResumeDto) { try { const { browser, page } = await this.setupBrowserPage(resume); const pagesBuffer = await this.generatePdfPages(page, resume.data.metadata.layout.length); const pdf = await PDFDocument.create(); pdf.registerFontkit(fontkit); await this.embedFonts(pdf, resume.data.metadata.typography.font); await this.mergePdfPages(pdf, pagesBuffer); const buffer = Buffer.from(await pdf.save()); const resumeUrl = await this.saveAndUpload(buffer, resume); await page.close(); await browser.disconnect(); return resumeUrl; } catch (error) { this.logger.error('Failed to generate resume', { userId: resume.userId, resumeId: resume.id, error: error instanceof Error ? error.message : String(error), }); throw new InternalServerErrorException( ErrorMessage.ResumeGenerationFailed, error instanceof Error ? error.message : String(error), ); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.eslintrc.json(1 hunks)apps/client/src/libs/lingui.ts(1 hunks)apps/server/src/printer/printer.service.ts(1 hunks)
🔇 Additional comments (1)
.eslintrc.json (1)
12-12: Verify existing console usage before enforcing error level.The stricter
"no-console": "error"rule will break builds if there are any existing console statements in TypeScript files. Let's verify the impact:
Summary by CodeRabbit
Release Notes v4.3.1
New Features
Improvements
Bug Fixes
Chores