Skip to content

Conversation

@vkolagi
Copy link
Contributor

@vkolagi vkolagi commented Mar 2, 2020

Move the click to action chain instead of clicking it via regular element click. This keeps the actions atomic and also handles weird bug where the focus from text box jumps in legacy/older for various rendering reasons.

Move the click to action chain instead of clicking it via regular element click. This keeps the actions atomic and also handles weird bug where the focus from text box jumps in legacy/older for various rendering reasons.
Comment on lines +257 to +258
if with_click:
actions.click(element)
Copy link
Contributor

@BeyondEvil BeyondEvil Mar 3, 2020

Choose a reason for hiding this comment

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

This seems like a weird placement of theclick. Isn't the initial click needed for the input in the if-statement to work? 🤔

Also, the actions.click is fundamentally different from the element.click. So this might not be backwards compat.

I would keep the "old" click, and introduce a use_actions_for_click boolean that defaults to false.

Like so:

def enter_text(self, locator, text, with_click=True, with_clear=False, with_enter=False, params=None, use_actions_for_click=False):
        element = locator
        if not isinstance(element, WebElement):
            element = self.get_visible_element(locator, params)

        actions = ActionChains(self.driver)
        if with_click:
           if use_actions_for_click:
               actions.click(element)
           else:
               element.click()

        if 'explorer' in self.driver.name and "@" in str(text):
            actions = BasePage.handle_at_sign_for_ie(text, actions)
        else:
            actions.send_keys_to_element(element, text)

        if with_clear:
            element.clear()
            # Click to regain focus as clear does clear w/ blur by design -
            # https://w3c.github.io/webdriver/webdriver-spec.html#element-clear
            self.click(element)

        if with_enter:
            actions.send_keys(Keys.ENTER)

        actions.perform()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am curious about how it breaks the backward compatibility. The backward versions of selenium supports action chains send_keys but not click ? I agree the if placement can stay at where it is now.

Copy link
Contributor

Choose a reason for hiding this comment

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

@vkolagi

Oh, I didn't mean it breaks backwards compat with Selenium, I meant with the test suite. Originally, before this was a pip-package, the click was action chains. But that caused a lot of tests to be flaky due to some inconsistency in the AC implementation.

So you risk bringing that flakyness back by replacing the click. :)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants