Merge pull request #2160 from barrymcgee/improve-accessibility

chiedo · web-flow · commit 98e74a511a04 · 2021-01-08T09:06:13.000-05:00
Fix accessibility issues on homepage and individual article pages
diff --git a/includes/article.html b/includes/article.html
@@ -1,4 +1,4 @@
-<main class="container-xl px-3 px-md-6 my-4 my-lg-4 d-xl-flex">
+<div class="container-xl px-3 px-md-6 my-4 my-lg-4 d-xl-flex">
   <article class="markdown-body width-full">
     <div class="d-lg-flex flex-justify-between">
       <div class="d-block d-lg-none">{% include article-version-switcher %}</div>
@@ -48,7 +48,7 @@ <h1 class="border-bottom-0">{{ page.title }}</h1>
     <div class="article-grid-toc border-bottom border-xl-0 pb-4 mb-5 pb-xl-0 mb-xl-0">
       <div class="article-grid-toc-content">
         {% if miniTocItems.size > 1 %}
-        <h3 id="in-this-article" class="f5 mb-2"><a class="link-gray-dark" href="#in-this-article">{% data ui.pages.miniToc %}</a></h3>
+        <h2 id="in-this-article" class="f5 mb-2"><a class="link-gray-dark" href="#in-this-article">{% data ui.pages.miniToc %}</a></h2>
         <ul class="list-style-none pl-0 f5 mb-0">
           {% for item in miniTocItems %}
           <li class="ml-{{ item.indentationLevel | times: 3 }} {{ item.platform }} mb-2 lh-condensed">{{ item.contents }}</li>
@@ -70,4 +70,4 @@ <h3 id="in-this-article" class="f5 mb-2"><a class="link-gray-dark" href="#in-thi
       {% unless page.hidden %}{% include contribution %}{% endunless %}
     </div>
   </article>
-</main>
+</div>
diff --git a/includes/contribution.html b/includes/contribution.html
@@ -1,11 +1,11 @@
 {% unless enterpriseServerReleases.isOldestReleaseDeprecated and currentVersion contains enterpriseServerReleases.oldestSupported %}
 <div class="mt-3 f5 contribution">
-  <h4>{% data ui.contribution_cta.title %}</h4>
+  <h2 class="f5">{% data ui.contribution_cta.title %}</h2>
   <p class="text-gray f6">{% data ui.contribution_cta.body %}</p>
   <a class="btn btn-outline" href={{ "https://github.com/github/docs/edit/main/content/" | append: page.relativePath }}>
     {% octicon "git-pull-request" height="16" %}
     {% data ui.contribution_cta.button %}
   </a>
   <p class="text-gray f6 mt-2">{% data ui.contribution_cta.or %} <a href="https://github.com/github/docs/blob/main/CONTRIBUTING.md" target="_blank">{% data ui.contribution_cta.to_guidelines %}</a></p>
 </div>
-{% endunless %}
+{% endunless %}
diff --git a/includes/header.html b/includes/header.html
@@ -5,8 +5,8 @@
 
   <header class="container-xl px-3 px-md-6 pt-3 pb-2 position-relative d-flex flex-justify-between width-full {% if error == '404' %} d-md-none {% endif %}">
 
-    <div class="d-flex flex-items-center d-lg-none" style="z-index: 3;" id="github-logo-mobile">
-      <a href="/{{ currentLanguage }}" aria-hidden="true">
+    <div class="d-flex flex-items-center d-lg-none" style="z-index: 3;" id="github-logo-mobile" role="banner">
+      <a href="/{{ currentLanguage }}" aria-hidden="true" tabindex="-1">
         {% octicon "mark-github" height="32" class="text-black" %}
       </a>
       <a href="/{{ currentLanguage }}" class="h4-mktg text-gray-dark no-underline no-wrap pl-2">{% data ui.header.github_docs %}</a>
diff --git a/includes/helpfulness.html b/includes/helpfulness.html
@@ -1,13 +1,13 @@
 {% unless enterpriseServerReleases.isOldestReleaseDeprecated and currentVersion contains enterpriseServerReleases.oldestSupported %}
 <form class="js-helpfulness mt-4 f5">
-  <h4
+  <h2
     data-help-start
     data-help-yes
     data-help-no
-    class="mb-1"
+    class="mb-1 f5"
   >
     {% data ui.helpfulness.able_to_find %}
-  </h4>
+  </h2>
   <p class="f6">
     <a href="/github/site-policy/github-privacy-statement">Privacy policy</a>
   </p>
diff --git a/includes/landing.html b/includes/landing.html
@@ -17,7 +17,7 @@ <h1 class="h1-mktg mb-3">{% data ui.search.need_help %}</h1>
   <!-- Explore by product -->
   <section class="container-xl pb-lg-4 my-8 px-3 px-md-6">
     <div class="">
-      <h3 class="text-mono f5 text-normal text-gray text-md-center mb-4">{% data ui.homepage.explore_by_product %}</h3>
+      <h2 class="text-mono f5 text-normal text-gray text-md-center mb-4">{% data ui.homepage.explore_by_product %}</h2>
       <div class="d-flex flex-wrap gutter gutter-xl-spacious">
       {% for product in activeProducts %}
       {% if product.versions contains currentVersion or currentVersion == 'homepage' %}
diff --git a/includes/liquid-tags/link-with-intro.html b/includes/liquid-tags/link-with-intro.html
@@ -1,4 +1,4 @@
 <a class="link-with-intro Bump-link--hover no-underline" href="{{ fullPath }}">
-  <h4 class="link-with-intro-title">{{ title }}<span class="Bump-link-symbol">→</span></h4>
+  <h2 class="link-with-intro-title f4">{{ title }}<span class="Bump-link-symbol">→</span></h2>
 </a>
 {% if intro %}<p class="link-with-intro-intro">{{ intro }}</p>{% endif %}
diff --git a/includes/search-form.html b/includes/search-form.html
@@ -5,8 +5,8 @@
   - On all other pages, in the header
  -->
 
-<form class="mb-0">
-  <div id="search-input-container" aria-hidden="true">
+<form class="mb-0" aria-hidden="true">
+  <div id="search-input-container">
     <!-- Algolia instantsearch.js will add a search input here -->
   </div>
 </form>
diff --git a/includes/sidebar-specific-product.html b/includes/sidebar-specific-product.html
@@ -13,6 +13,7 @@
   <a href="{{product.href}}" class="pl-4 pr-5 pb-1 f4">{{ product.title }}</a>
   {% endunless %}
 </li>
+<li>
 <ul class="sidebar-categories list-style-none">
   {% for category in product.categories %}
   {% capture fullPathToCategory %}{{category[1].href}}{% endcapture %}
@@ -74,3 +75,4 @@
   </li>
   {% endfor %}
 </ul>
+</li>
diff --git a/includes/sidebar.html b/includes/sidebar.html
@@ -1,13 +1,14 @@
 <!-- product > category > maptopic > article -->
 <div class="sidebar d-none d-lg-block">
 
-  <div class="d-flex flex-items-center p-4 position-sticky top-0 sidebar-background-color" style="z-index: 3;" id="github-logo">
-    <a href="/{{ currentLanguage }}" class="text-white" aria-hidden="true">
+  <div class="d-flex flex-items-center p-4 position-sticky top-0 sidebar-background-color" style="z-index: 3;" id="github-logo" role="banner">
+    <a href="/{{ currentLanguage }}" class="text-white" aria-hidden="true" tabindex="-1">
       {% octicon "mark-github" height="32" %}
     </a>
     <a href="/{{ currentLanguage }}" class="h4-mktg text-white no-underline no-wrap pl-2 flex-auto">{% data ui.header.github_docs %}</a>
   </div>
 
+  <nav>
     {% if error == '404' or page.relativePath == 'index.md' %}
     <ul class="sidebar-products mt-4">
       {% include sidebar-homepage %}
@@ -17,4 +18,5 @@
       {% include sidebar-specific-product %}
     </ul>
     {% endif %}
+  </nav>
 </div>
diff --git a/includes/support.html b/includes/support.html
@@ -1,9 +1,9 @@
 <!-- Contact support banner -->
 <section class="mt-lg-9 py-7 no-print js-support-banner" style="background-color: #fafbfc;">
   <div class="container-xl px-3 px-md-6 ">
-    <h4 class="mb-2">
+    <h3 class="mb-2 f4">
       {% data ui.support.still_need_help %}
