Skip to content
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

Add modal to confirm workflow draft version overriding #7758

Merged
merged 5 commits into from
Oct 21, 2024

Conversation

Devessier
Copy link
Contributor

In this PR:

  • Allow the <ConfirmationModal /> to take additional buttons to display between the cancel and the confirm buttons.
  • Create a modal that's displayed when the user tries wants to use a workflow version as draft while a draft version already exists. The displayed modal contains a link to the current draft version and a button to confirm the overriding of the current draft version.

A demo:

CleanShot.2024-10-16.at.17.01.13.mp4

Closes twentyhq/private-issues#114

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR introduces a confirmation modal for overriding existing workflow draft versions, enhancing user experience and preventing accidental data loss.

  • Added AdditionalButtons prop to ConfirmationModal in packages/twenty-front/src/modules/ui/layout/modal/components/ConfirmationModal.tsx for flexible button configurations
  • Created new OverrideWorkflowDraftConfirmationModal component in packages/twenty-front/src/modules/workflow/components/OverrideWorkflowDraftConfirmationModal.tsx to handle draft override confirmations
  • Implemented Recoil state management for modal visibility in packages/twenty-front/src/modules/workflow/states/openOverrideWorkflowDraftConfirmationModalState.ts
  • Updated RecordShowPageWorkflowVersionHeader in packages/twenty-front/src/modules/workflow/components/RecordShowPageWorkflowVersionHeader.tsx to integrate the new confirmation modal

4 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile

@@ -138,6 +141,9 @@ export const ConfirmationModal = ({
title="Cancel"
fullWidth
/>

{AdditionalButtons}
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider wrapping AdditionalButtons in a conditional to avoid rendering empty space when not provided

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it can cause problems. If AdditionalButtons equals undefined, React will render nothing.

isOpen={openOverrideWorkflowDraftConfirmationModal}
setIsOpen={setOpenOverrideWorkflowDraftConfirmationModal}
title="A draft already exists"
subtitle="A draft already exists for this workflow. Are you sure to erase it?"
Copy link
Contributor

Choose a reason for hiding this comment

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

style: The subtitle grammar could be improved

Copy link
Contributor

Choose a reason for hiding this comment

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

indeed

Suggested change
subtitle="A draft already exists for this workflow. Are you sure to erase it?"
subtitle="A draft already exists for this workflow. Are you sure you want to erase it?"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -46,6 +48,7 @@ export const RecordShowPageWorkflowVersionHeader = ({
skip: !isDefined(workflowVersion),
limit: 1,
});
const draftWorkflowVersion = draftWorkflowVersions[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Ensure draftWorkflowVersion is properly typed as WorkflowVersion | undefined

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

await updateOneWorkflowVersion({
idToUpdate: draftWorkflowVersionId,
updateOneRecordInput: workflowVersionUpdateInput,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

could you check with @Bonapara if we expect a redirection afterwards?

@Devessier Devessier force-pushed the confirm-workflow-draft-override-modal branch from 67f6c9b to 813526e Compare October 21, 2024 08:23
@Devessier Devessier merged commit fc6748d into main Oct 21, 2024
13 checks passed
@Devessier Devessier deleted the confirm-workflow-draft-override-modal branch October 21, 2024 09:51
Copy link

Thanks @Devessier for your contribution!
This marks your 16th PR on the repo. You're top 2% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

Contributions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants