Skip to content

Commit

Permalink
Revert the fix to link_to helper with a block (#121)
Browse files Browse the repository at this point in the history
It has caused several regressions. In order to unblock shipping
other improvements, I propose to revert the fix until we find a
non-regressing one.
  • Loading branch information
deivid-rodriguez authored and javierjulio committed Apr 12, 2019
1 parent 4438bcf commit 4d5858d
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 50 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Master (unreleased)

* Revert [#64][] to fix several regressions, at the cost of reintroducing [#46][]. [#121][] by [@deivid-rodriguez][]

## 1.2.0 [](https://github.com/activeadmin/arbre/compare/v1.2.0.rc1...v1.2.0)

_No changes_.
Expand Down Expand Up @@ -72,11 +74,13 @@ Initial release and extraction from Active Admin
[#36]: https://github.com/activeadmin/arbre/issues/36
[#39]: https://github.com/activeadmin/arbre/issues/39
[#40]: https://github.com/activeadmin/arbre/issues/40
[#46]: https://github.com/activeadmin/arbre/issues/46
[#49]: https://github.com/activeadmin/arbre/issues/49
[#59]: https://github.com/activeadmin/arbre/issues/59
[#64]: https://github.com/activeadmin/arbre/pull/64
[#78]: https://github.com/activeadmin/arbre/pull/78
[#110]: https://github.com/activeadmin/arbre/pull/110
[#121]: https://github.com/activeadmin/arbre/pull/121

[@aramvisser]: https://github.com/aramvisser
[@LTe]: https://github.com/LTe
Expand Down
11 changes: 1 addition & 10 deletions lib/arbre/element.rb
Original file line number Diff line number Diff line change
Expand Up @@ -178,20 +178,11 @@ def method_missing(name, *args, &block)
elsif assigns && assigns.has_key?(name)
assigns[name]
elsif helpers.respond_to?(name)
helper_capture(name, *args, &block)
helpers.send(name, *args, &block)
else
super
end
end

# The helper might have a block that builds Arbre elements
# which will be rendered (#to_s) inside ActionView::Base#capture.
# We do not want such elements added to self, so we push a dummy
# current_arbre_element.
def helper_capture(name, *args, &block)
s = ""
within(Element.new) { s = helpers.send(name, *args, &block) }
s.is_a?(Element) ? s.to_s : s
end
end
end
18 changes: 0 additions & 18 deletions lib/arbre/rails/template_handler.rb
Original file line number Diff line number Diff line change
@@ -1,21 +1,3 @@
# frozen_string_literal: true

ActionView::Base.class_eval do
def capture(*args)
value = nil
buffer = with_output_buffer { value = yield(*args) }

# Override to handle Arbre elements inside helper blocks.
# See https://github.com/rails/rails/issues/17661
# and https://github.com/rails/rails/pull/18024#commitcomment-8975180
value = value.to_s if value.is_a?(Arbre::Element)

if (string = buffer.presence || value) && string.is_a?(String)
ERB::Util.html_escape string
end
end
end

module Arbre
module Rails
class TemplateHandler
Expand Down
15 changes: 0 additions & 15 deletions spec/rails/integration/rendering_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ def render_partial_with_instance_variable
@my_instance_var = "From Instance Var"
render "arbre/page_with_arb_partial_and_assignment"
end

def render_page_with_helpers
render "arbre/page_with_helpers"
end
end


Expand Down Expand Up @@ -100,15 +96,4 @@ def render_page_with_helpers
expect(body).to have_selector("p", text: "Partial: From Instance Var")
end

it "should render a page with helpers" do
get "/test/render_page_with_helpers"
expect(response).to be_successful
expect(body).to eq <<~HTML
<span>before h1 link</span>
<h1><a href="/h1_link_path">h1 link text</a></h1>
<span>before link_to block</span>
<a href="/link_path"> <i class=\"link-class\">Link text</i>
</a><span>at end</span>
HTML
end
end
7 changes: 0 additions & 7 deletions spec/rails/templates/arbre/page_with_helpers.arb

This file was deleted.

0 comments on commit 4d5858d

Please sign in to comment.