Issue 29375829: Issue 4899 - set template META description on eyeo.com (Closed)
DescriptionIssue 4899 - set template META description on eyeo.com
Patch Set 1 #
Total comments: 4
Patch Set 2 : Deleted the include and added to the default template #
Total comments: 6
Patch Set 3 : Deleted META-title and changed META-description #Patch Set 4 : Deleted Space and added slash #MessagesTotal messages: 8
Please include which website is being changed in the subject. Also: - eyeo.com is not translated - therefore there is no need for the translate filter in includes/meta.tmpl - It's not necessary to put the meta data into an include unless there are several templates which include it https://codereview.adblockplus.org/29375829/diff/29375830/includes/meta.tmpl File includes/meta.tmpl (right): https://codereview.adblockplus.org/29375829/diff/29375830/includes/meta.tmpl#... includes/meta.tmpl:7: <meta name="title" content="{{ title | translate('page-title', 'Page Title') }}"> Title is already included in templates/default.tmpl
Small addition to Julian's comments. https://codereview.adblockplus.org/29375829/diff/29375830/templates/default.tmpl File templates/default.tmpl (right): https://codereview.adblockplus.org/29375829/diff/29375830/templates/default.t... templates/default.tmpl:4: <? include meta ?> Detail: please use 2 space indentation, as specified in https://adblockplus.org/en/coding-style
On 2017/02/22 13:10:47, saroyanm wrote: > Small addition to Julian's comments. > > https://codereview.adblockplus.org/29375829/diff/29375830/templates/default.tmpl > File templates/default.tmpl (right): > > https://codereview.adblockplus.org/29375829/diff/29375830/templates/default.t... > templates/default.tmpl:4: <? include meta ?> > Detail: please use 2 space indentation, as specified in > https://adblockplus.org/en/coding-style Sorry, I forgot to change my indentation settings.
https://codereview.adblockplus.org/29375829/diff/29375830/includes/meta.tmpl File includes/meta.tmpl (right): https://codereview.adblockplus.org/29375829/diff/29375830/includes/meta.tmpl#... includes/meta.tmpl:7: <meta name="title" content="{{ title | translate('page-title', 'Page Title') }}"> On 2017/02/21 15:20:14, juliandoucette wrote: > Title is already included in templates/default.tmpl Done. https://codereview.adblockplus.org/29375829/diff/29375830/templates/default.tmpl File templates/default.tmpl (right): https://codereview.adblockplus.org/29375829/diff/29375830/templates/default.t... templates/default.tmpl:4: <? include meta ?> On 2017/02/22 13:10:47, saroyanm wrote: > Detail: please use 2 space indentation, as specified in > https://adblockplus.org/en/coding-style Done.
https://codereview.adblockplus.org/29375829/diff/29377558/templates/default.tmpl File templates/default.tmpl (right): https://codereview.adblockplus.org/29375829/diff/29377558/templates/default.t... templates/default.tmpl:5: <meta charset="utf-8"> It's uppercase UTF-8 -> https://developer.mozilla.org/en-US/docs/Web/HTML/Element/meta#attr-charset https://codereview.adblockplus.org/29375829/diff/29377558/templates/default.t... templates/default.tmpl:10: <meta name="title" content="{{ title }}" > The title is being specified below, I don't think we need alternative title. See line: 17 https://codereview.adblockplus.org/29375829/diff/29377558/templates/default.t... templates/default.tmpl:11: <meta name="description" content="{{ description if description else 'eyeo features a host of products, all of which are geared toward giving users more control and more information about their online experience.' }}"> Why are you using the description below: "eyeo features a host of products, all of which are geared toward giving users more control and more information about their online experience." ? Please note that this description will be used over every single page we currently have in eyeo.com.
https://codereview.adblockplus.org/29375829/diff/29377558/templates/default.tmpl File templates/default.tmpl (right): https://codereview.adblockplus.org/29375829/diff/29377558/templates/default.t... templates/default.tmpl:5: <meta charset="utf-8"> On 2017/02/28 17:24:58, saroyanm wrote: > It's uppercase UTF-8 -> > https://developer.mozilla.org/en-US/docs/Web/HTML/Element/meta#attr-charset Done. https://codereview.adblockplus.org/29375829/diff/29377558/templates/default.t... templates/default.tmpl:10: <meta name="title" content="{{ title }}" > On 2017/02/28 17:24:58, saroyanm wrote: > The title is being specified below, I don't think we need alternative title. > See line: 17 You are right. I thought there is a difference when a crawler or smth. like that searches for a title without META-tag. I deleted the line. https://codereview.adblockplus.org/29375829/diff/29377558/templates/default.t... templates/default.tmpl:11: <meta name="description" content="{{ description if description else 'eyeo features a host of products, all of which are geared toward giving users more control and more information about their online experience.' }}"> On 2017/02/28 17:24:58, saroyanm wrote: > Why are you using the description below: > "eyeo features a host of products, all of which are geared toward giving users > more control and more information about their online experience." ? > Please note that this description will be used over every single page we > currently have in http://eyeo.com. It should be used as a placeholder, when there is no other description. But I agree that would not be the best for every single page. Should we use a default description for the META? Otherwise I would rather use a statement then a expression.
LGTM 
|
