-
Notifications
You must be signed in to change notification settings - Fork 47
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
[Feature] Create new project from the project dropdown #3360
base: develop
Are you sure you want to change the base?
[Feature] Create new project from the project dropdown #3360
Conversation
WalkthroughThe changes in this pull request involve modifications to several components related to task management in the web application. The Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
apps/web/lib/features/project/create-project-modal.tsxOops! Something went wrong! :( ESLint: 8.46.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
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: 4
🧹 Outside diff range and nitpick comments (4)
apps/web/components/ui/svgs/project-icon.tsx (1)
1-3
: LGTM! Consider adding prop documentation.The use of
Partial<IIconProps>
is appropriate for optional sizing props. Consider adding JSDoc comments to document the available props and their defaults.+/** + * Project icon component that renders a customizable SVG + * @param props.width - Optional width of the icon (default: 18) + * @param props.height - Optional height of the icon (default: 18) + */ export default function ProjectIcon(props: Partial<IIconProps>) {apps/web/lib/features/task/task-status.tsx (2)
1082-1082
: Consider improving dropdown menu accessibility and behaviorThe overflow-x-auto property on the Card component might cause unexpected horizontal scrolling in the dropdown menu. Consider the following improvements:
- Use overflow-y-auto instead since dropdowns typically only need vertical scrolling
- Add role="listbox" for better accessibility
-className="p-4 md:p-4 shadow-xlcard dark:shadow-lgcard-white dark:bg-[#1B1D22] dark:border dark:border-[#FFFFFF33] flex flex-col gap-2.5 overflow-x-auto" +className="p-4 md:p-4 shadow-xlcard dark:shadow-lgcard-white dark:bg-[#1B1D22] dark:border dark:border-[#FFFFFF33] flex flex-col gap-2.5 overflow-y-auto" +role="listbox"
Line range hint
1-1200
: Consider improving component architecture and reusabilityThe file contains multiple dropdown components with similar patterns and shared logic. Consider the following architectural improvements:
- Extract common styles into a shared constants file
- Create a custom hook for dropdown behavior
- Consider using a compound component pattern for better composition
This would improve maintainability and reduce code duplication.
Example approach:
// hooks/useDropdown.ts export function useDropdown({ multiple, onChange, values }) { // Common dropdown logic here } // constants/dropdownStyles.ts export const DROPDOWN_STYLES = { card: "p-4 md:p-4 shadow-xlcard dark:shadow-lgcard-white...", button: "justify-between capitalize...", // ... other common styles } // components/Dropdown/index.ts export const Dropdown = { Root: DropdownRoot, Trigger: DropdownTrigger, Content: DropdownContent, Item: DropdownItem }apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx (1)
429-430
: Clarify the button label to 'Create new project'The button label "Create new" is ambiguous. To improve clarity, consider changing it to "Create new project" so users understand they are creating a new project.
Apply this diff to update the button label:
- <span className=" truncate">Create new</span> + <span className=" truncate">Create new project</span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx
(5 hunks)apps/web/components/ui/svgs/project-icon.tsx
(1 hunks)apps/web/lib/features/task/task-status.tsx
(1 hunks)
🔇 Additional comments (1)
apps/web/components/ui/svgs/project-icon.tsx (1)
Line range hint 1-24
: Verify component usage with new props.
Let's verify that all instances of ProjectIcon in the codebase properly handle the new optional width and height props.
✅ Verification successful
All ProjectIcon usages properly handle the optional props
The component is used in task-secondary-info.tsx
in two places:
- One usage with default props:
<ProjectIcon />
- One usage with explicit width and height:
<ProjectIcon width={14} height={14} />
Both usages are correct since the props are optional (Partial) and have default values (18x18) in the component implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all ProjectIcon usages to verify prop handling
rg -A 2 "ProjectIcon" --type tsx
Length of output: 60
Script:
#!/bin/bash
# Search for all ProjectIcon usages with ripgrep, without file type filter
rg -l "ProjectIcon"
Length of output: 142
Script:
#!/bin/bash
# Search for all ProjectIcon usages with ast-grep to find JSX usage patterns
ast-grep --pattern '<ProjectIcon $$$>'
Length of output: 40
Script:
#!/bin/bash
# Use fd to find all TypeScript and TSX files, then search for ProjectIcon
fd -e ts -e tsx | xargs rg "ProjectIcon" -A 2
Length of output: 1435
apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx
Outdated
Show resolved
Hide resolved
apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx
Show resolved
Hide resolved
apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx
Show resolved
Hide resolved
4d6b944
to
7f54662
Compare
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: 6
🧹 Outside diff range and nitpick comments (7)
apps/web/app/services/client/api/projects.ts (1)
4-6
: Consider adding error handling and input validationWhile the API function implementation is clean, consider these improvements:
- Add error handling to catch and properly handle API failures
- Consider adding input validation before making the API call
Here's a suggested implementation:
export function createOrganizationProjectAPI(data: IProjectCreate) { + // Basic validation + if (!data.name?.trim()) { + throw new Error('Project name is required'); + } + + try { return post<IProject>(`/organization-projects`, data); + } catch (error) { + // Log error for monitoring + console.error('Failed to create project:', error); + throw error; + } }apps/web/lib/features/project/create-project-modal.tsx (4)
7-11
: Add JSDoc documentation for the onSuccess callback propThe interface is well-structured, but the
onSuccess
callback prop lacks documentation explaining its purpose and usage.Consider adding JSDoc documentation:
interface ICreateProjectModalProps { open: boolean; closeModal: () => void; + /** Callback function invoked after successful project creation + * @param project - The newly created project object + */ onSuccess?: (project: IProject) => void; }
51-62
: Enhance input field accessibility and internationalizationThe input field implementation can be improved in several ways:
- Placeholder text should use translations
- Missing form submission handling for Enter key
- Missing aria-label for accessibility
Consider these improvements:
+<form onSubmit={(e) => { + e.preventDefault(); + handleCreateProject(); +}}> <InputField name="name" autoCustomFocus value={name} onChange={(e) => setName(e.target.value)} - placeholder={'Please enter the project name'} + placeholder={t('common.PROJECT_NAME_PLACEHOLDER')} + aria-label={t('common.PROJECT_NAME_LABEL')} required className="w-full" wrapperClassName=" h-full border border-blue-500" noWrapper /> +</form>
64-82
: Improve button accessibility and form handlingThe button implementation should be enhanced for better accessibility and form handling:
Consider these improvements:
<div className="flex items-center justify-between w-full"> <Button disabled={createOrganizationProjectLoading} onClick={closeModal} className="h-[2.75rem]" variant="outline" + type="button" + aria-label={t('common.CANCEL_PROJECT_CREATION')} > {t('common.CANCEL')} </Button> <Button disabled={createOrganizationProjectLoading} loading={createOrganizationProjectLoading} className="h-[2.75rem]" type="submit" - onClick={handleCreateProject} + form="project-form" + aria-label={t('common.CONFIRM_PROJECT_CREATION')} > {t('common.CREATE')} </Button> </div>
21-87
: Implementation aligns well with PR objectivesThe modal component successfully implements the core functionality required by issue #3101, providing a streamlined way to create projects from the task form. The implementation handles loading states, callbacks, and basic error scenarios appropriately.
Consider adding these UX enhancements in future iterations:
- Color picker for project color (mentioned in requirements)
- Form validation feedback
- Success/error notifications
apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx (2)
Line range hint
308-321
: Enhance error handling in handleUpdateProjectThe current error handling only logs to console, which might leave users unaware of failures.
Consider implementing proper error handling:
const handleUpdateProject = useCallback( async (project: IProject) => { try { if (task) { await updateTask({ ...task, projectId: project.id }); } } catch (error) { - console.error(error); + // Log error for debugging + console.error('Failed to update project:', error); + // Notify user + toast.error(t('common.ERROR_UPDATING_PROJECT')); + // Revert selection + setSelected(task?.project); } }, [task, updateTask] );
399-401
: Consider making the dropdown height responsiveThe hardcoded height of 13rem (
h-[13rem] max-h-[13rem]
) might not accommodate varying numbers of projects efficiently.Consider using dynamic sizing:
className={clsxm( - 'p-0 md:p-0 shadow-xlcard dark:shadow-lgcard-white dark:bg-[#1B1D22] dark:border dark:border-[#FFFFFF33] flex flex-col gap-2.5 h-[13rem] max-h-[13rem] overflow-x-auto rounded-none overflow-hidden', + 'p-0 md:p-0 shadow-xlcard dark:shadow-lgcard-white dark:bg-[#1B1D22] dark:border dark:border-[#FFFFFF33] flex flex-col gap-2.5 max-h-[calc(100vh-20rem)] min-h-[13rem] overflow-x-auto rounded-none overflow-hidden', styles?.listCard )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (17)
apps/web/app/hooks/features/useOrganizationProjects.ts
(4 hunks)apps/web/app/services/client/api/projects.ts
(1 hunks)apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx
(5 hunks)apps/web/lib/features/project/create-project-modal.tsx
(1 hunks)apps/web/locales/ar.json
(1 hunks)apps/web/locales/bg.json
(1 hunks)apps/web/locales/de.json
(1 hunks)apps/web/locales/en.json
(1 hunks)apps/web/locales/es.json
(1 hunks)apps/web/locales/fr.json
(1 hunks)apps/web/locales/he.json
(1 hunks)apps/web/locales/it.json
(1 hunks)apps/web/locales/nl.json
(1 hunks)apps/web/locales/pl.json
(1 hunks)apps/web/locales/pt.json
(1 hunks)apps/web/locales/ru.json
(1 hunks)apps/web/locales/zh.json
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/locales/bg.json
🧰 Additional context used
📓 Learnings (3)
apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx (1)
Learnt from: CREDO23
PR: ever-co/ever-teams#3360
File: apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx:346-354
Timestamp: 2024-11-25T18:49:15.126Z
Learning: In `apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx`, within the `ProjectDropDown` component, both parent and child state should be synchronized for better UI updates, even in controlled mode. The `onChange` function carries the parent state, and `setSelected` maintains the selected state in the child component.
apps/web/locales/en.json (1)
Learnt from: CREDO23
PR: ever-co/ever-teams#3309
File: apps/web/locales/es.json:335-337
Timestamp: 2024-11-12T14:06:02.202Z
Learning: When reviewing additions of new translation keys in `apps/web/locales/`, ensure to verify whether the keys have been added to all relevant locale files before suggesting they are missing.
apps/web/locales/es.json (1)
Learnt from: CREDO23
PR: ever-co/ever-teams#3309
File: apps/web/locales/es.json:335-337
Timestamp: 2024-11-12T14:06:02.202Z
Learning: When reviewing additions of new translation keys in `apps/web/locales/`, ensure to verify whether the keys have been added to all relevant locale files before suggesting they are missing.
🔇 Additional comments (19)
apps/web/app/services/client/api/projects.ts (2)
1-1
: LGTM! Good use of shared interfaces
Using shared interfaces from @app/interfaces ensures type consistency across the application.
4-6
: Verify API usage across the codebase
Let's ensure all callers are properly updated and handle errors appropriately.
apps/web/app/hooks/features/useOrganizationProjects.ts (4)
5-6
: LGTM! Import changes are well-organized.
The new API import follows the existing pattern and is properly grouped with related imports.
31-32
: LGTM! Query hook setup follows existing patterns.
The new query hook setup maintains consistency with other API calls in the file.
105-106
: LGTM! Return object additions are well-organized.
The new function and loading state are properly exposed and maintain the existing structure.
77-93
: Verify integration with CreateProjectModal component.
Let's verify that the CreateProjectModal properly handles loading states and error scenarios when using this function.
✅ Verification successful
Integration with CreateProjectModal is properly implemented
The modal component correctly:
- Uses loading states from
createOrganizationProjectLoading
to disable buttons and show loading spinner - Handles success case by calling
onSuccess
callback and closing the modal - Properly manages error scenarios with try-catch block
- Maintains consistent UI state by disabling interactions during loading
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for CreateProjectModal usage to verify proper integration
ast-grep --pattern 'const { createOrganizationProject, createOrganizationProjectLoading } = useOrganizationProjects()'
# Check error handling in the modal
rg -A 5 'createOrganizationProject.*catch' apps/web/lib/features/project/create-project-modal.tsx
Length of output: 378
Script:
#!/bin/bash
# Let's check the CreateProjectModal implementation to verify error handling and loading states
cat apps/web/lib/features/project/create-project-modal.tsx
# Also check if there are any test files that verify this integration
fd "create-project-modal.test" --exec cat {}
Length of output: 2634
apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx (3)
218-218
: LGTM: Clean integration of ProjectDropDown
The ProjectDropDown component is properly integrated into the TaskSecondaryInfo component, maintaining consistency with other dropdowns in the section.
445-455
: LGTM: Proper implementation of CreateProjectModal
The modal integration handles both controlled and uncontrolled modes correctly, and properly updates both local and parent state on success.
285-294
: Verify project creation flow in different scenarios
Please ensure testing covers these scenarios:
- Creating a project when no projects exist
- Creating a project with the same name as an existing project
- Creating a project when the projects list is very long
- Network failure during project creation
apps/web/locales/zh.json (1)
271-271
: LGTM! Translation added for new project creation feature.
The new translation key "CREATE_NEW" with value "创建新的" is correctly implemented and follows the established naming conventions.
apps/web/locales/he.json (1)
270-271
: LGTM! The new translation key and value are correctly implemented.
The addition of the CREATE_NEW
key with its Hebrew translation "צור חדש" is consistent with the file's naming conventions and properly handles RTL text direction.
apps/web/locales/ar.json (1)
252-253
: LGTM! The translation changes look good.
The new translation key CREATE_NEW
and its Arabic translation are correctly implemented, supporting the feature of creating a new project from the project dropdown. The trailing comma after APPLY_FILTER
maintains proper JSON formatting.
apps/web/locales/nl.json (1)
270-271
: LGTM! The translations are correct and well-organized.
The changes maintain proper alphabetical ordering and the Dutch translation for "CREATE_NEW" is grammatically correct.
apps/web/locales/pl.json (1)
270-271
: LGTM! The translations are well-structured and support the PR objectives.
The new Polish translations for "APPLY_FILTER" and "CREATE_NEW" are correctly formatted, properly placed in the "common" section, and will effectively support the new feature for creating projects from dropdowns.
apps/web/locales/it.json (1)
270-271
: LGTM! Translation changes look good.
The new translation key "CREATE_NEW" with value "Crea nuovo" is:
- Correctly placed in the common section
- Follows the naming convention
- Provides an accurate Italian translation
- Aligns with the PR objective of adding project creation functionality
apps/web/locales/pt.json (1)
270-271
: LGTM! The changes look good.
The changes are well-structured and serve their purpose:
- Added proper comma after "Aplicar filtro" to maintain valid JSON format
- Added new translation key "CREATE_NEW" with appropriate Portuguese translation "Criar novo" which aligns with the feature requirements for creating new projects
apps/web/locales/es.json (1)
271-271
: Verify translation key exists in all locale files
Let's verify that the "CREATE_NEW" key has been added to all locale files as mentioned in the AI summary.
✅ Verification successful
Let me verify the actual content of these files to understand if there's a case sensitivity issue or if the key is truly missing.
The "CREATE_NEW" key exists in all locale files with proper translations
The case-insensitive search reveals that the "CREATE_NEW" key is present in all locale files with appropriate translations:
- Arabic (ar): "إنشاء جديد"
- Bulgarian (bg): "Създай нов"
- German (de): "Neu erstellen"
- English (en): "Crear nuevo"
- Spanish (es): "Create New"
- French (fr): "Créer nouveau"
- Hebrew (he): "צור חדש"
- Italian (it): "Crea nuovo"
- Dutch (nl): "Nieuwe maken"
- Polish (pl): "Utwórz nowy"
- Portuguese (pt): "Criar novo"
- Russian (ru): "Создать новый"
- Chinese (zh): "创建新的"
Note: The Spanish translation in the English locale file and the English text in the Spanish locale file appear to be swapped, but that's outside the scope of this verification.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the "CREATE_NEW" key exists in all locale files
# Expected: The key should exist in all locale files (ar, bg, de, en, fr, he, it, nl, pl, pt, ru, zh)
# Find all locale files and check for the CREATE_NEW key
fd -e json . apps/web/locales -x grep -l "CREATE_NEW" {} \; | sort
# Show locale files missing the key (if any)
echo "Files missing CREATE_NEW key:"
fd -e json . apps/web/locales -x sh -c 'grep -L "CREATE_NEW" "$1" && echo "$1"' sh {} \;
Length of output: 882
Script:
#!/bin/bash
# Check the actual content around potential "CREATE_NEW" key with case insensitive search
for locale in ar bg de en es fr he it nl pl pt ru zh; do
echo "=== $locale.json ==="
rg -i "create.?new" "apps/web/locales/$locale.json" -C 1 || echo "No match found"
echo
done
Length of output: 11320
apps/web/locales/de.json (1)
270-271
: LGTM! The German translations are accurate and consistent.
The translations for "APPLY_FILTER" and "CREATE_NEW" are grammatically correct and follow the established UI text conventions in the file.
apps/web/locales/fr.json (1)
270-271
: LGTM! The translations are properly formatted and linguistically correct.
The new French translations for filter application and creation actions are appropriately placed in the common section and align well with the PR's objective of enabling project creation functionality.
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.
Please Resolve EN and ES translations for "CREATE_NEW" key
@CREDO23
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
🧹 Outside diff range and nitpick comments (2)
apps/web/lib/features/project/create-project-modal.tsx (2)
12-20
: Enhance JSDoc documentationThe documentation is missing details for the
onSuccess
callback prop and could provide more context about the parameters./** * A modal that allow to create a new project * * @param {Object} props - The props Object * @param {boolean} props.open - If true open the modal otherwise close the modal - * @param {() => void} props.closeModal - A function to close the modal + * @param {() => void} props.closeModal - Callback function to handle modal closure + * @param {(project: IProject) => void} props.onSuccess - Optional callback function triggered after successful project creation * * @returns {JSX.Element} The modal element */
51-96
: Enhance UI implementationConsider the following improvements:
- Use translations for the input placeholder instead of hardcoded text
- Consider using theme variables for border color instead of hardcoded
border-blue-500
- Add form submission on Enter key press
-<InputField +<form onSubmit={(e) => { + e.preventDefault(); + handleCreateProject(); +}}> + <InputField name="name" autoCustomFocus value={name} onChange={(e) => setName(e.target.value)} - placeholder={'Please enter the project name'} + placeholder={t('common.PROJECT_NAME_PLACEHOLDER')} required className="w-full" - wrapperClassName=" h-full border border-blue-500" + wrapperClassName=" h-full border border-primary" noWrapper /> +</form>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
apps/web/lib/features/project/create-project-modal.tsx
(1 hunks)apps/web/locales/en.json
(1 hunks)apps/web/locales/es.json
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/locales/en.json
- apps/web/locales/es.json
🔇 Additional comments (3)
apps/web/lib/features/project/create-project-modal.tsx (3)
1-11
: LGTM! Clean imports and well-defined interface.
The interface and imports are well-organized and properly typed.
21-32
: LGTM! Clean hook implementation with proper cleanup.
The component properly implements hooks and includes cleanup logic to reset the state when unmounted.
34-49
: Refer to previous review comment about error handling
The previous review comment about enhancing error handling and input validation is still applicable to this code segment.
Description
Fix #3101
Type of Change
Checklist
Current screenshots
Loom
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements
ProjectIcon
component for dynamic sizing based on props.TaskStatusDropdown
for better usability and responsiveness.Bug Fixes