Patch #11704: Avoid the use of tag("...", "...", true) in layout - Redmine

Actions

Copy link

Patch #11704

closed

Avoid the use of tag("...", "...", true) in layout

Added by Jean-Baptiste Barth over 11 years ago. Updated over 11 years ago.

Status:

Closed

Priority:

Normal

Assignee:

Category:

Code cleanup/refactoring

Target version:

2.1.0

Start date:

Due date:

% Done:

Estimated time:

Description

Here's a very simplified version of Redmine base layout :

<html>
  <body>
    <%= tag('div', {:id => 'main', :class => (sidebar_content? ? '' : 'nosidebar')}, true) %>
      <div id="sidebar">
        <%= yield :sidebar %>
      </div>
      <div id="content">
        <%= yield %>
      </div>
    </div>
  </body>
</html>

There's a "tag" helper, processed with ERB, to display "div#main", and it's closed with standard html in the end.

It could be considered as bad practice, but the main problem is that you cannot parse the layout correctly before it's fully processed by ERB. Hence gems that hook in the rendering process don't work correctly for the layout partial. It's the case of deface (https://github.com/railsdog/deface) for instance, a gem I use these days so that I don't need 50 more view hooks in core.

1st solution : use inline erb just for the class

i.e. replace the tag line with :

  <div id="main" class="<%= sidebar_content? ? '' : 'nosidebar' %>">

2nd solution : use content_tag helper

i.e. something like that :

<html>
  <body>
    <%= content_tag 'div', :id => 'main', :class => (sidebar_content? ? '' : 'nosidebar') do %>
      <div id="sidebar">
        <%= yield :sidebar %>
      </div>
      <div id="content">
        <%= yield %>
      </div>
    <% end %>
  </body>
</html>

I understand this isn't a problem for redmine core for now, but such a change would ease plugin development a lot and avoid overriding the layout/base partial.

Any thought on this is welcome.