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.

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

Event page: Add keywords to metadata in header #3262

Merged
merged 1 commit into from Mar 29, 2018

Conversation

Projects
None yet
3 participants
@bpedersen2
Contributor

bpedersen2 commented Mar 12, 2018

Currently the assigned keywords are only use in some calendar views,
but for search engines they should also be provided in the keywords
meta header.

@@ -2,3 +2,6 @@
<meta property="og:image" content="{{ event.logo_url if event.has_logo else
(indico_config.IMAGES_BASE_URL + '/logo_indico.png') }}">
<meta property="og:description" content="{{ event.description|striptags|truncate(500) }}">
{%- if event.keywords -%}
<meta keywords="{{ event.keywords|join(',') }}">

This comment has been minimized.

@ThiefMaster

ThiefMaster Mar 12, 2018

Member

I think the correct syntax is <meta name="keywords" value="...">

This line should be indented (we care about readable templates, even if this makes the final HTML output uglier).

Also, I think right now this ends up right behind the og:description line without a linebreak, so I would replace the -%} with %} or {%- with {%} (the - strips whitespace on the side where it is).

This comment has been minimized.

@bpedersen2

bpedersen2 Mar 12, 2018

Contributor

Done

CHANGES.rst Outdated
@@ -65,6 +65,7 @@ Improvements
reviewing
- Show a nicer error message when entering an excessively high base
registration fee (:issue:`3260`)
- Add the keywords as meta tag to the page header(thanks :user:`bpedersen2`)

This comment has been minimized.

@ThiefMaster

ThiefMaster Mar 12, 2018

Member

Missing space, and IMHO this can be a bit more concise:

- Expose event keywords through an HTML meta tag (:issue:`3262`, thanks
  :user:`bpedersen2`)

This comment has been minimized.

@bpedersen2

bpedersen2 Mar 12, 2018

Contributor

Done

@ThiefMaster

This comment has been minimized.

Member

ThiefMaster commented Mar 12, 2018

There's a double space in your commit message (between the and assigned) - no big deal, but if you amend the commit with the suggestions above, this could be changed at the same time ;)

@bpedersen2 bpedersen2 force-pushed the bpedersen2:keywords branch from b7a4f56 to b7b2954 Mar 12, 2018

Event page: Add keywords to metadata in header
Currently the assigned keywords are only use in some calendar views,
but for search engines they should also be provided in the keywords
meta header.

@ThiefMaster ThiefMaster force-pushed the bpedersen2:keywords branch from b7b2954 to 8cf1ba7 Mar 29, 2018

@ThiefMaster ThiefMaster merged commit 8cf1ba7 into indico:master Mar 29, 2018

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@bpedersen2 bpedersen2 deleted the bpedersen2:keywords branch Apr 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment