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

Remove Proxy Location None from Proxy selector #1216

Merged
merged 1 commit into from
Nov 18, 2024

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Nov 18, 2024

Important

Remove 'None' option from proxy location selector in ProxySelector.tsx, disallowing users from selecting it.

  • Behavior:
    • Removed SelectItem with value ProxyLocation.None from ProxySelector in ProxySelector.tsx.
    • Users can no longer select 'None' as a proxy location.

This description was created by Ellipsis for ffbc5e7. It will automatically update as commits are pushed.

…src/'

<!-- ELLIPSIS_HIDDEN -->

> [!IMPORTANT]
> Remove 'None' option from proxy location selector in `ProxySelector.tsx`.
>
>   - **Behavior**:
>     - Removed `SelectItem` with value `ProxyLocation.None` from `ProxySelector` in `ProxySelector.tsx`.
>     - Users can no longer select 'None' as a proxy location.
>
> <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=Skyvern-AI%2Fskyvern-cloud&utm_source=github&utm_medium=referral)<sup> for 8bb7037c2c19c4d587ab0fb3b4f5bea84a150496. It will automatically update as commits are pushed.</sup>

<!-- ELLIPSIS_HIDDEN -->
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 ffbc5e7 in 11 seconds

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

Workflow ID: wflow_e2XdxEI5KuSLJZrd


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 ffbc5e7 in 43 seconds

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

Workflow ID: wflow_hguBXNeBP0Zxd7pg


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.

@@ -32,7 +32,6 @@ function ProxySelector({ value, onChange }: Props) {
<SelectItem value={ProxyLocation.ResidentialJP}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider updating the onChange function type to handle null values since value can be null. This ensures type safety and prevents potential runtime errors.

Suggested change
<SelectItem value={ProxyLocation.ResidentialJP}>
onChange: (value: ProxyLocation | null) => void;

@msalihaltun msalihaltun merged commit 10d7353 into main Nov 18, 2024
2 checks passed
@msalihaltun msalihaltun deleted the salih/remove-none-proxy branch November 18, 2024 21:22
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