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 options overwritten by super #17975

Merged
merged 1 commit into from Dec 9, 2014

Conversation

merongivian
Copy link
Contributor

search_field helper had no test for the onsearch option, an indeed it was not working, this pr fix this

@@ -6,11 +6,12 @@ module Tags # :nodoc:
class TextField < Base # :nodoc:
include Placeholderable

def render
def render(&block)
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't need (&block) (it's slower than not having it here) if just using block_given?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@egilburg cool, i'll do it

@rafaelfranca
Copy link
Member

It is hard to tell what is the fix here since we also have a refactoring. Why it was not working?

@rafaelfranca
Copy link
Member

Ok, got it. When we call super we reset the options argument with @options.stringify_keys

@rafaelfranca
Copy link
Member

Could you change your commit message to match the code change?

@merongivian merongivian changed the title add test for #search_field when called with onsearch option Fix options overwritten by super Dec 9, 2014
@merongivian
Copy link
Contributor Author

@rafaelfranca done

rafaelfranca added a commit that referenced this pull request Dec 9, 2014
@rafaelfranca rafaelfranca merged commit 5c7b5dc into rails:master Dec 9, 2014
rafaelfranca added a commit that referenced this pull request Dec 9, 2014
@rafaelfranca
Copy link
Member

Thank you

rafaelfranca added a commit that referenced this pull request Dec 9, 2014
Fix options overwritten by super
Conflicts:
	actionpack/lib/action_view/helpers/tags/text_field.rb
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.

None yet

3 participants