Skip to content
Browse files

Deprecate link_to :popup

  • Loading branch information...
1 parent 7f181e4 commit bddd1bb6268e8ac2142c8d8c738929d2f2e8447a @josh josh committed Jan 29, 2010
View
19 actionpack/lib/action_view/helpers/javascript_helper.rb
@@ -189,24 +189,21 @@ def javascript_cdata_section(content) #:nodoc:
protected
def convert_options_to_javascript!(html_options, url = '')
- confirm, popup = html_options.delete("confirm"), html_options.delete("popup")
+ confirm = html_options.delete("confirm")
+
+ if html_options.key?("popup")
+ ActiveSupport::Deprecation.warn(":popup has been deprecated", caller)
+ end
method, href = html_options.delete("method"), html_options['href']
- if popup && method
- raise ActionView::ActionViewError, "You can't use :popup and :method in the same link"
- elsif confirm && popup
- add_confirm_to_attributes!(html_options, confirm)
- html_options["data-popup"] = popup_attributes(popup)
- elsif confirm && method
+ if confirm && method
add_confirm_to_attributes!(html_options, confirm)
add_method_to_attributes!(html_options, method, url)
elsif confirm
add_confirm_to_attributes!(html_options, confirm)
elsif method
add_method_to_attributes!(html_options, method, url)
- elsif popup
- html_options["data-popup"] = popup_attributes(popup)
end
end
@@ -226,10 +223,6 @@ def add_disable_with_to_attributes!(html_options, disable_with)
html_options["data-disable-with"] = disable_with if disable_with
end
- def popup_attributes(popup)
- popup.is_a?(Array) ? "{title: '#{popup.first}', options: '#{popup.last}'}" : "true"
- end
-
def options_for_javascript(options)
if options.empty?
'{}'
View
18 actionpack/lib/action_view/helpers/url_helper.rb
@@ -120,10 +120,6 @@ def url_for(options = {})
# * <tt>:confirm => 'question?'</tt> - This will add a JavaScript confirm
# prompt with the question specified. If the user accepts, the link is
# processed normally, otherwise no action is taken.
- # * <tt>:popup => true || array of window options</tt> - This will force the
- # link to open in a popup window. By passing true, a default browser window
- # will be opened with the URL. You can also specify an array of options
- # that are passed to the <tt>window.open</tt> JavaScript call.
# * <tt>:method => symbol of HTTP verb</tt> - This modifier will dynamically
# create an HTML form and immediately submit the form for processing using
# the HTTP verb specified. Useful for having links perform a POST operation
@@ -136,10 +132,6 @@ def url_for(options = {})
# the request object's methods for <tt>post?</tt>, <tt>delete?</tt> or <tt>put?</tt>.
# * The +html_options+ will accept a hash of html attributes for the link tag.
#
- # You can mix and match the +html_options+ with the exception of
- # <tt>:popup</tt> and <tt>:method</tt> which will raise an
- # <tt>ActionView::ActionViewError</tt> exception.
- #
# ==== Examples
# Because it relies on +url_for+, +link_to+ supports both older-style controller/action/id arguments
# and newer RESTful routes. Current Rails style favors RESTful routes whenever possible, so base
@@ -203,17 +195,11 @@ def url_for(options = {})
# link_to "Nonsense search", searches_path(:foo => "bar", :baz => "quux")
# # => <a href="/searches?foo=bar&amp;baz=quux">Nonsense search</a>
#
- # The three options specific to +link_to+ (<tt>:confirm</tt>, <tt>:popup</tt>, and <tt>:method</tt>) are used as follows:
+ # The three options specific to +link_to+ (<tt>:confirm</tt> and <tt>:method</tt>) are used as follows:
#
# link_to "Visit Other Site", "http://www.rubyonrails.org/", :confirm => "Are you sure?"
# # => <a href="http://www.rubyonrails.org/" onclick="return confirm('Are you sure?');">Visit Other Site</a>
#
- # link_to "Help", { :action => "help" }, :popup => true
- # # => <a href="/testing/help/" onclick="window.open(this.href);return false;">Help</a>
- #
- # link_to "View Image", @image, :popup => ['new_window_name', 'height=300,width=600']
- # # => <a href="/images/9" onclick="window.open(this.href,'new_window_name','height=300,width=600');return false;">View Image</a>
- #
# link_to "Delete Image", @image, :confirm => "Are you sure?", :method => :delete
# # => <a href="/images/9" onclick="if (confirm('Are you sure?')) { var f = document.createElement('form');
# f.style.display = 'none'; this.parentNode.appendChild(f); f.method = 'POST'; f.action = this.href;
@@ -308,7 +294,7 @@ def button_to(name, options = {}, html_options = {})
url = options.is_a?(String) ? options : self.url_for(options)
name ||= url
-
+
convert_options_to_javascript!(html_options, url)
html_options.merge!("type" => "submit", "value" => name)
View
30 actionpack/test/template/url_helper_test.rb
@@ -183,32 +183,6 @@ def test_link_tag_with_javascript_confirm
)
end
- def test_link_tag_with_popup
- assert_dom_equal(
- "<a href=\"http://www.example.com\" data-popup=\"true\">Hello</a>",
- link_to("Hello", "http://www.example.com", :popup => true)
- )
- assert_dom_equal(
- "<a href=\"http://www.example.com\" data-popup=\"true\">Hello</a>",
- link_to("Hello", "http://www.example.com", :popup => 'true')
- )
- assert_dom_equal(
- "<a href=\"http://www.example.com\" data-popup=\"{title: 'window_name', options: 'width=300,height=300'}\">Hello</a>",
- link_to("Hello", "http://www.example.com", :popup => ['window_name', 'width=300,height=300'])
- )
- end
-
- def test_link_tag_with_popup_and_javascript_confirm
- assert_dom_equal(
- "<a href=\"http://www.example.com\" data-confirm=\"Fo\' sho\'?\" data-popup=\"true\">Hello</a>",
- link_to("Hello", "http://www.example.com", { :popup => true, :confirm => "Fo' sho'?" })
- )
- assert_dom_equal(
- "<a href=\"http://www.example.com\" data-confirm=\"Are you serious?\" data-popup=\"{title: 'window_name', options: 'width=300,height=300'}\">Hello</a>",
- link_to("Hello", "http://www.example.com", { :popup => ['window_name', 'width=300,height=300'], :confirm => "Are you serious?" })
- )
- end
-
def test_link_tag_using_post_javascript
assert_dom_equal(
"<a href='http://www.example.com' data-url='http://www.example.com' data-method=\"post\">Hello</a>",
@@ -245,10 +219,6 @@ def test_link_tag_using_delete_javascript_and_href_and_confirm
)
end
- def test_link_tag_using_post_javascript_and_popup
- assert_raise(ActionView::ActionViewError) { link_to("Hello", "http://www.example.com", :popup => true, :method => :post, :confirm => "Are you serious?") }
- end
-
def test_link_tag_using_block_in_erb
__in_erb_template = ''

4 comments on commit bddd1bb

@sikachu
Ruby on Rails member

actionpack/lib/action_view/helpers/url_helper.rb #198
I think the doc should be 'The two options specific to +link_to+' instead :)

BTW, so now we're back to :target => "_blank" ?

@JeffCohen

Why is the :popup option being removed?

@msheakoski

@JeffCohen: The answer lies in SHA: a06c47a

@msheakoski

Let's try that again: SHA: a06c47a

Please sign in to comment.
Something went wrong with that request. Please try again.