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

Change how parameters look in task node #851

Merged
merged 4 commits into from
Sep 18, 2024

Conversation

ykeremy
Copy link
Contributor

@ykeremy ykeremy commented Sep 18, 2024

🚀 This description was created by Ellipsis for commit ca0bd74

feat: enhance parameter selection UI with MultiSelect in TaskNodeParametersPanel

Summary:

Enhance TaskNodeParametersPanel with MultiSelect for improved parameter selection UI, adding new components and utilities.

Key points:

  • Enhance TaskNodeParametersPanel with MultiSelect for parameter selection.
  • Replace checkboxes with a popover interface.
  • Add MultiSelect in multi-select.tsx and Command in command.tsx.
  • Add utility functions isOutputParameterKey and getBlockNameOfOutputParameterKey in workflowEditorUtils.ts.
  • Update @radix-ui/react-dialog to ^1.1.1 and add cmdk in package.json.
  • Remove autoFocus from EditableNodeTitle.tsx.

Generated with ❤️ by ellipsis.dev

…src/'

<!-- ELLIPSIS_HIDDEN -->

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

feat: add MultiSelect component for parameter selection in TaskNodeParametersPanel

### Summary:
Introduce `MultiSelect` component in `TaskNodeParametersPanel` for enhanced parameter selection UI, replacing checkboxes with a popover interface.

**Key points**:
- Introduce `MultiSelect` component for parameter selection in `TaskNodeParametersPanel`.
- Enhance UI with popover interface, replacing checkboxes.
- Add `MultiSelect` in `multi-select.tsx` and `Command` in `command.tsx`.
- Use `isOutputParameterKey` and `getBlockNameOfOutputParameterKey` in `workflowEditorUtils.ts`.
- Update `@radix-ui/react-dialog` to `^1.1.1` and add `cmdk` in `package.json`.

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

<!-- ELLIPSIS_HIDDEN -->
@ykeremy ykeremy added the sync label Sep 18, 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.

❌ Changes requested. Reviewed everything up to 931da1e in 21 seconds

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

Workflow ID: wflow_4SYJ2PTD3ZcUmU7C


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.

})}
</div>
<MultiSelect
defaultValue={parameters}
Copy link
Contributor

Choose a reason for hiding this comment

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

The defaultValue prop is unnecessary here since value is already being used to control the selected values. Consider removing defaultValue.

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. Incremental review on 931da1e in 27 seconds

More details
  • Looked at 631 lines of code in 5 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern-frontend/src/routes/workflows/editor/nodes/components/EditableNodeTitle.tsx:44
  • Draft comment:
    The autoFocus prop was removed. Ensure this is intentional as it changes the user experience by not focusing the input automatically when editing starts.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The autoFocus prop was removed from the HorizontallyResizingInput component. This might affect user experience if the input was expected to be focused automatically when editing starts. However, since the PR description doesn't mention this change, it might be intentional.

Workflow ID: wflow_N8ZEqYkxnAq3nWqa


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.

})}
</div>
<MultiSelect
defaultValue={parameters}
Copy link
Contributor

Choose a reason for hiding this comment

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

The defaultValue prop is unnecessary here since value is already being used to control the selected values. Consider removing defaultValue.

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 85aa5c3 in 19 seconds

More details
  • Looked at 11 lines of code in 1 files
  • Skipped 1 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. package.json:1
  • Draft comment:
    The package.json is missing standard fields like 'name', 'version', and 'main'. Consider adding these for better package management.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is addressing a potential issue with the package.json file, which is missing standard fields. These fields are generally important for package management and distribution. The comment is actionable as it suggests adding these fields. Since the file is new and only contains dependencies, the comment seems relevant to the changes made.
    The package.json file might be intentionally minimal for a specific reason, such as being used in a non-standard way or as part of a larger configuration. The comment assumes a typical use case without considering other possibilities.
    While there might be specific cases where a minimal package.json is acceptable, the comment is still valid for most standard use cases. It provides a useful suggestion that can improve package management.
    The comment is relevant and provides a useful suggestion for improving the package.json file. It should be kept as it addresses a potential issue with the current file setup.

Workflow ID: wflow_eoHMxyabCUbmKdDt


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.

👍 Looks good to me! Incremental review on c948f34 in 43 seconds

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

Workflow ID: wflow_rkN4p5iPW06dZmoT


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.

👍 Looks good to me! Incremental review on ca0bd74 in 29 seconds

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

Workflow ID: wflow_vJLm1SpjNawFoDC3


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

@msalihaltun msalihaltun merged commit bd12625 into main Sep 18, 2024
2 checks passed
@msalihaltun msalihaltun deleted the salih/task-parameters-update branch September 18, 2024 19:52
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