Page titles #146

Open
glebm opened this Issue Feb 14, 2012 · 12 comments

Projects

None yet

4 participants

@glebm

The page titles are all the same, which Google (= The Web God) doesn't like.

We came up with a workaround like this:

- title = ""
- if @topic
   - title += "#{@topic.subject} - #{@topic.forum.title} - #{@topic.forum.category.name} forums - "
- elsif @forum
  - title += "#{@forum.category.name} forums - #{@forum.title} - "
- elsif @category
  - title += "#{@category.name} forums - "
#{title}Site Name

It would be nice to have some way of customizing them without resorting to using the controller's instance variables though.

One suggestion would be to use I18n.

@parndt

A helper would be better than introducing this conditional logic in the view :) I'm not sure how that allows customisation though. @knewter any thoughts?

Besides Google, I'm pretty sure ordinary internet uses like titles to differ too.

@knewter

So yeah, the traditional way we've handled this is with a helper....so our application layout tends to have:

%title= render_title

Where there's a

def render_title
  [@title, @site_name].compact.join(" | ")
end

def title=(title)
  @title = title
end

Then in our views or controllers, we'll just use title= to set the title. Does that make sense?

@glebm

@knewter However, forem controllers do not set @title

The idea I'm proposing is to have titles come from I18n interpolations. E.g., if forem does something like this (and exposes it somehow, e.g. forem.page_title):

I18n.t('forem.topic.title', topic: @topic.subject, forum: @topic.forum.title)

Then you gain sensible default titles and the ability to customize them.

Now, this is just the first idea, I am sure we can come with a better one.

PS

View vs helper is not the point here (that code was just an example)
Everyone has different techniques for this kind of thing.
For example, We use

%title= content_for?(:page_title) ? yield(:page_title) : site_name

And then in the view

- content_for :page_title, "My Account - #{site_name}"
@knewter

Right, so I'm at SpreeConf and completely not in-context :) I think using refinery for i18n page titles where they come from refinery, and locale files for pages that don't fall under that purview, makes plenty of sense...

@parndt

@knewter this is Forem not Refinery ;-)

@knewter

doesn't matter... :-\ I should've stopped when I said I wasn't in-context obviously :)

The proposal @glebm made is sensible. Why don't we do it that way?

Also, graceful bow-out before I make an ass of myself again :)

@radar

I think we should call a method at the top of each relevant view that sets a variable that the layout can reference as the title. Something like this:

forem_title("#{@forum.title} - Forums")

Rather than having an if statement with many possible directions.

@parndt

Yep..

@glebm

Implemented here: #195
glebm@e0d3262

@parndt

Totally not implemented in the linked pull request unfortunately due to it overriding most of Forem... still looking to fix this issue.

@glebm I tried to look at the specific commit you were mentioning but it's gone :'( did you mean glebm@e0d3262 ?

@glebm

Update: Yes, it's that commit. Should be easy to integrate
I've been using it on production for about 3 months now

@glebm

To whoever is willing to follow up on this:

Page numbers should be included in titles (e.g. as %{page_number} in I18n string)
This is to avoid duplicate page titles on the site

@radar radar added this to the v1.0 milestone Jul 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment