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.

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

Added hreflang alternate tags #74

Merged
merged 3 commits into from
Jul 8, 2019
Merged

Added hreflang alternate tags #74

merged 3 commits into from
Jul 8, 2019

Conversation

AymenLoukil
Copy link
Contributor

@AymenLoukil AymenLoukil commented Jul 3, 2019

I checked out your branch "lang". Please tell me if i should wait your PR merge :)

I loop over the available languages and add the link alternate tags to the head (on base.html). Sounds working

Fixes #61

@AymenLoukil AymenLoukil requested a review from rviscomi July 3, 2019 22:04
@AymenLoukil AymenLoukil added development Building the Almanac tech stack SEO SEO related translation world wide web labels Jul 4, 2019
@rviscomi rviscomi changed the base branch from master to langs July 4, 2019 21:27
src/templates/base.html Outdated Show resolved Hide resolved
@@ -25,6 +25,10 @@
<link rel="stylesheet" href="https://fonts.googleapis.com/css?family=Open+Sans:300,400,600,700,800">
<link rel="stylesheet" href="/static/css/styles.css">
{% endblock %}
{% set view_args = request.view_args.copy() %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation here is inconsistent. Could you match the rest of the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK i'll correct.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like they're one nested level too many. Everything should be aligned with the blocks/links before it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already corrected in the new commit no ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See screenshot below. New changes are too far indented:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I think it ok now :)

@rviscomi rviscomi merged commit 340e1df into langs Jul 8, 2019
@rviscomi rviscomi deleted the hreflang branch July 8, 2019 21:00
@rviscomi rviscomi added this to TODO in Web Almanac 2019 via automation Jul 8, 2019
@rviscomi rviscomi added this to the Infrastructure prepared milestone Jul 8, 2019
rviscomi added a commit that referenced this pull request Jul 12, 2019
* Language class

* fix tests

* Added hreflang alternate tags (#74)

* add link alternate hreflang tags

* correct indentation

* second try to correct indentation ..

* update view args in template method

* fix tests
@rviscomi rviscomi moved this from TODO to Done in Web Almanac 2019 Aug 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Building the Almanac tech stack SEO SEO related translation world wide web
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants