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 adder node after the first node is added #796

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

ykeremy
Copy link
Contributor

@ykeremy ykeremy commented Sep 9, 2024

🚀 This description was created by Ellipsis for commit a9cfa49

Summary:

Add nodeAdder node and edge in FlowRenderer.tsx for the first node in an empty workflow.

Key points:

  • Add nodeAdder node and edge in FlowRenderer.tsx when the first node is added to an empty workflow.
  • Modify addNode function in skyvern-frontend/src/routes/workflows/editor/FlowRenderer.tsx.
  • Ensure nodeAdder is present for further node additions.

Generated with ❤️ by ellipsis.dev

…src/'

<!-- ELLIPSIS_HIDDEN -->

| 🚀 | This description was created by [Ellipsis](https://www.ellipsis.dev) for commit 0a993fa8ef99f15c6aa4bf39d7cd475539528163  |
|--------|--------|

### Summary:
Add `nodeAdder` node and edge in `FlowRenderer.tsx` when the first node is added to an empty workflow.

**Key points**:
- Modify `addNode` function in `skyvern-frontend/src/routes/workflows/editor/FlowRenderer.tsx`.
- Add `nodeAdder` node and edge when the first node is added to an empty workflow.
- Ensure `nodeAdder` is present for further node additions.

----
Generated with ❤️ by [ellipsis.dev](https://www.ellipsis.dev)

<!-- ELLIPSIS_HIDDEN -->
@ykeremy ykeremy added the sync label Sep 9, 2024
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 a9cfa49 in 20 seconds

More details
  • Looked at 32 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern-frontend/src/routes/workflows/editor/FlowRenderer.tsx:192
  • Draft comment:
    Consider moving the if (nodes.length === 0) block before modifying newNodesAfter to ensure the nodeAdder is added only when the workflow is initially empty.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is suggesting a potential improvement in the logic flow. It is related to the new code added in the diff, and it is not asking for confirmation or speculative. It seems to be a valid suggestion for a code quality refactor.
    The comment might be unnecessary if the current logic already handles the intended behavior correctly. It assumes that moving the block will improve the logic without concrete evidence.
    The suggestion seems to be about improving code clarity and ensuring the logic is straightforward, which is generally beneficial.
    The comment is about a change made in the diff and suggests a potential improvement in the logic. It should be kept as it is actionable and clear.

Workflow ID: wflow_ENk1G7AYMMpfo45f


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 a9cfa49 in 21 seconds

More details
  • Looked at 33 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_xbCSMLAdi2EBjY0C


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.

@@ -188,6 +188,27 @@ function FlowRenderer({
...newNodes,
...nodes.slice(previousNodeIndex + 1),
];

if (nodes.length === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The logic for adding a nodeAdder node and edge when the first node is added is duplicated. Consider refactoring to avoid redundancy.

@msalihaltun msalihaltun merged commit d6193bc into main Sep 9, 2024
2 checks passed
@msalihaltun msalihaltun deleted the salih/fix-cant-add-second-node branch September 9, 2024 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants