Skip to content

fix(PM-1610): Show confirm modal while accepting application #1180

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 10, 2025
Merged

fix(PM-1610): Show confirm modal while accepting application #1180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9689622
fix: show confirm modal
hentrymartin Aug 10, 2025
c2eaa92
fix: show confirm modal
hentrymartin Aug 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions ...copilots/src/pages/copilot-opportunity-details/tabs/copilot-applications/ConfirmModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/* eslint-disable react/jsx-no-bind */
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

import { FC } from 'react'

import { BaseModal, Button } from '~/libs/ui'

import styles from './styles.module.scss'

interface ConfirmModalProps {
onClose: (e: React.MouseEvent<HTMLButtonElement, MouseEvent>) => void
onApply: () => void
}

const ConfirmModal: FC<ConfirmModalProps> = props => (
<BaseModal
onClose={props.onClose as () => void}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Casting props.onClose to () => void may hide potential type mismatches. Ensure that props.onClose is indeed compatible with () => void to avoid runtime errors.

open
size='lg'
title='Confirm to accept as copilot'
buttons={(
<>
<Button primary onClick={props.onApply} label='Confirm' />
<Button secondary onClick={props.onClose} label='Cancel' />
</>
)}
>
<div className={styles.applyCopilotModal}>
<div className={styles.info}>
Click &apos;Confirm&apos; to accept by updating project role to &apos;Copilot&apos;
and complete this opportunity
</div>
</div>
</BaseModal>
)

export default ConfirmModal
28 changes: 14 additions & 14 deletions .../pages/copilot-opportunity-details/tabs/copilot-applications/CopilotApplicationAction.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { CopilotApplication, CopilotApplicationStatus } from '~/apps/copilots/sr
import { IconSolid, Tooltip } from '~/libs/ui'

import AlreadyMemberModal from './AlreadyMemberModal'
import ConfirmModal from './ConfirmModal'
import styles from './styles.module.scss'

const CopilotApplicationAction = (
Expand All @@ -15,6 +16,7 @@ const CopilotApplicationAction = (
): JSX.Element => {
const { opportunityId }: {opportunityId?: string} = useParams<{ opportunityId?: string }>()
const [showAlreadyMemberModal, setShowAlreadyMemberModal] = useState(false)
const [showConfirmModal, setShowConfirmModal] = useState(false)
const isInvited = useMemo(
() => allCopilotApplications
&& allCopilotApplications.findIndex(item => item.status === CopilotApplicationStatus.INVITED) > -1,
Expand All @@ -31,19 +33,8 @@ const CopilotApplicationAction = (

if (copilotApplication.existingMembership) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

setShowAlreadyMemberModal(true)
return
}

if (opportunityId) {
try {
await assignCopilotOpportunity(opportunityId, copilotApplication.id)
toast.success('Accepted as copilot')
copilotApplication.onApplied()
} catch (e) {
const error = e as Error
toast.error(error.message)
}

} else {
setShowConfirmModal(true)
}
}, [opportunityId, copilotApplication])

Expand All @@ -57,6 +48,7 @@ const CopilotApplicationAction = (
toast.success('Accepted as copilot')
copilotApplication.onApplied()
setShowAlreadyMemberModal(false)
setShowConfirmModal(false)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

} catch (e) {
const error = e as Error
toast.error(error.message)
Expand All @@ -67,6 +59,7 @@ const CopilotApplicationAction = (
e.preventDefault()
e.stopPropagation()
setShowAlreadyMemberModal(false)
setShowConfirmModal(false)
}, [showAlreadyMemberModal])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


return (
Expand All @@ -83,7 +76,7 @@ const CopilotApplicationAction = (
!isInvited
&& copilotApplication.status === CopilotApplicationStatus.PENDING
&& copilotApplication.opportunityStatus === 'active' && (
<Tooltip content='Accept Application'>
<Tooltip content='Accept'>
<IconSolid.UserAddIcon />
</Tooltip>
)
Expand All @@ -106,6 +99,13 @@ const CopilotApplicationAction = (
copilotApplication={copilotApplication}
/>
)}

{showConfirmModal && (
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

<ConfirmModal
onClose={onCloseModal}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

onApply={onApply}
/>
)}
</div>
)
}
Expand Down