Skip to content

Conversation

@shbenzer
Copy link
Contributor

@shbenzer shbenzer commented Aug 9, 2024

User description

I have added the new expected condition "elements_not_overlapping". This condition checks that two elements are not overlapping on the dom based on their .rect specs.

Description

  1. Created the function elements_not_overlapping. This EC returns true if two WebElements are not overlapping based on their .rect specs or at least one element is stale.
  2. Added a pytest test_expected_conditions_elements_not_overlapping to ensure functionality runs as expected
  3. Gave the parent div of "clickToShow" and "clickToHide" in JavascriptPage.html the id "clickToShowParent"

Motivation and Context

This update provides added functionality and checks for cases where two elements overlap in the DOM temporarily. Examples include: elements dynamically overlapping during scrolling, multiple instances of loading screens occurring simultaneously on the page, etc.

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.

PR Type

Enhancement, Tests


Description

  • Added a new expected condition elements_not_overlapping to check if two elements do not overlap based on their rectangle specifications.
  • Implemented a test case test_expected_condition_elements_not_overlapping to ensure the new condition works as expected.
  • Updated javascriptPage.html to include an id attribute for a div element to facilitate testing.

Changes walkthrough 📝

Relevant files
Enhancement
expected_conditions.py
Add `elements_not_overlapping` expected condition function

py/selenium/webdriver/support/expected_conditions.py

  • Added elements_not_overlapping function to check if two elements do
    not overlap.
  • The function returns true if either element is stale or if their
    rectangles do not intersect.
  • +21/-0   
    javascriptPage.html
    Add id to div element in javascriptPage.html                         

    common/src/web/javascriptPage.html

    • Added id attribute to a div element for testing purposes.
    +1/-1     
    Tests
    webdriverwait_tests.py
    Add test for `elements_not_overlapping` expected condition

    py/test/selenium/webdriver/common/webdriverwait_tests.py

  • Added a test test_expected_condition_elements_not_overlapping to
    verify the new expected condition.
  • The test checks both non-overlapping and overlapping scenarios.
  • +17/-0   

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

    …s_not_overlapping, and added an id to javascriptPage.html
    @CLAassistant
    Copy link

    CLAassistant commented Aug 9, 2024

    CLA assistant check
    Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
    1 out of 2 committers have signed the CLA.

    ✅ shbenzer
    ❌ Simon Benzer


    Simon Benzer seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
    You have signed the CLA already but the status is still pending? Let us recheck it.

    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Aug 9, 2024

    PR Reviewer Guide 🔍

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

    Code Clarity
    The function elements_not_overlapping could be improved by adding more detailed docstrings explaining the parameters and the return type. Additionally, the overlap logic could be encapsulated in a separate helper function for better readability and potential reuse.

    Error Handling
    Consider handling potential exceptions that might occur when accessing the rect attribute of WebElements, which might not always be present or could lead to attribute errors if the elements are not rendered or accessible.

    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Aug 9, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Add null checks for element1 and element2 to prevent attribute access on None

    To prevent potential errors, ensure that element1 and element2 are not None before
    accessing their rect attribute in the _predicate function.

    py/selenium/webdriver/support/expected_conditions.py [401-402]

    +if element1 is None or element2 is None:
    +    return False
     rect1 = element1.rect
     rect2 = element2.rect
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: This suggestion addresses a potential bug by ensuring that element1 and element2 are not None before accessing their rect attribute, which can prevent runtime errors.

    9
    Use .get() for dictionary access to prevent KeyError

    Replace the direct dictionary key access with the .get() method to avoid potential
    KeyError if the keys are missing.

    py/selenium/webdriver/support/expected_conditions.py [403-406]

    -return (rect1['x'] + rect1['width'] < rect2['x'] or
    -        rect2['x'] + rect2['width'] < rect1['x'] or
    -        rect1['y'] + rect1['height'] < rect2['y'] or
    -        rect2['y'] + rect2['height'] < rect1['y'])
    +return (rect1.get('x', 0) + rect1.get('width', 0) < rect2.get('x', 0) or
    +        rect2.get('x', 0) + rect2.get('width', 0) < rect1.get('x', 0) or
    +        rect1.get('y', 0) + rect1.get('height', 0) < rect2.get('y', 0) or
    +        rect2.get('y', 0) + rect2.get('height', 0) < rect1.get('y', 0))
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: This suggestion prevents potential KeyError exceptions by using the .get() method for dictionary access, which enhances the robustness of the code.

    8
    Enhancement
    Refactor overlapping logic into a separate function for better clarity and reusability

    Refactor the overlapping logic into a separate function to improve code reusability
    and clarity.

    py/selenium/webdriver/support/expected_conditions.py [403-406]

    -return (rect1['x'] + rect1['width'] < rect2['x'] or
    -        rect2['x'] + rect2['width'] < rect1['x'] or
    -        rect1['y'] + rect1['height'] < rect2['y'] or
    -        rect2['y'] + rect2['height'] < rect1['y'])
    +def are_elements_overlapping(rect1, rect2):
    +    return not (rect1['x'] + rect1['width'] < rect2['x'] or
    +                rect2['x'] + rect2['width'] < rect1['x'] or
    +                rect1['y'] + rect1['height'] < rect2['y'] or
    +                rect2['y'] + rect2['height'] < rect1['y'])
    +return are_elements_overlapping(rect1, rect2)
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Refactoring the overlapping logic into a separate function improves code clarity and reusability, but it does not address any immediate functional issues.

    7
    Maintainability
    Improve code readability by adding a space after the comma in the function definition

    Consider adding a space after the comma in the function definition for
    elements_not_overlapping to maintain consistent code style and improve readability.

    py/selenium/webdriver/support/expected_conditions.py [389]

    -def elements_not_overlapping(element1: WebElement, element2:WebElement) -> Callable[[Any], bool]:
    +def elements_not_overlapping(element1: WebElement, element2: WebElement) -> Callable[[Any], bool]:
     
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: This suggestion improves code readability and maintains consistent code style, but it does not address any functional issues.

    5

    @shbenzer
    Copy link
    Contributor Author

    shbenzer commented Aug 9, 2024

    I'm not sure why it thinks two people committed these changes; Simon Benzer is shbenzer is me

    @shbenzer
    Copy link
    Contributor Author

    EC support not currently being considered - can reopen/recreate later if necessary

    @shbenzer shbenzer closed this Oct 30, 2024
    @shbenzer shbenzer deleted the feature_EC_elements_not_overlapping branch January 16, 2025 00:38
    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