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

[py] Support move_to_location on action_chains #14376

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

kkb912002
Copy link

@kkb912002 kkb912002 commented Aug 12, 2024

User description

Description

Expose an interface to move the pointer to the given coordinates in ActionChains.

Motivation and Context

In the current state, accessing the internal w3c_actions member is necessary to move the pointer to the given coordinates.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Enhancement


Description

  • Introduced a new method move_to_location in the ActionChains class to move the mouse pointer to specified coordinates.
  • Updated the w3c_actions to support the new method.
  • Added docstring for the move_to_location method explaining its parameters and usage.

Changes walkthrough 📝

Relevant files
Enhancement
action_chains.py
Add move_to_location method to ActionChains class               

py/selenium/webdriver/common/action_chains.py

  • Added move_to_location method to ActionChains class.
  • Allows moving the mouse pointer to specified coordinates.
  • Includes docstring for the new method.
  • +13/-0   

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Missing Input Validation
    The method move_to_location does not validate that the x and y parameters are positive integers. This could lead to unexpected behavior if negative or non-integer values are passed.

    Copy link
    Contributor

    codiumai-pr-agent-pro bot commented Aug 12, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    ✅ Add input validation for non-negative integers in move_to_location
    Suggestion Impact:The suggestion to add input validation for non-negative integers in the move_to_location method was implemented.

    code diff:

    +        if x < 0 or y < 0:
    +            raise ValueError("Coordinates x and y must be non-negative integers") 
    +

    Consider adding input validation for the x and y parameters in the move_to_location
    method to ensure they are non-negative integers. This can prevent unintended
    behavior when negative coordinates are passed.

    py/selenium/webdriver/common/action_chains.py [246-252]

     def move_to_location(self, x: int, y: int) -> ActionChains:
    +    if x < 0 or y < 0:
    +        raise ValueError("Coordinates x and y must be non-negative integers")
         """Moving the mouse to the coordinates.
     
         :Args:
          - x: X coordinate to move to, as a positive integer.
          - y: Y coordinate to move to, as a positive integer.
         """
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Adding input validation for the x and y parameters ensures that the method behaves as expected and prevents unintended behavior when negative coordinates are passed. This is a significant improvement for robustness and reliability.

    9

    @AutomatedTester
    Copy link
    Member

    Can you add a test to make sure it works

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

    Successfully merging this pull request may close these issues.

    2 participants