2
\$\begingroup\$

I would like to refactor code below and get rid of if statements for classes from view.

.container
  .tabbable.tabs-left
    %ul.nav.nav-tabs
      - @related_tasks.each do |task|
        %li{class: ("active" if @task == task)}
          = link_to task.name, "#tab#{task.id}", data: { toggle: 'tab' }, class: @current_user_tasks.include?(task.id) ? '' : 'inactive' 

    .tab-content
      - @related_tasks.each do |task|
        .tab-pane{id: "tab#{task.id}", class: ("active" if @task == task)}
          = render 'task_tab', task: task
\$\endgroup\$
3
\$\begingroup\$

Helper methods are good for this. In your view's helper file:

def task_link_class(task)
  if @current_user_tasks.include?(task)
    ''
  else
    'inactive'
  end
end

Some would prefer the trinary operator instead:

def task_link_class(task)
  @current_user_tasks.include?(task) ? '' : 'inactive'
end

And in your view:

= link_to task.name, "#tab#{task.id}", data: { toggle: 'tab' }, class: task_link_class(task)
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Its my understanding that using @instance_variables in helpers is not a good practice. Better pass values as arguments. \$\endgroup\$ – tokland Jan 7 '14 at 19:09
  • \$\begingroup\$ @tokland, Thanks for pointing that out. Do you have a link where I can learn why? \$\endgroup\$ – Wayne Conrad Jan 7 '14 at 19:16
  • 1
    \$\begingroup\$ @tokland, How about this? rails-bestpractices.com/posts/… \$\endgroup\$ – Wayne Conrad Jan 7 '14 at 21:10
  • 1
    \$\begingroup\$ Yes, that would be the idea, in partials, in helpers, etc, use arguments. The key is reusability, this way you can use and tests those functions in isolation. \$\endgroup\$ – tokland Jan 8 '14 at 9:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.