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 i18n support #262
Added i18n support #262
Conversation
_layouts/default.html
Outdated
@@ -1,5 +1,11 @@ | |||
<!DOCTYPE html> | |||
<html class="no-js"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Il faudrait changer l'attribut lang
sur la balise <html>
:
<!DOCTYPE html>
<html class="no-js" lang="{{ site.lang }}">
...
</html>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bonne remarque, merci @ulrik59, j'ai ajouté l'attribut :)
9a1bfbc
to
cd2e202
Compare
_includes/disqus.html
Outdated
@@ -1,5 +1,8 @@ | |||
<div id="disqus_thread"></div> | |||
<script type="text/javascript"> | |||
var disqus_config = function () { | |||
this.language = window.site.lang; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah j'avais pas vu ce fichier... Alors le souci ici, c'est que la variable window.site
n'existe pas... Tu auras une erreur javascript...
Il faut soit bouger le js pour lancer disqus dans le fichier scripts.html
soit il faut remplacer window.site.lang
par '{{ site.lang }}'
(je suis pour la seconde option)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ulrik59 Ça fonctionne pourtant parfaitement ainsi (dû au fait que le window.site.lang
est appelé depuis une function
).
Snippet pour reproduire :
function baz() {
console.log('value: ' + window.foo);
}
window.foo = 'bar';
baz();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ça fonctionne mais c'est une mauvaise pratique et c'est dangereux. Rien ne dit que Disqus change son code et que la fonction s’exécute avant le parsing du code JS déclarant la variable. Si tu ne veux pas changer le fichier disqus.html
, je serais plus confiant si la déclaration du window.site
se faisait à la fin du <head/>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
La fonction de config est déclarée de notre côté donc Disqus peut changer ce qu'il voudra ça n'impactera pas notre déclaration ...
J'ai tout de même modifié pour mettre directement {{ site.lang }}
, comme ça le débat du JS est clôt et on évite de revenir en arrière en remettant la déclaration du window.site
dans le head.
8efc3bc
to
d8da3bb
Compare
_algolia.yml
Outdated
algolia: | ||
application_id: '5IGTHBX5JS' | ||
index_name: 'blog_eleven' | ||
- index.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
le yaml n'est pas valide, je pense qu'il manque le exclude:
@VEBERArnaud Bien vu ! Merci, c'est corrigé |
@@ -11,7 +11,7 @@ <h3 class="read-also-title">A lire aussi</h3> | |||
{% continue %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tu as oublié de traduire la balise <h3>A lire aussi</h3>
au dessus.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merci, j'ai ajouté la traduction
_includes/newsletter_link.html
Outdated
@@ -1,7 +1,7 @@ | |||
<footer class="slice newsletter"> | |||
<div class="container"> | |||
<a class="newsletter-link" target="_blank" href="http://eepurl.com/cOuOIf"> | |||
abonnez vous à notre newsletter ! | |||
{% translate global.newsletter_subscribe %} | |||
</a> | |||
</div> | |||
</footer> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
manque un retour à la ligne
@eko le conflit et je valide :) |
@CaptainJojo done :) |
Hello,
This is the pull request for the following issue: #26
Overview
I've added i18n support thanks to the following plugin: https://github.com/Anthony-Gaudino/jekyll-multiple-languages-plugin
Here is the features added in this PR:
/
) and brings an english version of this page:/en
,Additional information
This PR cannot be merged actually because one thing I find weird with it is that he needs to put all posts under
i18n/{fr,en}/_posts
subdirectory which is not cool so I've opened a PR to propose a parameter that will allow us to customized this directory and use:_posts/{fr,en}
.PR is opened here: kurtsson/jekyll-multiple-languages-plugin#107
Thank you