8f5553306c2cf7f4b14153f6117f8e9b

Probably a more efficient way to accomplish this.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
def button(title, values)
  # title: Accepts a string that will appear on the button
  # values: accepts the possible optional parameters.
    # category: Accepts one of the following:
      # - primary: Standard "green" button with white text
      # - primary_secure: Standard "green" button with "lock" icon
      # - primary_mini: Yes / No small "green" buttons
      # - secondary: Standard "blue" button with white text
      # - secondary_secure:
      # - secondary_prev: Standard "blue" button with left arrow
      # - secondary_next: Standard "blue" button with right arrow
      # - purchase: Standard "gold" purchase button
      # - purchase_tight: Like purchase but half the padding
    # type: Accepts "button", "submit" or "reset" -- Defaults to "button" if not passed.
    # onclick: Will accept an onclick event.
    # url: Will accept a URL or path to another page.
    # class: Will accept an additional class to assign to the button.
    # id: Will accept an ID to attach to the button.
  
  #Checks the category for certain keywords so the proper icon can be inlined in to the button.
  if values[:category] then
    button_icon = "buttons_secure" if values[:category].match("secure")
    button_icon = "buttons_prev" if values[:category].match("prev")
    button_icon = "buttons_next" if values[:category].match("next")
    img_tag = content_tag(:img, '', :src => "../images/" + button_icon + ".gif")
  else
    img_tag = ""
    # if no button category is given, it defaults to primary.
    values[:category] = "primary"
  end
  
  # The input type defaults to "button" if not given.
  if !values[:type] then
    values[:type] = "button"
  end
  
  if values[:onclick] then
    onclick_event = values[:onclick]
  elsif values[:url] then
    onclick_event = "window.location=" + values[:url]
  end
  
  # The tag is created to accomplish the sliding door technique.
  div_tag = content_tag(:div, '&nbsp', :class => 'endcap')
  
  content_tag(:div, content_tag(
                                :input, img_tag + div_tag,
                                :value => title,
                                :type => values[:type],
                                :onclick => onclick_event,
                                :class => values[:class],
                                :id => values[:id]
                               ),
             :class => "button button_" + values[:category])
end

Refactorings

No refactoring yet !

37a8c316fa7753ae9be63e6476d16a20

Jordan Running

September 9, 2009, September 09, 2009 16:06, permalink

No rating. Login to rate! 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
def button(title, values)
  #Checks the category for certain keywords so the proper icon can be inlined in to the button.
  if values[:category]
    if values[:category] =~ /secure|prev|next/
      # $& contains the most recent match
      img_tag = content_tag(:img, '', :src => "../images/buttons_#{$&}.gif")
    else
      # may want an else condition for unexpected categories
    end
  else
    img_tag = ""
    # if no button category is given, it defaults to primary.
    values[:category] = "primary"
  end
  
  # The input type defaults to "button" if not given.
  values[:type] ||= "button
  
  if values[:onclick] then
    onclick_event = values[:onclick]
  elsif values[:url] then
    onclick_event = "window.location=" + values[:url]
  end
  
  # The tag is created to accomplish the sliding door technique.
  div_tag = content_tag(:div, ' ', :class => 'endcap')
  
  content_tag(:div, content_tag(
                                :input, img_tag + div_tag,
                                :value => title,
                                :type => values[:type],
                                :onclick => onclick_event,
                                :class => values[:class],
                                :id => values[:id]
                               ),
             :class => "button button_" + values[:category])
end
A9c6e626501542623f7008b445c6ad82

Jordan Running

September 9, 2009, September 09, 2009 16:38, permalink

1 rating. Login to rate!

Of course I screwed that up and wasn't logged in.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
def button(title, values)
  #Checks the category for certain keywords so the proper icon can be inlined in to the button.
  if values[:category]
    if values[:category] =~ /secure|prev|next/
      # $& contains the most recent match
      img_tag = content_tag(:img, '', :src => "../images/buttons_#{$&}.gif")
    else
      # may want an else condition for unexpected categories
    end
  else
    img_tag = ""
    # if no button category is given, it defaults to primary.
    values[:category] = "primary"
  end
  
  # The input type defaults to "button" if not given.
  values[:type] ||= "button"
  
  if values[:onclick] then
    onclick_event = values[:onclick]
  elsif values[:url] then
    onclick_event = "window.location=" + values[:url]
  end
  
  # The tag is created to accomplish the sliding door technique.
  div_tag = content_tag(:div, ' ', :class => 'endcap')
  
  content_tag(:div, content_tag(
                                :input, img_tag + div_tag,
                                :value => title,
                                :type => values[:type],
                                :onclick => onclick_event,
                                :class => values[:class],
                                :id => values[:id]
                               ),
             :class => "button button_" + values[:category])
end
F30b04ed4d0b3fc4bc791a28815f34ca

ReinH

September 9, 2009, September 09, 2009 17:41, permalink

2 ratings. Login to rate!

This code has issues in the HTML it generates (input tags are self-closing and do not have content, window.location= is an ugly hack), but nevertheless refactored for science. Assignment of onclick_event is buggy in original implementation (branches where the variable is undefined), has been fixed in refactor. Also used more intention revealing variable names.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
def button(title, values)
  values[:type] ||= "button"
  values[:category] ||= "primary"

  onclick_event = values[:url] ? "window.location=#{values[:url]}" : values[:onclick]
  endcap = content_tag(:div, '&nbsp', :class => 'endcap')

  inline_icon = ""
  if values[:category]  =~ /secure|prev|next/
    inline_icon = content_tag(:img, '', :src => "../images/#{$&}.gif")
  end
      
  content_tag(:div, :class => "button button_" + values[:category]) do
    content_tag(:input, inline_icon + endcap,
      :value => title,
      :type => values[:type],
      :onclick => onclick_event,
      :class => values[:class],
      :id => values[:id]
    )
  end
end
8f5553306c2cf7f4b14153f6117f8e9b

Danny Peck

September 9, 2009, September 09, 2009 18:31, permalink

No rating. Login to rate!

Thanks for the help. Good guidance too.

A8d3f35baafdaea851914b17dae9e1fc

Adam

September 9, 2009, September 09, 2009 18:43, permalink

No rating. Login to rate! 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
def button(title, options = {})
  category = options.delete(:category) || 'primary'
  endcap   = content_tag(:div, ' ', :class => 'endcap')
  
  options.reverse_merge!(
    :value => title,
    :type => 'button',
    :onclick => ("window.location=#{options[:url]}" if options[:url])
  )
  
  content_tag(:div, :class => "button button_#{category}") do
    content_tag(:input, options) do
      category == 'primary' ? endcap :
        image_tag("../images/buttons_#{category[/secure|prev|next/]}.gif") +
        endcap      
    end
  end
end

Your refactoring





Format Copy from initial code

or Cancel