Skip to content

Move click to action chain #8

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.

Sign up for GitHub

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

Open
vkolagi wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Move click to action chain #8

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions basepage/base_page.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,13 +250,12 @@ def enter_text(self, locator, text, with_click=True, with_clear=False, with_ente
if not isinstance(element, WebElement):
element = self.get_visible_element(locator, params)

if with_click:
self.click(element)

actions = ActionChains(self.driver)
if 'explorer' in self.driver.name and "@" in str(text):
actions = BasePage.handle_at_sign_for_ie(text, actions)
else:
if with_click:
actions.click(element)
Comment on lines +257 to +258
Copy link
Contributor

@BeyondEvil BeyondEvil Mar 3, 2020 •
edited
Loading

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

@vkolagi vkolagi Mar 3, 2020

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

@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.

@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. :)

actions.send_keys_to_element(element, text)

if with_clear:
Expand Down