fix(PM-1610): Show confirm modal while accepting application #1180
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -0,0 +1,35 @@ | |||
| /* eslint-disable react/jsx-no-bind */ | |||
There was a problem hiding this comment.
Avoid using /* eslint-disable react/jsx-no-bind */. Instead, consider refactoring the code to avoid inline function definitions, which can lead to performance issues due to unnecessary re-renders.
|
|
||
| const ConfirmModal: FC<ConfirmModalProps> = props => ( | ||
| <BaseModal | ||
| onClose={props.onClose as () => void} |
There was a problem hiding this comment.
Casting props.onClose to () => void may hide potential type mismatches. Ensure that props.onClose is indeed compatible with () => void to avoid runtime errors.
| @@ -31,19 +33,8 @@ const CopilotApplicationAction = ( | |||
|
|
|||
| if (copilotApplication.existingMembership) { | |||
There was a problem hiding this comment.
The return statement has been removed. Ensure that this change does not affect the intended flow of the function, as previously the function would exit early if copilotApplication.existingMembership was true.
| @@ -57,6 +48,7 @@ const CopilotApplicationAction = ( | |||
| toast.success('Accepted as copilot') | |||
| copilotApplication.onApplied() | |||
| setShowAlreadyMemberModal(false) | |||
| setShowConfirmModal(false) | |||
There was a problem hiding this comment.
It seems like setShowConfirmModal(false) is being called after a successful application acceptance. Ensure that this line is necessary and that the modal state is being managed correctly throughout the component lifecycle.
| @@ -67,6 +59,7 @@ const CopilotApplicationAction = ( | |||
| e.preventDefault() | |||
| e.stopPropagation() | |||
| setShowAlreadyMemberModal(false) | |||
| setShowConfirmModal(false) | |||
| }, [showAlreadyMemberModal]) | |||
There was a problem hiding this comment.
The dependency array for the useCallback hook includes showAlreadyMemberModal, but it should also include setShowConfirmModal since it is used within the callback. Consider adding setShowConfirmModal to the dependency array to ensure the callback updates correctly when setShowConfirmModal changes.
| @@ -106,6 +99,13 @@ const CopilotApplicationAction = ( | |||
| copilotApplication={copilotApplication} | |||
| /> | |||
| )} | |||
|
|
|||
| {showConfirmModal && ( | |||
There was a problem hiding this comment.
The variable showConfirmModal should be initialized and managed properly to ensure the modal displays correctly. Ensure that the logic for setting showConfirmModal is implemented and tested.
|
|
||
| {showConfirmModal && ( | ||
| <ConfirmModal | ||
| onClose={onCloseModal} |
There was a problem hiding this comment.
Consider adding error handling or user feedback in case onApply fails to execute successfully. This will improve the user experience by providing clarity on the action's outcome.
Related JIRA Ticket:
https://topcoder.atlassian.net/browse/PM-1610
What's in this PR?