link_to_function does not work with do block #3093

Closed
PsychoPhobic opened this Issue Sep 21, 2011 · 9 comments

Comments

Projects
None yet
5 participants

Hi,
i am missing "do block" nottation on link_to_function. I wrote some code which is working fine for me atm.

def link_to_function(*args, &block)
#(name, function, html_options={})
if block_given?
options = args.first || {}
html_options = args.second
link_to_function(capture(&block), options, html_options)
else
name = args[0]
function = args[1]
html_options = args[2] || {}

  html_options = convert_options_to_data_attributes(options, html_options)

  onclick = "#{"#{html_options[:onclick]}; " if html_options[:onclick]}#{function}; return false;"
  href = html_options[:href] || '#'

  content_tag(:a, name, html_options.merge(:href => href, :onclick => onclick))
end

end

cause i dont realy know github and and bugtesting maybe somebody can test and push this code

thx

Ninju commented Sep 23, 2011

Just submitted a pull request for this.

Member

arunagw commented Dec 17, 2011

@Ninju Can you paste the Pull Request URL. If merged then we can close this.

Member

arunagw commented Dec 30, 2011

Can you please submit a NEW PR? or Fix the older one? It seems that you have included extra commits.

Member

arunagw commented Jan 24, 2012

Any updates on this guys?? @Ninju can you cleanup your PR #3108

Ninju commented Feb 2, 2012

Yeah, I'm not sure how to fix it, however I'm reluctant to submit a new one as it seems link_to_function was removed in Rails 3 (and I'm sure I agree with all the reasons). I only submitted this PR to preserve consistency while link_to_function was still in existence.

If one of the core guys comes forward and wants me to sort it out for merging into an earlier version of rails (or something), then I'm happy to do so. @arunagw, if you're part of the core team, apologies! Just let me know.

Member

arunagw commented Apr 12, 2012

Hey @Ninju sorry to replying late here. Let me ask somebody to check this for you.

cc/ @drogus

Member

drogus commented Apr 13, 2012

@Ninju to clean the pull request you need to rebase against current master. I see that you have a little mess there so probably the easiest way would be to start branch from fresh master and then cherry-pick commits that you want to include:

git checkout master
git pull
git checkout -b new_branch_for_a_fix
git cherry-pick shas-of-the-commits-you-want-to-incldude

Ninju commented Apr 18, 2012

Ok, all sorted now. New pull request is at #5886

I'll go and close the old one.

@jeremy jeremy closed this Apr 30, 2012

rafaelfranca added a commit to rafaelfranca/omg-rails that referenced this issue Apr 30, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment