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

Declare Session#send_keys #2422

Merged
merged 1 commit into from
Nov 17, 2020
Merged

Conversation

seanpdoyle
Copy link
Contributor

When covering hotkeys and tab-navigation with test coverage, sending
keys via particular elements (via Node#send_keys) can be tedious.

As an alternative, invoke Session#send_keys to delegate to the element
with current focus. In most (all?) browsers, document.activeElement
will return the <body> element when there are no elements with focus.

In Selenium-enabled environments, Session#send_keys delegates to the
current element with focus (via document.activeElement). Treat
Session#send_keys as a no-operation in Rack Test environments.

@@ -303,6 +303,10 @@ def go_forward
driver.go_forward
end

def send_keys(*args, **kw_args)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a way to share documentation with the Node#send_keys version, or would we need to declare comment docs here as well?

Copy link
Member

Choose a reason for hiding this comment

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

Just link to Node#send_keys docs (See ...)

spec/rack_test_spec.rb Outdated Show resolved Hide resolved
@twalpole
Copy link
Member

I need to check but I think the selenium api might provide access to the active element which would be preferable to use instead of evaluate_script

@twalpole
Copy link
Member

Along with the current code send_keys would need to be added to Capybara::Driver::Base

@seanpdoyle seanpdoyle force-pushed the session-send-keys branch 3 times, most recently from 806fb2c to 35bcecd Compare November 10, 2020 22:20
@@ -303,6 +303,14 @@ def go_forward
driver.go_forward
end

##
# @!method send_keys
# @see Capybara::Node::Element#send_keys
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@twalpole is this the correct Rubydoc syntax to link?

@@ -50,6 +50,10 @@ def refresh
browser.refresh
end

def send_keys(*)
raise Capybara::NotSupportedByDriverError, "RackTest driver doesn't have the concept of an active element, please call send_keys on a specific element"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@twalpole I've updated this error with the message you've suggested.

Reading it back: is it true that Rack Test supports key handlers?

Copy link
Member

Choose a reason for hiding this comment

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

Good catch -- I hadn't thought that through -- no it doesn't, and calling send_keys on an element would just raise a NotImplementedError, so this might as well be the same via the Capybara::Driver::Base default -- sorry about that.

@seanpdoyle seanpdoyle force-pushed the session-send-keys branch 4 times, most recently from 35eb1e3 to 571d35b Compare November 12, 2020 21:31
@@ -133,6 +133,10 @@ def evaluate_async_script(script, *args)
unwrap_script_result(result)
end

def send_keys(*args)
active_element.send_keys(*args)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@twalpole there is a test failure in CI:

1) Capybara::Session with remote Chrome with selenium driver #send_keys defaults to sending keys to the document.activeElement

Failure/Error: raise Capybara::NotSupportedByDriverError, 'Capybara::Driver::Base#send_keys'

  Capybara::NotSupportedByDriverError:
    Capybara::Driver::Base#send_keys
  Shared Example Group: "Capybara::Session" called from ./spec/selenium_spec_chrome_remote.rb:73

# ./lib/capybara/driver/base.rb:63:in `send_keys'
# ./lib/capybara/session.rb:311:in `send_keys'
# ./lib/capybara/dsl.rb:58:in `block (2 levels) in <module:DSL>'
# ./spec/shared_selenium_session.rb:231:in `block (4 levels) in <top (required)>'

It seems like there are Selenium-enabled tests that are not using the Selenium::Driver, and appear to be using Driver::Base or RackTest::Driver. Are the tests I've added missing a crucial setup step?

Copy link
Member

Choose a reason for hiding this comment

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

@seanpdoyle You'll need to tag the test(s) as requiring send_keys - like https://github.com/teamcapybara/capybara/blob/master/lib/capybara/spec/session/node_spec.rb#L1062 - although I am curious why the remote chrome driver version of the test would be failing.

Copy link
Member

@twalpole twalpole Nov 16, 2020

Choose a reason for hiding this comment

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

OH - I see the issue - your test isn't calling send_keys on the session - https://github.com/teamcapybara/capybara/pull/2422/files#diff-0794c7a2e4cba5e9448e4dfb1bd723358f79c2e7a1a3a0fc879c6a8e86e793c6R231

This is triggering the creation of a new default session which defaults to the RackTest driver

Changing to session.send_keys(:tab) should make the test correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦 , thank you!

With the way that the DSL module delegates to #page, will that style of invocation work in consumer applications' test suites (outside this test suite)?

Copy link
Member

Choose a reason for hiding this comment

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

