Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

HTML title of a news entry should be the news title (not only "News") #44

Open
perlpunk opened this issue Feb 13, 2013 · 5 comments
Open

HTML title of a news entry should be the news title (not only "News") #44

perlpunk opened this issue Feb 13, 2013 · 5 comments

Comments

@perlpunk
Copy link

The current HTML title of a news entry is only "News".
@maddingue
Copy link
Collaborator

Surprisingly, this is not easy with the way the current templates are made: a news item is actually a news list with one item. Same template, different code paths. But the title is set from within the template. Not saying it's impossible, but it would need to be modified to detect how many news items there are, and in the case there's only one, set the page title with the news title. My TT2-fu is currently too weak for this.

@book
Copy link
Owner

book commented Dec 1, 2014

What exactly is the problem?

In the following, the news entries seem to have their title shown properly:

@book
Copy link
Owner

book commented Dec 1, 2014

Oh, I got it: by "HTML title" you mean the content of <html><head><title>.

@book
Copy link
Owner

book commented Dec 1, 2014

OK, so the main ui template seems to have support for a title variable. This is the one you want to set.

Looking at Act::News, that seems to collect all the news items (from the news_item table) into a hash keyed by language. I think the main title and text methods are actually incorrect. They look like the following (in lib/Act/News.pm):

sub title { $_[0]{title} }
sub text  { $_[0]{text}  }

but in fact, these would be empty/undef, because the actual title and text are under the items hash.

Therefore I would suggest replacing those by:

sub title { $_[0]{items}{$Request{language}}{title} }
sub title { $_[0]{items}{$Request{language}}{text} }

to provide auto-localization. This has the drawback of adding a dependency on $Request{language}, which is probably undesirable. And since those methods are not used anywhere (because there are broken), we might as well just remove them.

The real issue here is to set the title template variable. Individual news items are shown using the news/item template and handled by the Act::Handler::News::Edit handler (which also shows them, it seems).

Template variables are set using this line:

$template->variables( %$fields );

So we need to fill in the news item title, properly localized.

At lib/Act/Handler/News/Edit.pm line 135, we see this:

    if (exists $Request{args}{news_id}) {                                 
        $fields = { %$news };                                             
        $fields->{items} = $news->items;                                  
    }

We can simply add the title with this line:

        $fields->{title} = $news->{items}{ $Request{language} }{title};

As an aside, setting $fields->{items} directly is probably not needed, since there is already an items key in $news.

@book
Copy link
Owner

book commented Dec 1, 2014

Title set in 1fadb2c.

Some cleanup done in 4c629dd.

These haven't been tested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@book @maddingue @perlpunk