-    </h4>
+    </h3>
     {% if currentVersion contains 'enterprise' %}{% assign isEnterprise = true %}{% else %}{% assign isEnterprise = false %}{% endif %}
     <a id="ask-community" href="https://github.community" class="btn btn-outline mr-4 mt-2">
       {% octicon "people" width="16" %}
diff --git a/layouts/enterprise-server-releases.html b/layouts/enterprise-server-releases.html
@@ -9,9 +9,9 @@
       <main class="container-xl px-3 px-md-6 my-4 my-lg-4 d-xl-flex">
         <article class="markdown-body width-full">
           <div class="d-lg-flex flex-justify-between"></div>
-      
+
           <div class="mt-2 article-grid-container">
-      
+
           <div class="article-grid-head">
             <div class="d-flex flex-items-baseline flex-justify-between mt-3">
               <h1 class="border-bottom-0">{% data ui.enterprise_releases_list.title %}</h1>
@@ -20,7 +20,7 @@ <h1 class="border-bottom-0">{% data ui.enterprise_releases_list.title %}</h1>
           <div class="article-grid-toc border-bottom border-xl-0 pb-4 mb-5 pb-xl-0 mb-xl-0">
             <div class="article-grid-toc-content">
               {% if miniTocItems.size > 1 %}
-              <h3 id="in-this-article" class="f5 mb-2"><a class="link-gray-dark" href="#in-this-article">{% data ui.pages.miniToc %}</a></h3>
+              <h2 id="in-this-article" class="f5 mb-2"><a class="link-gray-dark" href="#in-this-article">{% data ui.pages.miniToc %}</a></h2>
               <ul class="list-style-none pl-0 f5 mb-0">
                 {% for item in miniTocItems %}
                 <li class="ml-{{ item.indentationLevel | times: 3 }} mb-2 lh-condensed">{{ item.contents }}</li>
@@ -58,7 +58,7 @@ <h2 id="deprecated-on-developer.github.com"><a href="#deprecated-on-developer.gi
           </div>
         </article>
       </main>
-      
+
     </main>
   </body>
 </html>
diff --git a/layouts/product-landing.html b/layouts/product-landing.html
@@ -80,7 +80,7 @@ <h3 class="f4 text-normal text-mono text-uppercase color-gray-5">{% data ui.toc.
               {% for link in featuredLinks.popular %}
                 <li class="border-top">
                   <a class="link-with-intro Bump-link--hover no-underline d-block py-3" href="{{ link.href }}">
-                    <h4 class="link-with-intro-title">{{ link.title }}<span class="Bump-link-symbol">→</span></h4>
+                    <h2 class="link-with-intro-title f5">{{ link.title }}<span class="Bump-link-symbol">→</span></h2>
                   </a>
                 </li>
               {% endfor %}
diff --git a/tests/rendering/server.js b/tests/rendering/server.js
@@ -221,30 +221,30 @@ describe('server', () => {
     // TODO disable the mini TOC tests when we replace it with sticky TOC header
     test('renders mini TOC in articles with more than one heading', async () => {
       const $ = await getDOM('/en/github/getting-started-with-github/githubs-products')
-      expect($('h3#in-this-article').length).toBe(1)
-      expect($('h3#in-this-article + ul li a').length).toBeGreaterThan(1)
+      expect($('h2#in-this-article').length).toBe(1)
+      expect($('h2#in-this-article + ul li a').length).toBeGreaterThan(1)
     })
 
     test('renders mini TOC in articles that includes h4s when specified by frontmatter', async () => {
       const $ = await getDOM('/en/github/setting-up-and-managing-your-enterprise/enforcing-security-settings-in-your-enterprise-account')
-      expect($('h3#in-this-article').length).toBe(1)
-      expect($('h3#in-this-article + ul li.ml-0').length).toBeGreaterThan(0) // non-indented items
-      expect($('h3#in-this-article + ul li.ml-3').length).toBeGreaterThan(0) // indented items
+      expect($('h2#in-this-article').length).toBe(1)
+      expect($('h2#in-this-article + ul li.ml-0').length).toBeGreaterThan(0) // non-indented items
+      expect($('h2#in-this-article + ul li.ml-3').length).toBeGreaterThan(0) // indented items
     })
 
     test('does not render mini TOC in articles with only one heading', async () => {
       const $ = await getDOM('/en/github/visualizing-repository-data-with-graphs/about-repository-graphs')
-      expect($('h3#in-this-article').length).toBe(0)
+      expect($('h2#in-this-article').length).toBe(0)
     })
 
     test('does not render mini TOC in articles with no headings', async () => {
       const $ = await getDOM('/en/github/authenticating-to-github/reviewing-your-deploy-keys')
-      expect($('h3#in-this-article').length).toBe(0)
+      expect($('h2#in-this-article').length).toBe(0)
     })
 
     test('does not render mini TOC in non-articles', async () => {
       const $ = await getDOM('/github/getting-started-with-github')
-      expect($('h3#in-this-article').length).toBe(0)
+      expect($('h2#in-this-article').length).toBe(0)
     })
   })
 
@@ -459,16 +459,16 @@ describe('server', () => {
       expect($('.markdown-body ul li a').length).toBeGreaterThan(5)
     })
 
-    test('map topic renders with h4 links to articles', async () => {
+    test('map topic renders with h2 links to articles', async () => {
       const $ = await getDOM('/en/github/setting-up-and-managing-your-github-user-account/managing-user-account-settings')
-      expect($(`a[href="/en/${nonEnterpriseDefaultVersion}/github/setting-up-and-managing-your-github-user-account/changing-your-github-username"] h4`).length).toBe(1)
+      expect($(`a[href="/en/${nonEnterpriseDefaultVersion}/github/setting-up-and-managing-your-github-user-account/changing-your-github-username"] h2`).length).toBe(1)
     })
 
-    test('map topic renders with one intro for every h4', async () => {
+    test('map topic renders with one intro for every h2', async () => {
       const $ = await getDOM('/en/github/setting-up-and-managing-your-github-user-account/managing-user-account-settings')
-      const $h4s = $('article a.link-with-intro')
-      expect($h4s.length).toBeGreaterThan(3)
-      $h4s.each((i, el) => {
+      const $h2s = $('article a.link-with-intro')
+      expect($h2s.length).toBeGreaterThan(3)
+      $h2s.each((i, el) => {
         expect($(el).next()[0].name).toBe('p')
       })
     })
@@ -635,10 +635,10 @@ describe('extended Markdown', () => {
 
   test('renders expected mini TOC headings in platform-specific content', async () => {
     const $ = await getDOM('/en/github/using-git/associating-text-editors-with-git')
-    expect($('h3#in-this-article').length).toBe(1)
-    expect($('h3#in-this-article + ul li.extended-markdown.mac').length).toBeGreaterThan(1)
-    expect($('h3#in-this-article + ul li.extended-markdown.windows').length).toBeGreaterThan(1)
-    expect($('h3#in-this-article + ul li.extended-markdown.linux').length).toBeGreaterThan(1)
+    expect($('h2#in-this-article').length).toBe(1)
+    expect($('h2#in-this-article + ul li.extended-markdown.mac').length).toBeGreaterThan(1)
+    expect($('h2#in-this-article + ul li.extended-markdown.windows').length).toBeGreaterThan(1)
+    expect($('h2#in-this-article + ul li.extended-markdown.linux').length).toBeGreaterThan(1)
   })
 })
 
diff --git a/tests/unit/liquid-helpers.js b/tests/unit/liquid-helpers.js
@@ -54,7 +54,7 @@ describe('liquid helper tags', () => {
     const template = '{% link_with_intro /contributing-and-collaborating-using-github-desktop %}'
     const page = pageMap[`/en/${nonEnterpriseDefaultVersion}/desktop/contributing-and-collaborating-using-github-desktop`]
     const expected = `<a class="link-with-intro Bump-link--hover no-underline" href="/en/desktop/contributing-and-collaborating-using-github-desktop">
-  <h4 class="link-with-intro-title">${page.title}<span class="Bump-link-symbol">→</span></h4>
+  <h2 class="link-with-intro-title f4">${page.title}<span class="Bump-link-symbol">→</span></h2>
 </a>
 <p class="link-with-intro-intro">${page.intro}</p>`
     const output = entities.decode(await liquid.parseAndRender(template, context))