Yes -- it's just that in the tests the session is passed in rather than letting a default one be created since it makes it easier for testing against multiple drivers or with multiple sessions. In most users case they're working with Capybara.current_session and wouldn't be passing the actual session around.

When covering hotkeys and tab-navigation with test coverage, sending
keys via particular elements (via `Node#send_keys`) can be tedious.

As an alternative, invoke `Session#send_keys` to delegate to the element
with current focus. In most (all?) browsers, `document.activeElement`
will return the `<body>` element when there are no elements with focus.

In Selenium-enabled environments, `Session#send_keys` delegates to the
current element with focus (via [document.activeElement][]).

Selenium-Webdriver provides a public method for reading the browser's
active element, so use that in place of `Driver#evaluate_script`.

In Rack Test environments, `Session#send_keys` raises an error.

[active_element]: https://www.selenium.dev/documentation/en/webdriver/web_element/#get-active-element
[document.activeElement]: https://developer.mozilla.org/en-US/docs/Web/API/DocumentOrShadowRoot/activeElement
@twalpole twalpole merged commit 8c4bffd into teamcapybara:master Nov 17, 2020
@twalpole
Copy link
Member

Thank you

@seanpdoyle seanpdoyle deleted the session-send-keys branch November 17, 2020 01:31
twalpole added a commit that referenced this pull request Jan 17, 2021
* master:
  Protect against optimization returning nil
  Update CI config for Ruby 3.0 release and update rubocop
  Wrapping injected disable_animation script in CDATA to prevent XHTML/XML parser errors
  History.md: fix typo [ci skip]
  Gem updates
  Update History.md for release date
  Update README for 3.34 [ci skip]
  Update History.md and version to prepare for 3.34.0 release
  Rubocop cleanup
  Update History.md
  Declare `Session#send_keys` (#2422)
  Minor rubocop updates
  Fix bug in Capybara::Node::Element#drop causing Pathname drops to be ignored
  Update History.md
  Support multiple labels when using allow_label_click on a checkbox - Issue #2421
seanpdoyle added a commit to seanpdoyle/capybara that referenced this pull request Aug 14, 2021
As a [follow-up][] to [teamcapybara#2422][], expose the [document.activeElement][]
(that is, the element with focus) via the `Session`.

When called from Rack Test environments, `Session#active_element` raises
an error.

[Document.activeElement]: https://developer.mozilla.org/en-US/docs/Web/API/Document/activeElement
[follow-up]: teamcapybara#2422 (comment)
[teamcapybara#2422]: teamcapybara#2422
[document.activeElement]: https://developer.mozilla.org/en-US/docs/Web/API/Document/activeElement
seanpdoyle added a commit to seanpdoyle/capybara that referenced this pull request Aug 14, 2021
As a [follow-up][] to [teamcapybara#2422][], expose the [document.activeElement][]
(that is, the element with focus) via the `Session`.

When called from Rack Test environments, `Session#active_element` raises
an error.

[Document.activeElement]: https://developer.mozilla.org/en-US/docs/Web/API/Document/activeElement
[follow-up]: teamcapybara#2422 (comment)
[teamcapybara#2422]: teamcapybara#2422
[document.activeElement]: https://developer.mozilla.org/en-US/docs/Web/API/Document/activeElement
seanpdoyle added a commit to seanpdoyle/capybara that referenced this pull request Aug 15, 2021
As a [follow-up][] to [teamcapybara#2422][], expose the [document.activeElement][]
(that is, the element with focus) via the `Session`.

When called from Rack Test environments, `Session#active_element` raises
an error.

[Document.activeElement]: https://developer.mozilla.org/en-US/docs/Web/API/Document/activeElement
[follow-up]: teamcapybara#2422 (comment)
[teamcapybara#2422]: teamcapybara#2422
[document.activeElement]: https://developer.mozilla.org/en-US/docs/Web/API/Document/activeElement
seanpdoyle added a commit to seanpdoyle/capybara that referenced this pull request Aug 15, 2021
As a [follow-up][] to [teamcapybara#2422][], expose the [document.activeElement][]
(that is, the element with focus) via the `Session`.

When called from Rack Test environments, `Session#active_element` raises
an error.

[Document.activeElement]: https://developer.mozilla.org/en-US/docs/Web/API/Document/activeElement
[follow-up]: teamcapybara#2422 (comment)
[teamcapybara#2422]: teamcapybara#2422
[document.activeElement]: https://developer.mozilla.org/en-US/docs/Web/API/Document/activeElement
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