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

fix assert_select wanting something to call .css on #39

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

Conversation

grosser
Copy link
Contributor

@grosser grosser commented Mar 15, 2016

@grosser
Copy link
Contributor Author

grosser commented Mar 15, 2016

otherwise

assert_select_rjs do
        assert_select "#1"
end

fails with nasty undefined method css for Array :(

@pschambacher
Copy link

Yes please

@grosser
Copy link
Contributor Author

grosser commented Jun 15, 2017

@rafaelfranca this is some old 💩 but we still need that :D

Copy link
Member

@rafaelfranca rafaelfranca left a comment

Choose a reason for hiding this comment

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

Could you add a test case?

@@ -126,9 +126,15 @@ def assert_select_rjs(*args, &block)
if matches
assert true # to count the assertion
if block_given? && !([:remove, :show, :hide, :toggle].include? rjs_type)
to_select = if ActionPack::VERSION::STRING > "4.2.0"
Nokogiri::HTML(matches.map(&:to_s).join)
Copy link
Member

Choose a reason for hiding this comment

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

Does this gem depends on Nokogiri? I don't think so. If we want to use nokogiri we need to make it a dependency

@grosser
Copy link
Contributor Author

grosser commented Jun 15, 2017

yeah I guess you are right ... this is too dirty
... I'll just leave this here as docs and try again to kill prototype instead :/

@rafaelfranca
Copy link
Member

What I did in Shopify was to keep the prototype JavaScript code and just remove all ruby methods calls that we had.

@grosser
Copy link
Contributor Author

grosser commented Jun 15, 2017 via email

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.

3 participants