Skip to content
This repository was archived by the owner on Oct 7, 2022. It is now read-only.

Conversation

@kanagarajkm
Copy link
Contributor

No description provided.

@kanagarajkm kanagarajkm changed the title Add lang attribute to html element Add lang attribute hreflang meta tags Apr 4, 2019
@kanagarajkm kanagarajkm changed the title Add lang attribute hreflang meta tags Add lang attribute and hreflang meta tags Apr 4, 2019
@kanagarajkm
Copy link
Contributor Author

@deekoder can you please review this.

@deekoder deekoder requested review from deekoder and kaankabalak April 4, 2019 18:24
@deekoder
Copy link
Contributor

deekoder commented Apr 4, 2019

@kanagarajkm - Let me think more about changing the structure of the yml file so much. I feel we could do this much more tightly without so many changes to the yml file. Will get back to you with exact actionable suggestions for you to evaluate further.

site-example.yml Outdated
Language: # Optional - Translation links
- Chinese: https://<domain.com>/cn
- Japanese: https://<domain.com>/jp
OtherLanguages: # Optional - Translation links
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not for changing the structure. How about we infer the Language. We already know if its english or chinese or japanese language.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@deekoder do you mean we can infer the language from the name of the file? Like site.yaml is english, site_CN.yml is chinese and so on.

site-example.yml Outdated
Options: hidden # Optional - The value 'hidden' allows user to render page but hide sidebar link

OtherLanguages: # Optional - Available Translations
- cn
Copy link
Contributor

Choose a reason for hiding this comment

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

@kanagarajkm do we really need this in the site.yml file? Can we infer this based on the original Language settings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@deekoder yes, we could have avoided it. But some of the docs which are in english does not have equivalent chinese docs. We might end up with lot of 404 links.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Ok @kaankabalak explained that to me. So that's why it's at a document level.

@kanagarajkm
Copy link
Contributor Author

@deekoder @kaankabalak we have another problem. As per Google, the translation links should be fully qualified https://support.google.com/webmasters/answer/189077

Alternate URLs must be fully-qualified, including the transport method (http/https), so:
https://example.com/foo, not //example.com/foo or /foo

@kanagarajkm
Copy link
Contributor Author

@deekoder @kaankabalak please review again.

site-example.yml Outdated
Output: "docs" # Required - Output directory where html files are generated
Logo: "https://<domain.com>/url-to-logo.svg" # Site Branding
SEO Author: "Author Name" # Optional - Meta author tag for SEO
Language: "English" # Required - Language of the docs
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it might be a good idea to indicate (either on the key name or the comment for this field) that this is the current language of the docs for the pages listed in this yml 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.

@kaankabalak will update the comment to convey it correctly, thanks

@kaankabalak
Copy link
Collaborator

There just a slight problem on the dropdown where we list the translations:

Screen Shot 2019-04-08 at 11 28 34 AM

It seems like this was being caused because we were iterating one level deeper than we were supposed to and we can apply the following change to fix this issue:

diff --git a/lib/_header.erb b/lib/_header.erb
index c1dc2f2..663f133 100644
--- a/lib/_header.erb
+++ b/lib/_header.erb
@@ -43,11 +43,9 @@
               </a>
 
               <div class="dropdown__menu dropdown__menu--right">
-                <%- @site_map['Languages'].each do |categories| -%>
-                  <%- categories.each do |key, value| -%>
-                    <%- unless key == @site_map['Language']%>
-                      <a class="dropdown__item" href="<%=value -%>"><%= key -%></a>
-                    <%- end -%>
+                <%- @site_map['Languages'].each do |key, value| -%>
+                  <%- unless key == @site_map['Language']%>
+                    <a class="dropdown__item" href="<%=value -%>"><%= key -%></a>
                   <%- end -%>
                 <%- end -%>
               </div>

@kaankabalak
Copy link
Collaborator

kaankabalak commented Apr 8, 2019

It also seems like the tags weren't appearing on the index page as we were not including them for the case where we are rendering the index page. We can fix this issue by applying the following diff:

diff --git a/lib/index.erb b/lib/index.erb
index a267944..84f5f17 100644
--- a/lib/index.erb
+++ b/lib/index.erb
@@ -17,14 +17,23 @@
       <%- end -%>
       <meta name="description" content="<%= key2["SEO Description"] %>">
       <meta name="keywords" content="<%= key2["SEO Keywords"] %>">
-
       <%- if !@site_map['Languages'].nil? && !key2["SEO Languages"].nil? -%>
-        <link rel="alternate" hreflang="<%= ISO_639.find_by_english_name(@site_map['Language']).alpha2 %>" href="<%=@site_map['Languages'][@site_map['Language']]%>/<%=key2["Slug"]%>.html" title="lang">
-        
+        <link
+          rel="alternate"
+          hreflang="<%= ISO_639.find_by_english_name(@site_map['Language']).alpha2 %>"
+          href="<%=@site_map['Languages'][@site_map['Language']]%>/<%=key2["Slug"]%>.html"
+          title="lang"
+        />
         <% key2["SEO Languages"].each do |lang| %>
-          <link rel="alternate" hreflang="<%= ISO_639.find_by_english_name(lang).alpha2 %>" href="<%=@site_map['Languages'][lang]%>/<%=key2["Slug"]%>.html" title="lang">
-        <% end %>
+          <link
+            rel="alternate"
+            hreflang="<%= ISO_639.find_by_english_name(lang).alpha2 %>"
+            href="<%=@site_map['Languages'][lang]%>/<%=key2["Slug"]%>.html"
+            title="lang"
+          />
+        <%- end -%>
       <%- end -%>
+    <%-# this is the case where we render index.html -%>
     <%- else -%>
       <%-# Take first entry specified under the 'Documents' key for index page-%>
       <%- if !@site_map['Documents'][0].values[0][0]["SEO Title"] -%>
@@ -35,6 +44,22 @@
       <%- end -%>
       <meta name="description" content="<%= @site_map['Documents'][0].values[0][0]["SEO Description"] %>">
       <meta name="keywords" content="<%= @site_map['Documents'][0].values[0][0]["SEO Keywords"] %>">
+      <%- if !@site_map['Languages'].nil? && !@site_map['Documents'][0].values[0][0]["SEO Languages"].nil? -%>
+        <link
+          rel="alternate"
+          hreflang="<%= ISO_639.find_by_english_name(@site_map['Language']).alpha2 %>"
+          href="<%=@site_map['Languages'][@site_map['Language']]%>/<%=@site_map['Documents'][0].values[0][0]["Slug"]%>.html"
+          title="lang"
+        />
+        <% @site_map['Documents'][0].values[0][0]["SEO Languages"].each do |lang| %>
+          <link
+            rel="alternate"
+            hreflang="<%= ISO_639.find_by_english_name(lang).alpha2 %>"
+            href="<%=@site_map['Languages'][lang]%>/<%=@site_map['Documents'][0].values[0][0]["Slug"]%>.html"
+            title="lang"
+          />
+        <% end %>
+      <%- end -%>
     <%- end -%>
 
     <link href="https://cdn.jsdelivr.net/npm/docsearch.js@2/dist/cdn/docsearch.min.css" rel="stylesheet">

Copy link
Collaborator

@kaankabalak kaankabalak left a comment

Choose a reason for hiding this comment

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

Please see the comments for the required changes

@kanagarajkm
Copy link
Contributor Author

kanagarajkm commented Apr 9, 2019

@kaankabalak thanks for the feedback. For the index page, i have added the languages url as an alternate link instead of the first page link. Please have a look.

https://docs.min.io/ and https://docs.min.io/docs/minio-quickstart-guide.html have the same content, we need to add a canonical tag for this, i will open a separate issue.

@kanagarajkm
Copy link
Contributor Author

@deekoder can you please review this?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants