sunlightlabs / opencongress

An open website for opening Congress.
http://www.opencongress.org
GNU General Public License v3.0
48 stars 16 forks source link

Helpers for meta tags #484

Closed plantfansam closed 9 years ago

plantfansam commented 9 years ago

Want to get eyes on this because it's not super DRY. Despite that, I think this is a dead simple way to do page-specific meta tags.

Basically, the pattern is to call helper methods like meta_tags_for_search from the main application layout.

These helper methods will either yield to a :meta_tags_for_search block defined in a template or render a partial with default meta tags.

If you want to override defaults, you specify that within a template, such as /views/bill/show:

<% content_for :meta_tags_for_search do %>
  <meta name="description" content="<%="#{@bill.typenumber} - #{@bill.title_common}, a bill on OpenCongress"%>" >
<% end %>

Check out e1f477bedfd786ffafc0ff3056ccd4b2e4e49fa9 for what this will look like when implemented.

plantfansam commented 9 years ago

So we just talked about this afk and agreed that this pattern is indeed not DRY enough. I'm going take another shot at a more abstracted solution.

plantfansam commented 9 years ago

So 68905c9a6400b14c25ec446e81bc53a342c08c66 is a more abstract approach.

The idea is that you can use the generic set_meta_tags(options) to set attributes common to all meta tags. You can also use set_meta_tags_for_#{service} to override specific attributes. Per-service defaults are set in ApplicationController::META_TAG_DEFAULTS.

meta_tags_for_#{service}.html.erb partials iterate over the key/value pairs in the instance variable hash and dump them into service-specific tag templates.

crdunwel commented 9 years ago

This ready to be merged?

plantfansam commented 9 years ago

cleaning it up just a little

plantfansam commented 9 years ago

@crdunwel these commits address your point about not setting the title in the controller, which I think was a valid one. Now you can set all the :title attributes in one go, which is cool.

I agree that this could be made a bit more abstract but I think this will do.