-
Notifications
You must be signed in to change notification settings - Fork 798
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 delete action in workflows table #802
Conversation
<!-- ELLIPSIS_HIDDEN --> | 🚀 | This description was created by [Ellipsis](https://www.ellipsis.dev) for commit 6ea20bb37cf65de9d6ad048ffdc697c0dd22264f | |--------|--------| ### Summary: Added delete functionality to workflows table with `DeleteWorkflowButton`, updated `GarbageIcon`, and removed `WorkflowsBetaAlertCard`. **Key points**: - Added `DeleteWorkflowButton` component in `skyvern-frontend/src/routes/workflows/editor/DeleteWorkflowButton.tsx` to handle workflow deletion. - Updated `GarbageIcon` in `skyvern-frontend/src/components/icons/GarbageIcon.tsx` to make `className` prop optional. - Removed `WorkflowsBetaAlertCard` from `skyvern-frontend/src/routes/workflows/Workflows.tsx`. - Integrated `DeleteWorkflowButton` into workflows table in `skyvern-frontend/src/routes/workflows/Workflows.tsx`. - `DeleteWorkflowButton` uses `react-query` for mutation and `axios` for API calls. ---- Generated with ❤️ by [ellipsis.dev](https://www.ellipsis.dev) <!-- ELLIPSIS_HIDDEN -->
…src/' <!-- ELLIPSIS_HIDDEN --> | 🚀 | This description was created by [Ellipsis](https://www.ellipsis.dev) for commit 6ea20bb37cf65de9d6ad048ffdc697c0dd22264f | |--------|--------| ### Summary: Added delete functionality to workflows table with `DeleteWorkflowButton`, updated `GarbageIcon`, and removed `WorkflowsBetaAlertCard`. **Key points**: - Added `DeleteWorkflowButton` component in `skyvern-frontend/src/routes/workflows/editor/DeleteWorkflowButton.tsx` to handle workflow deletion. - Updated `GarbageIcon` in `skyvern-frontend/src/components/icons/GarbageIcon.tsx` to make `className` prop optional. - Removed `WorkflowsBetaAlertCard` from `skyvern-frontend/src/routes/workflows/Workflows.tsx`. - Integrated `DeleteWorkflowButton` into workflows table in `skyvern-frontend/src/routes/workflows/Workflows.tsx`. - `DeleteWorkflowButton` uses `react-query` for mutation and `axios` for API calls. ---- Generated with ❤️ by [ellipsis.dev](https://www.ellipsis.dev) <!-- ELLIPSIS_HIDDEN -->
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.
❌ Changes requested. Reviewed everything up to d42a30a in 13 seconds
More details
- Looked at
244
lines of code in5
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_RDe1Ep5ohL1bVYbH
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
onClick={() => { | ||
deleteWorkflowMutation.mutate(id); | ||
}} | ||
disabled={deleteWorkflowMutation.isPending} |
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.
deleteWorkflowMutation.isPending
should be deleteWorkflowMutation.isLoading
to correctly disable the button during the mutation process.
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.
👍 Looks good to me! Incremental review on d42a30a in 20 seconds
More details
- Looked at
228
lines of code in4
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. skyvern-frontend/src/routes/workflows/editor/DeleteWorkflowButton.tsx:35
- Draft comment:
Consider adding validation to ensureid
is provided and valid before attempting to delete the workflow. This can prevent potential runtime errors. - Reason this comment was not posted:
Confidence changes required:50%
ThedeleteWorkflowMutation
should handle the case where theid
is not provided or is invalid. This can prevent potential runtime errors.
2. skyvern-frontend/src/routes/workflows/editor/DeleteWorkflowButton.tsx:67
- Draft comment:
Review the necessity ofonCloseAutoFocus={(e) => e.preventDefault()}
. Preventing default behavior might not be needed here. - Reason this comment was not posted:
Confidence changes required:33%
TheonCloseAutoFocus
event handler is preventing the default behavior, which might not be necessary. It should be reviewed to ensure it's needed.
3. skyvern-frontend/src/components/icons/GarbageIcon.tsx:2
- Draft comment:
Consider providing a default value forclassName
to avoid potential issues when it's undefined. - Reason this comment was not posted:
Confidence changes required:50%
TheclassName
prop inGarbageIcon
is optional, but it's being used directly without a default value. This might lead to issues ifclassName
is undefined.
Workflow ID: wflow_Wqj9abbc9s4esjdh
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
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.
👍 Looks good to me! Incremental review on c403384 in 9 seconds
More details
- Looked at
12
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_NvEWjbFKzInuOPBB
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Summary:
Added delete functionality to workflows with
DeleteWorkflowButton
, updatedGarbageIcon
, and removedWorkflowsBetaAlertCard
.Key points:
DeleteWorkflowButton
for workflow deletion.GarbageIcon
to makeclassName
prop optional.WorkflowsBetaAlertCard
from workflows.DeleteWorkflowButton
into workflows table.react-query
for mutation andaxios
for API calls.Generated with ❤️ by ellipsis.dev