[WEB-2577] improvement: use common create/update issue modal for accepting intake issues for consistency #5830
Conversation
…pting intake issues for consistency
WalkthroughThe changes in this pull request involve significant modifications to several components related to issue management in the application. Key updates include the renaming and replacement of modal components, the introduction of new props for enhanced configurability, and the removal of obsolete components. Additionally, adjustments to existing interfaces and methods improve the handling of issue properties and user interactions. Overall, the changes streamline the modal functionality and improve the user experience without altering the underlying logic. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (12)
web/core/components/inbox/modals/create-modal/modal.tsx (2)
14-27: LGTM: Component implementation is clean and aligns with PR objectives.The
InboxIssueCreateModalRootcomponent is well-structured and correctly implements the modal for creating inbox issues. It aligns with the PR objectives of using a common modal for consistency.Consider adding a brief JSDoc comment above the component to describe its purpose and props. This can improve code documentation and help other developers understand the component's role more quickly.
Example:
/** * Modal component for creating inbox issues. * @param {string} workspaceSlug - The slug of the current workspace. * @param {string} projectId - The ID of the current project. * @param {boolean} modalState - Whether the modal is open or closed. * @param {() => void} handleModalClose - Function to close the modal. */ export const InboxIssueCreateModalRoot: FC<TInboxIssueCreateModalRoot> = (props) => { // ... (rest of the component code) };
1-27: Overall, excellent implementation that meets PR objectives.This new
InboxIssueCreateModalRootcomponent successfully implements a common modal for creating inbox issues, which aligns perfectly with the PR objectives of improving consistency across the application. The code is clean, well-structured, and follows React and TypeScript best practices.To further enhance the component's reusability and flexibility, consider the following suggestions for future improvements:
- If this modal pattern is used in multiple places, you might want to create a more generic
IssueCreateModalcomponent that can be configured for different contexts (inbox, backlog, etc.).- Consider using a context provider for workspace and project information if these are frequently passed down to many components. This could help reduce prop drilling in larger component trees.
web/core/components/issues/issue-modal/modal.tsx (1)
18-29: LGTM! Consider adding JSDoc comments for better documentation.The new properties
beforeFormSubmit,modalTitle, andprimaryButtonTextenhance the flexibility and customization of the modal, aligning well with the PR objective of improving consistency and reusability. The changes are backwards compatible as all new properties are optional.Consider adding JSDoc comments to describe the purpose and usage of these new properties. This would improve the self-documentation of the code. For example:
/** * Function to be called before form submission. Can be used for validation or data preparation. * @returns A Promise that resolves when the pre-submission tasks are complete. */ beforeFormSubmit?: () => Promise<void>; /** Custom title for the modal. */ modalTitle?: string; /** * Custom text for the primary button in different states. * @property {string} default - Text to display in the default state. * @property {string} loading - Text to display when the form is being submitted. */ primaryButtonText?: { default: string; loading: string; };web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/inbox/header.tsx (2)
72-77: Update documentation to reflect changes in modal functionality.The changes in the component usage are consistent with the renaming of
InboxIssueCreateEditModalRoottoInboxIssueCreateModalRoot. The removal of theissueprop suggests that this modal is now specifically for creating new issues, not editing existing ones.Consider updating any relevant documentation or comments to reflect this change in functionality. This will help maintain clear and accurate documentation for the development team.
Line range hint
1-85: Summary of changes and potential impactThe changes in this file are part of a larger refactoring effort to separate the creation and editing functionalities for inbox issues. The main changes include:
- Renaming
InboxIssueCreateEditModalRoottoInboxIssueCreateModalRoot.- Removing the
issueprop from the modal component usage.These changes align with the PR objective of using a common modal for creating and updating issues related to intake processes.
Consider the following points:
- Ensure that the separation of create and edit functionalities is consistently applied across the entire application.
- Update user documentation to reflect any changes in the user interface or workflow.
- If not already done, consider adding or updating unit tests to cover the new functionality of the
InboxIssueCreateModalRootcomponent.web/core/store/issue/issue.store.ts (1)
128-128: Approve changes with a minor suggestion for consistencyThe consolidation of the conditional logic improves code readability and potentially performance. The new implementation correctly filters issues based on their archived status for both "archived" and "un-archived" types.
For consistency, consider removing the optional chaining operator (
?.) from the "un-archived" condition, as it's not used in the "archived" condition. This change would make the code more uniform:- if (issue && ((type === "archived" && issue.archived_at) || (type === "un-archived" && !issue?.archived_at))) { + if (issue && ((type === "archived" && issue.archived_at) || (type === "un-archived" && !issue.archived_at))) {This minor adjustment doesn't affect functionality but enhances code consistency.
web/core/components/issues/issue-modal/base.tsx (3)
31-39: LGTM! Consider updating documentation for new props.The addition of
beforeFormSubmit,modalTitle, andprimaryButtonTextprops enhances the component's flexibility. This change allows for better customization of the modal's behavior and appearance.Consider updating the component's documentation or adding JSDoc comments to describe these new props and their usage.
257-259: LGTM! Consider refactoring for consistency.The addition of
issueTypeIdtohandleCreateUpdatePropertyValuesin the update flow is consistent with the earlier change in the create flow. This ensures that issue type is considered when updating property values for existing issues.For better maintainability, consider extracting the call to
handleCreateUpdatePropertyValuesinto a separate function, as it's used in bothhandleCreateIssueandhandleUpdateIssue. This would reduce code duplication and make future changes easier. For example:const updatePropertyValues = async (issueId: string, projectId: string, issueTypeId: string) => { await handleCreateUpdatePropertyValues({ issueId, issueTypeId, projectId, workspaceSlug: workspaceSlug.toString(), }); };Then you can call this function in both
handleCreateIssueandhandleUpdateIssue.
296-298: LGTM! Consider adding error handling forbeforeFormSubmit.The addition of
beforeFormSubmitinhandleFormSubmitand passingmodalTitleandprimaryButtonTexttoIssueFormRootare good improvements. These changes enhance the component's flexibility and customization options.Consider adding error handling for
beforeFormSubmitto gracefully handle any errors that might occur during its execution. For example:if (beforeFormSubmit) { try { await beforeFormSubmit(); } catch (error) { console.error("Error in beforeFormSubmit:", error); // Optionally, you could set an error state or show a toast notification here } }This would prevent any errors in
beforeFormSubmitfrom silently breaking the form submission process.Also applies to: 357-358
web/core/components/issues/issue-modal/form.tsx (2)
84-88: LGTM! Default values for new props enhance usability.The default values for
modalTitleandprimaryButtonTextare well-implemented, providing context-appropriate text based on the issue's state. This ensures the component works well even without custom prop values.For consistency, consider extracting the condition
data?.id ? "Update" : isDraft ? "Create draft" : "Create new"into a variable, as it's used multiple times. This would make future modifications easier and reduce the risk of inconsistencies.
427-427: LGTM! Dynamic submit button text enhances user feedback.The use of
primaryButtonTextfor the submit button improves user feedback by clearly indicating the current state (submitting or ready to submit). This change enhances the overall user experience.Consider adding an
aria-live="polite"attribute to the button or a nearby element to ensure screen readers announce the change in button text when the form submission state changes. This would further improve accessibility for users relying on assistive technologies.web/core/components/inbox/content/inbox-issue-header.tsx (1)
237-248: LGTM: Improved modal implementation with suggestionThe replacement of
InboxIssueCreateEditModalRootwithCreateUpdateIssueModalis a positive change. The new implementation provides more flexibility and reusability, with custom props for better control over the modal's behavior.Consider extracting the modal title to a constant or a function for better readability:
const getModalTitle = () => `Move ${currentProjectDetails?.identifier}-${issue?.sequence_id} to project issues`; // Then use it in the component modalTitle={getModalTitle()}This change would improve code readability and make it easier to modify the title in the future if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (16)
- web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/inbox/header.tsx (2 hunks)
- web/core/components/inbox/content/inbox-issue-header.tsx (3 hunks)
- web/core/components/inbox/modals/create-edit-modal/edit-root.tsx (0 hunks)
- web/core/components/inbox/modals/create-edit-modal/modal.tsx (0 hunks)
- web/core/components/inbox/modals/create-modal/create-root.tsx (2 hunks)
- web/core/components/inbox/modals/create-modal/index.ts (0 hunks)
- web/core/components/inbox/modals/create-modal/issue-properties.tsx (1 hunks)
- web/core/components/inbox/modals/create-modal/modal.tsx (1 hunks)
- web/core/components/inbox/modals/index.ts (1 hunks)
- web/core/components/issues/issue-modal/base.tsx (6 hunks)
- web/core/components/issues/issue-modal/context/issue-modal.tsx (1 hunks)
- web/core/components/issues/issue-modal/draft-issue-layout.tsx (5 hunks)
- web/core/components/issues/issue-modal/form.tsx (5 hunks)
- web/core/components/issues/issue-modal/modal.tsx (1 hunks)
- web/core/components/issues/workspace-draft/delete-modal.tsx (1 hunks)
- web/core/store/issue/issue.store.ts (1 hunks)
💤 Files with no reviewable changes (3)
- web/core/components/inbox/modals/create-edit-modal/edit-root.tsx
- web/core/components/inbox/modals/create-edit-modal/modal.tsx
- web/core/components/inbox/modals/create-modal/index.ts
✅ Files skipped from review due to trivial changes (3)
- web/core/components/inbox/modals/create-modal/issue-properties.tsx
- web/core/components/inbox/modals/index.ts
- web/core/components/issues/workspace-draft/delete-modal.tsx
🧰 Additional context used
🔇 Additional comments (20)
web/core/components/inbox/modals/create-modal/modal.tsx (2)
1-5: LGTM: Imports are appropriate and well-structured.The imports are concise and relevant to the component's requirements. Using an absolute path for the custom component import is a good practice for maintainability.
7-12: LGTM: Type definition is clear and comprehensive.The
TInboxIssueCreateModalRoottype is well-defined, including all necessary props for the modal component. The naming convention follows TypeScript best practices.web/core/components/issues/issue-modal/context/issue-modal.tsx (1)
24-24: Approved: Type expansion forissueTypeIdenhances flexibility.The change to allow
issueTypeIdto bestring | null | undefinedinstead of juststringprovides more flexibility in handling different states of the issue type. This aligns well with the PR objective of improving consistency for accepting intake issues.To ensure this change doesn't introduce any unintended side effects, please run the following script to verify the usage of
issueTypeIdacross the codebase:This script will help identify any places where
issueTypeIdis used, set, or checked, allowing you to verify that the new type is handled correctly throughout the codebase.web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/inbox/header.tsx (1)
11-11: Verify the impact of component renaming across the codebase.The import statement has been updated from
InboxIssueCreateEditModalRoottoInboxIssueCreateModalRoot. This change aligns with the renaming mentioned in the AI-generated summary.To ensure this change has been consistently applied throughout the codebase, please run the following script:
This script will help identify any places where the old component name might still be in use and verify the implementation of the new component.
web/core/components/issues/issue-modal/draft-issue-layout.tsx (5)
34-38: LGTM! Enhances component flexibility.The addition of
modalTitleandprimaryButtonTextproperties to theDraftIssuePropsinterface improves the component's customizability. This change aligns well with the PR objective of improving consistency for accepting intake issues by allowing for context-specific UI text.
55-56: LGTM! Proper implementation of new props.The destructuring of
modalTitleandprimaryButtonTextfrom props is correctly implemented, making these new properties available for use within the component.
114-114: LGTM! Improved success message clarity.The updated success message "Draft Issue created successfully." is more specific and informative than the previous version. It clearly indicates the type of item created (a draft issue) and confirms the successful completion of the action.
142-142: LGTM, but additional context needed.The addition of
issueTypeIdto the payload inhandleCreateUpdatePropertyValuesis a good improvement for maintaining consistency in issue type information. However, it would be helpful to have some additional context or documentation explaining the necessity of this change and how it relates to the PR objectives.Could you provide more information about why this change was made and how it contributes to the goal of using a common create/update issue modal for accepting intake issues?
173-174: LGTM! Proper implementation of new props in child component.The addition of
modalTitleandprimaryButtonTextprops to theIssueFormRootcomponent correctly implements the new customization options. This change aligns well with the PR objective of improving consistency in the UI for accepting intake issues by allowing for context-specific customization of the modal title and primary button text.web/core/components/inbox/modals/create-modal/create-root.tsx (3)
200-200: Minor formatting improvement.The unnecessary space in the
onChangeprop of the ToggleSwitch component has been removed. This small change improves code consistency and readability.
Line range hint
1-238: Overall assessment: Changes align with PR objectives.The modifications in this file are minimal and focused:
- Updated import path for issue-related components.
- Minor formatting improvement in the ToggleSwitch component.
These changes align well with the PR objective of improving consistency in the issue modal usage. The core functionality of the component remains intact, which is a positive sign for this refactoring effort.
To ensure the refactoring hasn't introduced any regressions, consider the following:
- Verify that the new import path is correct and the imported components exist in the new location.
- Run the component's associated unit tests, if any, to confirm that the functionality remains unchanged.
- Perform a quick manual test of the inbox issue creation flow to ensure everything works as expected.
To help with the manual testing, you can use the following command to find the relevant test files:
#!/bin/bash # Find test files related to inbox issue creation fd -t f "create.*test.tsx" web/core/components/inboxThis will help locate the test files that should be run to verify the functionality of the inbox issue creation process.
12-12: LGTM! Verify the new import path.The import statement has been updated to reflect the new location of the components, which aligns with the PR objective of using a common create/update issue modal. This change improves consistency in the codebase structure.
To ensure the new import path is correct, please run the following command:
This command will search for the component files in the new directory. If the files are found, it confirms that the import path is correct.
web/core/components/issues/issue-modal/base.tsx (2)
213-213: LGTM! Improved user feedback for issue creation.The updated success message now differentiates between draft issues and regular issues. This change provides more specific feedback to the user, enhancing the overall user experience.
204-206: Verify the impact of addingissueTypeIdtohandleCreateUpdatePropertyValues.The addition of
issueTypeIdto the parameters ofhandleCreateUpdatePropertyValuesis a good improvement. It suggests that issue type is now being considered when creating or updating property values.Please ensure that this change is consistent with any new features or requirements related to issue types. You may want to verify that:
- The
handleCreateUpdatePropertyValuesfunction in theuseIssueModalhook correctly handles the newissueTypeIdparameter.- This change doesn't break any existing functionality.
web/core/components/issues/issue-modal/form.tsx (3)
64-68: LGTM! Improved customization for modal title and button text.The addition of
modalTitleandprimaryButtonTextprops enhances the reusability and flexibility of theIssueFormRootcomponent. These new props allow for dynamic customization of the modal title and submit button text, which can improve user experience by providing more context-specific information.
285-285: LGTM! Dynamic modal title improves context-awareness.The replacement of the static title with the dynamic
modalTitleprop enhances the component's flexibility. This change allows for more context-specific titles, improving the user experience by providing clearer information about the current action being performed.
Line range hint
64-427: Overall, these changes significantly improve the component's flexibility and user experience.The additions of
modalTitleandprimaryButtonTextprops, along with their implementation in the component, enhance the reusability and context-awareness of theIssueFormRoot. These changes allow for more dynamic and informative UI elements, which can lead to a better user experience across different use cases of this component.A few minor suggestions have been made to further improve consistency and accessibility, but the overall implementation is solid and well-thought-out.
web/core/components/inbox/content/inbox-issue-header.tsx (3)
28-28: LGTM: Import changes align with component updatesThe import changes correctly reflect the replacement of
InboxIssueCreateEditModalRootwithCreateUpdateIssueModal. This update suggests a move towards a more standardized issue management system, which is a positive change.
72-72: LGTM: Enhanced context with project detailsThe introduction of
currentProjectDetailsfrom theuseProjecthook is a good addition. It provides more context for the modal title and potentially for other parts of the component. This change enhances the user interface by including project-specific information.
Line range hint
1-448: Summary: Improved consistency and flexibility in issue managementThe changes in this file successfully implement a more consistent and flexible approach to managing intake issues. Key improvements include:
- Replacing the custom
InboxIssueCreateEditModalRootwith a more standardizedCreateUpdateIssueModal.- Enhanced context through the use of project-specific details in the UI.
- Improved flexibility with new props for better control over the modal's behavior.
These changes align well with the PR objectives and contribute to a more consistent user experience when accepting intake issues. The overall structure and logic of the component have been preserved, ensuring that existing functionality remains intact while introducing these improvements.
Issue link: WEB-2577
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation