API for operation with modal dialogs #1322

Merged
merged 5 commits into from Jul 1, 2014

Projects

None yet

4 participants

Collaborator
twalpole commented Jun 3, 2014

This extends on PR #1037

Collaborator
abotalov commented Jun 3, 2014

IMO the following code isn't a good read:

respond_to_prompt 'Linus Torvalds' do
  click_link('Show Prompt About Linux')
end

When I read it it wasn't clear to me if 'Linus Torvalds' is a message shown in the modal or the text entered into the text input.

Do you see see some way to solve it?

Collaborator
abotalov commented Jun 3, 2014

What about?

respond_to_prompt(input: 'Linus Torvalds') do
  click_link('Show Prompt About Linux')
end
Collaborator
twalpole commented Jun 3, 2014

I don't have an issue with respond_to_prompt - its the only one of the new methods that has a required parameter and I think its relatively clear that parameter is what will be entered into the prompt

Collaborator
twalpole commented Jun 3, 2014

@abotalov hmmm -- I'm having second thoughts about respond_to_prompt -- maybe the parameter should be :with so it matches up with fill_in --- I need to think about this a bit longer

Collaborator
abotalov commented Jun 3, 2014

hmmm -- I'm having second thoughts about respond_to_prompt -- maybe the parameter should be :with so it matches up with fill_in --- I need to think about this a bit longer

I think :with is not a good parameter too:

respond_to_prompt(with: 'Linus Torvalds') do
  click_link('Show Prompt About Linux')
end

IMO it may mean Respond to prompt that contains 'Linus Torvalds'.

Contributor
mhoran commented Jun 26, 2014

cc @jferris, this is a continuation of #1037, which I believe you had spent some time looking into. I've not gotten up to speed on where this API is currently, but this is something we should have on the radar for capybara-webkit.

@twalpole twalpole merged commit c589f6a into master Jul 1, 2014

1 check was pending

continuous-integration/travis-ci The Travis CI build is in progress
Details
@abotalov abotalov commented on the diff Jul 1, 2014
lib/capybara/driver/base.rb
+ # @raise [Capybara::ModalNotFound] if modal dialog hasn't been found
+ #
+ def accept_modal(type, options={}, &blk)
+ raise Capybara::NotSupportedByDriverError, 'Capybara::Driver::Base#accept_modal'
+ end
+
+ ##
+ #
+ # Execute the block, and then dismiss the modal opened.
+ # @param type [:alert, :confirm, :prompt]
+ # @option options [Numeric] :wait How long to wait for the modal to appear after executing the block.
+ # @option options [String, Regexp] :text Text to verify is in the message shown in the modal
+ # @return [String] the message shown in the modal
+ # @raise [Capybara::ModalNotFound] if modal dialog hasn't been found
+ #
+ def dismiss_modal(type, &blk)
abotalov
abotalov Jul 1, 2014 Collaborator

options = {}

@abotalov abotalov commented on the diff Jul 1, 2014
lib/capybara/session.rb
@@ -525,6 +529,112 @@ def evaluate_script(script)
##
#
+ # Execute the block, accepting a alert.
+ #
+ # @overload accept_alert(text, options = {}, &blk)
+ # @param text [String, Regexp] Text or regex to match against the text in the modal. If not provided any prompt modal is matched
abotalov
abotalov Jul 1, 2014 Collaborator

Writing overloads in such way doesn't produce correct documentation -http://rubydoc.info/github/jnicklas/capybara/master/Capybara/Session#accept_alert-instance_method (@param text isn't documented)

twalpole
twalpole Jul 1, 2014 Collaborator

hmmm -- generating the docs locally (yard 0.8.7.4) produces the correct output for me -

  • (String) accept_confirm(text, options = {}, &blk) - (String) accept_confirm(options = {}, &blk)

Execute the block, accepting a confirm.

Overloads:

- (String) accept_confirm(text, options = {}, &blk)

Parameters:
    text (String, Regexp) —

    Text or regex to match against the text in the modal. If not provided any prompt modal is matched

Parameters:

options (Hash) (defaults to: {}) —

a customizable set of options

Options Hash (options):

:wait (Numeric) —

How long to wait for the modal to appear after executing the block.

Returns:

(String) —

the message shown in the modal

Raises:

(Capybara::ModalNotFound) —

if modal dialog hasn't been found
twalpole
twalpole Jul 1, 2014 Collaborator

Its correct here too - http://rubydoc.info/github/jnicklas/capybara/master/Capybara/Session:accept_alert
not sure why its correct in one place but not the other

@abotalov abotalov commented on the diff Jul 1, 2014
lib/capybara/spec/session/accept_alert_spec.rb
+ end
+
+ it "should accept the alert if the text matches" do
+ @session.accept_alert 'Alert opened' do
+ @session.click_link('Open alert')
+ end
+ expect(@session).to have_xpath("//a[@id='open-alert' and @opened='true']")
+ end
+
+ it "should not accept the alert if the text doesnt match" do
+ expect do
+ @session.accept_alert 'Incorrect Text' do
+ @session.click_link('Open alert')
+ end
+ end.to raise_error(Capybara::ModalNotFound)
+ # @session.accept_alert {} # clear the alert so browser continues to function
abotalov
abotalov Jul 1, 2014 Collaborator

Should it be commented?

twalpole
twalpole Jul 1, 2014 Collaborator

yes - its handled in reset now, so the line can eventually go away

@twalpole twalpole deleted the modal branch Aug 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment